Skip to content

Commit 3245b93

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 8dfbd79 commit 3245b93

File tree

6 files changed

+152
-5
lines changed

6 files changed

+152
-5
lines changed

Makefile

+2-2
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ IMAGE_REPO = securego
55
BUILDFLAGS := '-w -s'
66
CGO_ENABLED = 0
77
GO := GO111MODULE=on go
8-
GO_NOMOD :=GO111MODULE=off go
8+
GO_NOMOD :=GO111MODULE=on go
99
GOPATH ?= $(shell $(GO) env GOPATH)
1010
GOBIN ?= $(GOPATH)/bin
1111
GOLINT ?= $(GOBIN)/golint
@@ -21,7 +21,7 @@ install-test-deps:
2121
$(GO_NOMOD) get -u golang.org/x/crypto/ssh
2222
$(GO_NOMOD) get -u github.com/lib/pq
2323

24-
test: install-test-deps build fmt lint sec
24+
test: install-test-deps build fmt sec # lint -- disabled for minimal builds
2525
$(GINKGO) -r -v
2626

2727
fmt:

rules/rulelist.go

+1
Original file line numberDiff line numberDiff line change
@@ -109,6 +109,7 @@ func Generate(filters ...RuleFilter) RuleList {
109109
{"G703", "Errors that don't result in rollback", sdk.NewErrorNotPropagated},
110110
{"G704", "Strconv invalid bitSize and cast", sdk.NewStrconvIntBitSizeOverflow},
111111
{"G705", "Iterating over maps undeterministically", sdk.NewMapRangingCheck},
112+
{"G706", "Non-consensus aware time.Now() usage", sdk.NewTimeNowRefusal},
112113
}
113114

114115
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 non-consensus aware time.Now() usage", func() {
99+
runner("G706", testutils.SampleCodeTimeNowNonCensusAware)
100+
})
101+
98102
It("should detect DoS vulnerability via decompression bomb", func() {
99103
runner("G110", testutils.SampleCodeG110)
100104
})

rules/sdk/iterate_over_maps.go

+42-3
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,22 @@ func (mr *mapRanging) ID() string {
3535
return mr.MetaData.ID
3636
}
3737

