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

Author: odeke-em

Makefile

@@ -5,7 +5,7 @@ IMAGE_REPO = securego
 BUILDFLAGS := '-w -s'
 CGO_ENABLED = 0
 GO := GO111MODULE=on go
-GO_NOMOD :=GO111MODULE=off go
+GO_NOMOD :=GO111MODULE=on go
 GOPATH ?= $(shell $(GO) env GOPATH)
 GOBIN ?= $(GOPATH)/bin
 GOLINT ?= $(GOBIN)/golint
@@ -21,7 +21,7 @@ install-test-deps:
 	$(GO_NOMOD) get -u golang.org/x/crypto/ssh
 	$(GO_NOMOD) get -u github.com/lib/pq
 
-test: install-test-deps build fmt lint sec
+test: install-test-deps build fmt sec # lint -- disabled for minimal builds
 	$(GINKGO) -r -v
 
 fmt:

rules/rulelist.go

@@ -109,6 +109,7 @@ func Generate(filters ...RuleFilter) RuleList {
 		{"G703", "Errors that don't result in rollback", sdk.NewErrorNotPropagated},
 		{"G704", "Strconv invalid bitSize and cast", sdk.NewStrconvIntBitSizeOverflow},
 		{"G705", "Iterating over maps undeterministically", sdk.NewMapRangingCheck},
+		{"G706", "Non-consensus aware time.Now() usage", sdk.NewTimeNowRefusal},
 	}
 
 	ruleMap := make(map[string]RuleDefinition)

rules/rules_test.go

@@ -95,6 +95,10 @@ var _ = Describe("gosec rules", func() {
 			runner("G705", testutils.SampleCodeMapRangingNonDeterministic)
 		})
 
+		It("should detect non-consensus aware time.Now() usage", func() {
+			runner("G706", testutils.SampleCodeTimeNowNonCensusAware)
+		})
+
 		It("should detect DoS vulnerability via decompression bomb", func() {
 			runner("G110", testutils.SampleCodeG110)
 		})

rules/sdk/iterate_over_maps.go

@@ -35,12 +35,35 @@ func (mr *mapRanging) ID() string {
 	return mr.MetaData.ID
 }
 
