Skip to content

Commit

Permalink
rules/sdk: add pass to reject time.Now()
Browse files Browse the repository at this point in the history
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
  • Loading branch information
odeke-em committed Nov 22, 2021
1 parent 8dfbd79 commit 3245b93
Show file tree
Hide file tree
Showing 6 changed files with 152 additions and 5 deletions.
4 changes: 2 additions & 2 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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:
Expand Down
1 change: 1 addition & 0 deletions rules/rulelist.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
4 changes: 4 additions & 0 deletions rules/rules_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
})
Expand Down
45 changes: 42 additions & 3 deletions rules/sdk/iterate_over_maps.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,22 @@ 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
}
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 {
Expand All @@ -61,7 +77,7 @@ 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
idecl := extractIdent(rangeRHS.Fun).Obj.Decl
switch idecl := idecl.(type) {
case *ast.FuncDecl:
decl = idecl.Type
Expand All @@ -83,8 +99,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
}

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

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) (string, bool) {
fn, ok := callExpr.Fun.(*ast.Ident)
if !ok {
Expand Down Expand Up @@ -187,6 +219,13 @@ func typeOf(value interface{}) ast.Node {
return decl.Args[0]
}
return typeOf(decl.Args[0])

case *ast.AssignStmt:
ident, ok := typ.Lhs[0].(*ast.Ident)
if ok {
return ident
}
return typeOf(typ.Lhs[0])
}

panic(fmt.Sprintf("Unexpected type: %T", value))
Expand Down
84 changes: 84 additions & 0 deletions rules/sdk/time_now.go
Original file line number Diff line number Diff line change
@@ -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
}
19 changes: 19 additions & 0 deletions testutils/source.go
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
},
}
)

0 comments on commit 3245b93

Please sign in to comment.