38+
func extractIdent(call ast.Expr) *ast.Ident {
39+
switch n := call.(type) {
40+
case *ast.Ident:
41+
return n
42+
43+
case *ast.SelectorExpr:
44+
if ident, ok := n.X.(*ast.Ident); ok {
45+
return ident
46+
}
47+
return extractIdent(n.X)
48+
49+
default:
50+
panic(fmt.Sprintf("Unhandled type: %T", call))
51+
}
52+
}
53+
3854
func (mr *mapRanging) Match(node ast.Node, ctx *gosec.Context) (*gosec.Issue, error) {
3955
rangeStmt, ok := node.(*ast.RangeStmt)
4056
if !ok {
@@ -61,7 +77,7 @@ func (mr *mapRanging) Match(node ast.Node, ctx *gosec.Context) (*gosec.Issue, er
6177
case *ast.CallExpr:
6278
// Synthesize the declaration to be an *ast.FuncType from
6379
// either function declarations or function literals.
64-
idecl := rangeRHS.Fun.(*ast.Ident).Obj.Decl
80+
idecl := extractIdent(rangeRHS.Fun).Obj.Decl
6581
switch idecl := idecl.(type) {
6682
case *ast.FuncDecl:
6783
decl = idecl.Type
@@ -83,8 +99,7 @@ func (mr *mapRanging) Match(node ast.Node, ctx *gosec.Context) (*gosec.Issue, er
8399
}
84100

85101
case *ast.AssignStmt:
86-
rhs0 := decl.Rhs[0].(*ast.CompositeLit)
87-
if _, ok := rhs0.Type.(*ast.MapType); !ok {
102+
if skip := mapHandleAssignStmt(decl); skip {
88103
return nil, nil
89104
}
90105

@@ -154,6 +169,23 @@ func (mr *mapRanging) Match(node ast.Node, ctx *gosec.Context) (*gosec.Issue, er
154169
}
155170
}
156171

172+
func mapHandleAssignStmt(decl *ast.AssignStmt) (skip bool) {
173+
switch rhs0 := decl.Rhs[0].(type) {
174+
case *ast.CompositeLit:
175+
if _, ok := rhs0.Type.(*ast.MapType); !ok {
176+
return true
177+
}
178+
return false
179+
180+
case *ast.CallExpr:
181+
return true
182+
183+
default:
184+
// TODO: handle other types.
185+
return true
186+
}
187+
}
188+
157189
func eitherAppendOrDeleteCall(callExpr *ast.CallExpr) (string, bool) {
158190
fn, ok := callExpr.Fun.(*ast.Ident)
159191
if !ok {
@@ -187,6 +219,13 @@ func typeOf(value interface{}) ast.Node {
187219
return decl.Args[0]
188220
}
189221
return typeOf(decl.Args[0])
222+
223+
case *ast.AssignStmt:
224+
ident, ok := typ.Lhs[0].(*ast.Ident)
225+
if ok {
226+
return ident
227+
}
228+
return typeOf(typ.Lhs[0])
190229
}
191230

192231
panic(fmt.Sprintf("Unexpected type: %T", value))

rules/sdk/time_now.go

+84
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,84 @@
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/securego/gosec/v2"
22+
)
23+
24+
// This static analyzer discourages the use of time.Now() as it was discovered that
25+
// its usage caused local non-determinism as reported and detailed at
26+
// https://forum.cosmos.network/t/cosmos-sdk-vulnerability-retrospective-security-advisory-jackfruit-october-12-2021/5349
27+
28+
type timeNowCheck struct {
29+
gosec.MetaData
30+
calls gosec.CallList
31+
}
32+
33+
func (tmc *timeNowCheck) ID() string { return tmc.MetaData.ID }
34+
35+
func (tmc *timeNowCheck) Match(node ast.Node, ctx *gosec.Context) (*gosec.Issue, error) {
36+
// We want to catch all function invocations as well as assignments of any of the form:
37+
// .Value = time.Now().*
38+
// fn := time.Now
39+
callExpr, ok := node.(*ast.CallExpr)
40+
if !ok {
41+
return nil, nil
42+
}
43+
44+
sel, ok := callExpr.Fun.(*ast.SelectorExpr)
45+
if !ok {
46+
return nil, nil
47+
}
48+
49+
if sel.Sel.Name != "Now" {
50+
return nil, nil
51+
}
52+
53+
switch x := sel.X.(type) {
54+
case *ast.Ident:
55+
if x.Name != "time" {
56+
return nil, nil
57+
}
58+
59+
case *ast.SelectorExpr:
60+
if x.Sel.Name != "time" {
61+
return nil, nil
62+
}
63+
}
64+
65+
// By this point issue the error.
66+
return nil, errors.New("time.Now() is non-deterministic for distributed consensus, you should use the current Block's timestamp")
67+
}
68+
69+
func NewTimeNowRefusal(id string, config gosec.Config) (rule gosec.Rule, nodes []ast.Node) {
70+
calls := gosec.NewCallList()
71+
72+
tnc := &timeNowCheck{
73+
MetaData: gosec.MetaData{
74+
ID: id,
75+
Severity: gosec.High,
76+
Confidence: gosec.High,
77+
What: "Non-determinism from using non-consensus aware time.Now()",
78+
},
79+
calls: calls,
80+
}
81+
82+
nodes = append(nodes, (*ast.CallExpr)(nil))
83+
return tnc, nodes
84+
}

testutils/source.go

+19
Original file line numberDiff line numberDiff line change
@@ -2456,4 +2456,23 @@ func do() map[string]string { return nil }
24562456
`}, 3, gosec.NewConfig(),
24572457
},
24582458
}
2459+
2460+
SampleCodeTimeNowNonCensusAware = []CodeSample{
2461+
{
2462+
[]string{`
2463+
package main
2464+
2465+
func main() {
2466+
_ = time.Now()
2467+
}`}, 3, gosec.NewConfig(),
2468+
},
2469+
{
2470+
[]string{`
2471+
package main
2472+
2473+
func main() {
2474+
_ = time.Now().Unix()
2475+
}`}, 3, gosec.NewConfig(),
2476+
},
2477+
}
24592478
)

0 commit comments

Comments
 (0)