Skip to content

Commit 308efee

Browse files
committed
rules/sdk: add pass to reject time.Now()
time.Now() uses local clocks that unfortunately when used in distributed consensus introduce lots of skew and can be exploited in vulnerabilities. Instead there is consensus aware clock whose timestamp is embedded in the current Block. This pass rejects usages as per https://forum.cosmos.network/t/cosmos-sdk-vulnerability-retrospective-security-advisory-jackfruit-october-12-2021/5349 Updates #1
1 parent cffc933 commit 308efee

File tree

4 files changed

+109
-0
lines changed

4 files changed

+109
-0
lines changed

rules/rulelist.go

+1
Original file line numberDiff line numberDiff line change
@@ -117,6 +117,7 @@ func Generate(filters ...RuleFilter) RuleList {
117117
{"G703", "Errors that don't result in rollback", sdk.NewErrorNotPropagated},
118118
{"G704", "Strconv invalid bitSize and cast", sdk.NewStrconvIntBitSizeOverflow},
119119
{"G705", "Iterating over maps undeterministically", sdk.NewMapRangingCheck},
120+
{"G706", "Use of time.Now() in consensus code could lead to chain halt", sdk.NewTimeNowRefusal},
120121
}
121122

122123
ruleMap := make(map[string]RuleDefinition)

rules/rules_test.go

+4
Original file line numberDiff line numberDiff line change
@@ -95,6 +95,10 @@ var _ = Describe("gosec rules", func() {
9595
runner("G705", testutils.SampleCodeMapRangingNonDeterministic)
9696
})
9797

98+
It("should detect use of time.Now() in consensus code which could lead to chain halt", func() {
99+
runner("G706", testutils.SampleCodeTimeNowNonConsensusAware)
100+
})
101+
98102
It("should detect DoS vulnerability via decompression bomb", func() {
99103
runner("G110", testutils.SampleCodeG110)
100104
})

rules/sdk/time_now.go

+83
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,83 @@
1+
// (c) Copyright 2021 Hewlett Packard Enterprise Development LP
2+
//
3+
// Licensed under the Apache License, Version 2.0 (the "License");
4+
// you may not use this file except in compliance with the License.
5+
// You may obtain a copy of the License at
6+
//
7+
// http://www.apache.org/licenses/LICENSE-2.0
8+
//
9+
// Unless required by applicable law or agreed to in writing, software
10+
// distributed under the License is distributed on an "AS IS" BASIS,
11+
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
12+
// See the License for the specific language governing permissions and
13+
// limitations under the License.
14+
15+
package sdk
16+
17+
import (
18+
"errors"
19+
"go/ast"
20+
21+
"github.com/cosmos/gosec/v2"
22+
)
23+
24+
type timeNowCheck struct {
25+
gosec.MetaData
26+
calls gosec.CallList
27+
}
28+
29+
func (tmc *timeNowCheck) ID() string { return tmc.MetaData.ID }
30+
31+
func (tmc *timeNowCheck) Match(node ast.Node, ctx *gosec.Context) (*gosec.Issue, error) {
32+
// We want to catch all function invocations as well as assignments of any of the form:
33+
// .Value = time.Now().*
34+
// fn := time.Now
35+
callExpr, ok := node.(*ast.CallExpr)
36+
if !ok {
37+
return nil, nil
38+
}
39+
40+
sel, ok := callExpr.Fun.(*ast.SelectorExpr)
41+
if !ok {
42+
return nil, nil
43+
}
44+
45+
if sel.Sel.Name != "Now" {
46+
return nil, nil
47+
}
48+
49+
switch x := sel.X.(type) {
50+
case *ast.Ident:
51+
if x.Name != "time" {
52+
return nil, nil
53+
}
54+
55+
case *ast.SelectorExpr:
56+
if x.Sel.Name != "time" {
57+
return nil, nil
58+
}
59+
}
60+
61+
// By this point issue the error.
62+
return nil, errors.New("time.Now() is non-deterministic for distributed consensus, you should use the current Block's timestamp")
63+
}
64+
65+
// NewTimeNowRefusal discourages the use of time.Now() as it was discovered that
66+
// its usage caused local non-determinism and chain halting, as reported and detailed at
67+
// https://forum.cosmos.network/t/cosmos-sdk-vulnerability-retrospective-security-advisory-jackfruit-october-12-2021/5349
68+
func NewTimeNowRefusal(id string, config gosec.Config) (rule gosec.Rule, nodes []ast.Node) {
69+
calls := gosec.NewCallList()
70+
71+
tnc := &timeNowCheck{
72+
MetaData: gosec.MetaData{
73+
ID: id,
74+
Severity: gosec.High,
75+
Confidence: gosec.High,
76+
What: "Non-determinism from using non-consensus aware time.Now() can cause a chain halt",
77+
},
78+
calls: calls,
79+
}
80+
81+
nodes = append(nodes, (*ast.CallExpr)(nil))
82+
return tnc, nodes
83+
}

testutils/source.go

+21
Original file line numberDiff line numberDiff line change
@@ -2528,4 +2528,25 @@ func noop(keys []string) []string {return keys}
25282528
`}, 13, gosec.NewConfig(),
25292529
},
25302530
}
2531+
2532+
// SampleCodeTimeNowNonConsensusAware is a sample in which we
2533+
// should flag the usage of time.Now() which can lead to chain halt.
2534+
SampleCodeTimeNowNonConsensusAware = []CodeSample{
2535+
{
2536+
[]string{`
2537+
package main
2538+
2539+
func main() {
2540+
_ = time.Now()
2541+
}`}, 3, gosec.NewConfig(),
2542+
},
2543+
{
2544+
[]string{`
2545+
package main
2546+
2547+
func main() {
2548+
_ = time.Now().Unix()
2549+
}`}, 3, gosec.NewConfig(),
2550+
},
2551+
}
25312552
)

0 commit comments

Comments
 (0)