+func extractIdent(call ast.Expr) *ast.Ident {
+	switch n := call.(type) {
+	case *ast.Ident:
+		return n
+
+	case *ast.SelectorExpr:
+		if ident, ok := n.X.(*ast.Ident); ok {
+			return ident
+		}
+		if n.Sel != nil {
+			return extractIdent(n.Sel)
+		}
+		return extractIdent(n.X)
+
+	default:
+		panic(fmt.Sprintf("Unhandled type: %T", call))
+	}
+}
+
 func (mr *mapRanging) Match(node ast.Node, ctx *gosec.Context) (*gosec.Issue, error) {
 	rangeStmt, ok := node.(*ast.RangeStmt)
 	if !ok {
 		return nil, nil
 	}
 
+	if rangeStmt.X == nil {
+		return nil, nil
+	}
+
 	// Algorithm:
 	// 1. Ensure that right hand side's eventual type is a map.
 	// 2. Ensure that only the form:
@@ -61,14 +84,44 @@ func (mr *mapRanging) Match(node ast.Node, ctx *gosec.Context) (*gosec.Issue, er
 	case *ast.CallExpr:
 		// Synthesize the declaration to be an *ast.FuncType from
 		// either function declarations or function literals.
-		idecl := rangeRHS.Fun.(*ast.Ident).Obj.Decl
+		ident := extractIdent(rangeRHS.Fun)
+		if ident == nil {
+			panic(fmt.Sprintf("Couldn't find ident: %#v\n", rangeRHS.Fun))
+		}
+		if ident.Obj == nil {
+			sel, ok := rangeRHS.Fun.(*ast.SelectorExpr)
+			if ok && sel.Sel != nil {
+				ident = extractIdent(sel.Sel)
+			}
+		}
+		if ident.Obj == nil {
+			return nil, nil
+		}
+
+		idecl := ident.Obj.Decl
 		switch idecl := idecl.(type) {
 		case *ast.FuncDecl:
 			decl = idecl.Type
 
 		case *ast.AssignStmt:
-			decl = idecl.Rhs[0].(*ast.FuncLit).Type
+			var err error
+			decl, err = typeOf(idecl.Rhs[0])
+			if err != nil {
+				return nil, err
+			}
+
 		}
+
+	case *ast.SelectorExpr:
+		if ident := extractIdent(rangeRHS.X); ident != nil {
+			decl = ident.Obj.Decl
+		} else {
+			panic(fmt.Sprintf("%#v\n", rangeRHS.X.(*ast.Ident)))
+		}
+	}
+
+	if decl == nil {
+		return nil, fmt.Errorf("failed to extract decl from: %T", rangeStmt.X)
 	}
 
 	switch decl := decl.(type) {
@@ -83,8 +136,7 @@ func (mr *mapRanging) Match(node ast.Node, ctx *gosec.Context) (*gosec.Issue, er
 		}
 
 	case *ast.AssignStmt:
-		rhs0 := decl.Rhs[0].(*ast.CompositeLit)
-		if _, ok := rhs0.Type.(*ast.MapType); !ok {
+		if skip := mapHandleAssignStmt(decl); skip {
 			return nil, nil
 		}
 
@@ -135,7 +187,12 @@ func (mr *mapRanging) Match(node ast.Node, ctx *gosec.Context) (*gosec.Issue, er
 		if !ok {
 			return nil, fmt.Errorf("expecting an identifier for an append call to a slice, got %T", stmt.Lhs[0])
 		}
-		if _, ok := typeOf(lhs0.Obj).(*ast.ArrayType); !ok {
+
+		typ, err := typeOf(lhs0.Obj)
+		if err != nil {
+			return nil, err
+		}
+		if _, ok := typ.(*ast.ArrayType); !ok {
 			return nil, fmt.Errorf("expecting an array/slice being used to retrieve keys, got %T", lhs0.Obj)
 		}
 
@@ -154,7 +211,24 @@ func (mr *mapRanging) Match(node ast.Node, ctx *gosec.Context) (*gosec.Issue, er
 	}
 }
 
-func eitherAppendOrDeleteCall(callExpr *ast.CallExpr) (string, bool) {
+func mapHandleAssignStmt(decl *ast.AssignStmt) (skip bool) {
+	switch rhs0 := decl.Rhs[0].(type) {
+	case *ast.CompositeLit:
+		if _, ok := rhs0.Type.(*ast.MapType); !ok {
+			return true
+		}
+		return false
+
+	case *ast.CallExpr:
+		return true
+
+	default:
+		// TODO: handle other types.
+		return true
+	}
+}
+
+func eitherAppendOrDeleteCall(callExpr *ast.CallExpr) (fnName string, ok bool) {
 	fn, ok := callExpr.Fun.(*ast.Ident)
 	if !ok {
 		return "", false
@@ -167,7 +241,7 @@ func eitherAppendOrDeleteCall(callExpr *ast.CallExpr) (string, bool) {
 	}
 }
 
-func typeOf(value interface{}) ast.Node {
+func typeOf(value interface{}) (ast.Node, error) {
 	switch typ := value.(type) {
 	case *ast.Object:
 		return typeOf(typ.Decl)
@@ -178,15 +252,31 @@ func typeOf(value interface{}) ast.Node {
 		if _, ok := rhs.(*ast.CallExpr); ok {
 			return typeOf(rhs)
 		}
+		if _, ok := rhs.(*ast.CompositeLit); ok {
+			return typeOf(rhs)
+		}
+
+		panic(fmt.Sprintf("Non-CallExpr: %#v\n", rhs))
 
 	case *ast.CallExpr:
 		decl := typ
 		fn := decl.Fun.(*ast.Ident)
 		if fn.Name == "make" {
 			// We can infer the type from the first argument.
-			return decl.Args[0]
+			return decl.Args[0], nil
 		}
 		return typeOf(decl.Args[0])
+
+	case *ast.CompositeLit:
+		return typ.Type, nil
+		panic(fmt.Sprintf("Non-CallExpr: %#v\n", typ))
+
+	case *ast.FuncLit:
+		returns := typ.Type.Results
+		if g, w := len(returns.List), 1; g != w {
+			return nil, fmt.Errorf("returns %d arguments, want %d", g, w)
+		}
+		return returns.List[0].Type, nil
 	}
 
 	panic(fmt.Sprintf("Unexpected type: %T", value))

rules/sdk/time_now.go

@@ -0,0 +1,84 @@
+// (c) Copyright 2021 Hewlett Packard Enterprise Development LP
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package sdk
+
+import (
+	"errors"
+	"go/ast"
+
+	"github.com/securego/gosec/v2"
+)
+
+// This static analyzer discourages the use of time.Now() as it was discovered that
+// its usage caused local non-determinism as reported and detailed at
+// https://forum.cosmos.network/t/cosmos-sdk-vulnerability-retrospective-security-advisory-jackfruit-october-12-2021/5349
+
+type timeNowCheck struct {
+	gosec.MetaData
+	calls gosec.CallList
+}
+
+func (tmc *timeNowCheck) ID() string { return tmc.MetaData.ID }
+
+func (tmc *timeNowCheck) Match(node ast.Node, ctx *gosec.Context) (*gosec.Issue, error) {
+	// We want to catch all function invocations as well as assignments of any of the form:
+	// .Value = time.Now().*
+	// fn := time.Now
+	callExpr, ok := node.(*ast.CallExpr)
+	if !ok {
+		return nil, nil
+	}
+
+	sel, ok := callExpr.Fun.(*ast.SelectorExpr)
+	if !ok {
+		return nil, nil
+	}
+
+	if sel.Sel.Name != "Now" {
+		return nil, nil
+	}
+
+	switch x := sel.X.(type) {
+	case *ast.Ident:
+		if x.Name != "time" {
+			return nil, nil
+		}
+
+	case *ast.SelectorExpr:
+		if x.Sel.Name != "time" {
+			return nil, nil
+		}
+	}
+
+	// By this point issue the error.
+	return nil, errors.New("time.Now() is non-deterministic for distributed consensus, you should use the current Block's timestamp")
+}
+
+func NewTimeNowRefusal(id string, config gosec.Config) (rule gosec.Rule, nodes []ast.Node) {
+	calls := gosec.NewCallList()
+
+	tnc := &timeNowCheck{
+		MetaData: gosec.MetaData{
+			ID:         id,
+			Severity:   gosec.High,
+			Confidence: gosec.High,
+			What:       "Non-determinism from using non-consensus aware time.Now()",
+		},
+		calls: calls,
+	}
+
+	nodes = append(nodes, (*ast.CallExpr)(nil))
+	return tnc, nodes
+}

testutils/source.go

@@ -2456,4 +2456,23 @@ func do() map[string]string { return nil }
 `}, 3, gosec.NewConfig(),
 		},
 	}
+
+	SampleCodeTimeNowNonCensusAware = []CodeSample{
+		{
+			[]string{`
+package main
+
+func main() {
+        _ = time.Now()
+}`}, 3, gosec.NewConfig(),
+		},
+		{
+			[]string{`
+package main
+
+func main() {
+        _ = time.Now().Unix()
+}`}, 3, gosec.NewConfig(),
+		},
+	}
 )