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/time_now.go b/rules/sdk/time_now.go new file mode 100644 index 0000000..a145d5a --- /dev/null +++ b/rules/sdk/time_now.go @@ -0,0 +1,76 @@ +// (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 + switch n := node.(type) { + default: + // TODO: Add more cases for which time.Now can be aliased or used as a variable. + return nil, nil + + case *ast.CallExpr: + sel, ok := n.Fun.(*ast.SelectorExpr) + if !ok { + return nil, nil + } + if sel.X.(*ast.Ident).Name != "time" { + return nil, nil + } + if sel.Sel.Name != "Now" { + return nil, nil + } + + // Otherwise issue the error. + return nil, errors.New("time.Now() is non-deterministic for distributed computing, 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(), + }, + } )