diff --git a/Makefile b/Makefile index c720619..ea42cfc 100644 --- a/Makefile +++ b/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: diff --git a/rules/rulelist.go b/rules/rulelist.go index fdfd2e4..ebcb556 100644 --- a/rules/rulelist.go +++ b/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) diff --git a/rules/rules_test.go b/rules/rules_test.go index f3403ee..02adb9a 100644 --- a/rules/rules_test.go +++ b/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) }) diff --git a/rules/sdk/iterate_over_maps.go b/rules/sdk/iterate_over_maps.go index 62627df..074b2da 100644 --- a/rules/sdk/iterate_over_maps.go +++ b/rules/sdk/iterate_over_maps.go @@ -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 { @@ -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 @@ -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 } @@ -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 { @@ -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)) diff --git a/rules/sdk/time_now.go b/rules/sdk/time_now.go new file mode 100644 index 0000000..c174ea7 --- /dev/null +++ b/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 +} diff --git a/testutils/source.go b/testutils/source.go index cd49d07..f09a6dc 100644 --- a/testutils/source.go +++ b/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(), + }, + } )