rules/sdk: use ctx.Info.TypeOf() to get types (#25)

This lets us determine types much more reliably without having to worry about adding more cases to switch statements and pulling out the debugger when we panic on a new AST structure we haven't handled yet. It's always nice to half the number of lines in a file as well :) And of course this change causes the rule to notice more map statements that it previously mixed, so I've also fixed those. I also discovered a new case that this rule incorrectly flags -- map copying is safe to do directly. I've filed #24 and suppressed the rule for the map copy in analyzer.go. With this change, I'm able to run gosec on cosmos/cosmos-sdk without it crashing. Updates cosmos/cosmos-sdk#10572
cosmos · Jun 9, 2022 · f6f1167 · f6f1167
1 parent 74c0f4d
commit f6f1167
Show file tree

Hide file tree

Showing 11 changed files with 98 additions and 207 deletions.
diff --git a/analyzer.go b/analyzer.go
@@ -356,6 +356,8 @@ func (gosec *Analyzer) Visit(n ast.Node) ast.Visitor {
 	// Now create the union of exclusions.
 	ignores := map[string]bool{}
 	if len(gosec.context.Ignores) > 0 {
+		// TODO(https://github.com/informalsystems/gosec/issues/24): Map copying is currently incorrectly flagged by G705. When this is fixed, we can remove the below annotation.
+		// #nosec G705
 		for k, v := range gosec.context.Ignores[0] {
 			ignores[k] = v
 		}

diff --git a/cmd/gosecutil/tools.go b/cmd/gosecutil/tools.go
@@ -23,6 +23,7 @@ import (
 	"go/token"
 	"go/types"
 	"os"
+	"sort"
 	"strings"
 )
 
@@ -46,12 +47,11 @@ func newUtils() *utilities {
 }
 
 func (u *utilities) String() string {
-	i := 0
-	keys := make([]string, len(u.commands))
+	keys := make([]string, 0, len(u.commands))
 	for k := range u.commands {
-		keys[i] = k
-		i++
+		keys = append(keys, k)
 	}
+	sort.Slice(keys, func(i, j int) bool { return keys[i] < keys[j] })
 	return strings.Join(keys, ", ")
 }
 
@@ -207,8 +207,13 @@ func dumpUses(files ...string) {
 		if !checkContext(context, file) {
 			return
 		}
-		for ident, obj := range context.info.Uses {
-			fmt.Printf("IDENT: %v, OBJECT: %v\n", ident, obj)
+		idents := make([]*ast.Ident, 0, len(context.info.Uses))
+		for ident := range context.info.Uses {
+			idents = append(idents, ident)
+		}
+		sort.Slice(idents, func(i, j int) bool { return idents[i].String() < idents[j].String() })
+		for _, ident := range idents {
+			fmt.Printf("IDENT: %v, OBJECT: %v\n", ident, context.info.Uses[ident])
 		}
 	}
 }
@@ -222,8 +227,13 @@ func dumpTypes(files ...string) {
 		if !checkContext(context, file) {
 			return
 		}
-		for expr, tv := range context.info.Types {
-			fmt.Printf("EXPR: %v, TYPE: %v\n", expr, tv)
+		exprs := make([]ast.Expr, 0, len(context.info.Types))
+		for expr := range context.info.Types {
+			exprs = append(exprs, expr)
+		}
+		sort.Slice(exprs, func(i, j int) bool { return fmt.Sprintf("%v", exprs[i]) < fmt.Sprintf("%v", exprs[j]) })
+		for _, expr := range exprs {
+			fmt.Printf("EXPR: %v, TYPE: %v\n", expr, context.info.Types[expr])
 		}
 	}
 }
@@ -237,8 +247,13 @@ func dumpDefs(files ...string) {
 		if !checkContext(context, file) {
 			return
 		}
-		for ident, obj := range context.info.Defs {
-			fmt.Printf("IDENT: %v, OBJ: %v\n", ident, obj)
+		idents := make([]*ast.Ident, 0, len(context.info.Defs))
+		for ident := range context.info.Defs {
+			idents = append(idents, ident)
+		}
+		sort.Slice(idents, func(i, j int) bool { return idents[i].String() < idents[j].String() })
+		for _, ident := range idents {
+			fmt.Printf("IDENT: %v, OBJECT: %v\n", ident, context.info.Defs[ident])
 		}
 	}
 }

diff --git a/config.go b/config.go
@@ -6,6 +6,7 @@ import (
 	"fmt"
 	"io"
 	"io/ioutil"
+	"sort"
 )
 
 const (
@@ -46,8 +47,13 @@ func (c Config) convertGlobals() {
 	if globals, ok := c[Globals]; ok {
 		if settings, ok := globals.(map[string]interface{}); ok {
 			validGlobals := map[GlobalOption]string{}
-			for k, v := range settings {
-				validGlobals[c.keyToGlobalOptions(k)] = fmt.Sprintf("%v", v)
+			keys := make([]string, 0, len(settings))
+			for k := range settings {
+				keys = append(keys, k)
+			}
+			sort.Slice(keys, func(i, j int) bool { return keys[i] < keys[j] })
+			for _, k := range keys {
+				validGlobals[c.keyToGlobalOptions(k)] = fmt.Sprintf("%v", settings[k])
 			}
 			c[Globals] = validGlobals
 		}

diff --git a/go.mod b/go.mod
@@ -8,17 +8,17 @@ require (
 	github.com/nbutton23/zxcvbn-go v0.0.0-20180912185939-ae427f1e4c1d
 	github.com/onsi/ginkgo v1.16.5
 	github.com/onsi/gomega v1.10.3
-	golang.org/x/tools v0.1.10
+	golang.org/x/tools v0.1.11
 	gopkg.in/yaml.v2 v2.3.0
 )
 
 require (
 	github.com/fsnotify/fsnotify v1.5.4 // indirect
 	github.com/kr/pretty v0.1.0 // indirect
 	github.com/nxadm/tail v1.4.8 // indirect
-	golang.org/x/mod v0.6.0-dev.0.20220106191415-9b9b3d81d5e3 // indirect
+	golang.org/x/mod v0.6.0-dev.0.20220419223038-86c51ed26bb4 // indirect
 	golang.org/x/net v0.0.0-20211112202133-69e39bad7dc2 // indirect
-	golang.org/x/sys v0.0.0-20220520151302-bc2c85ada10a // indirect
+	golang.org/x/sys v0.0.0-20220608164250-635b8c9b7f68 // indirect
 	golang.org/x/text v0.3.7 // indirect
 	golang.org/x/xerrors v0.0.0-20200804184101-5ec99f83aff1 // indirect
 	gopkg.in/check.v1 v1.0.0-20190902080502-41f04d3bba15 // indirect

diff --git a/go.sum b/go.sum
@@ -53,8 +53,8 @@ golang.org/x/crypto v0.0.0-20191011191535-87dc89f01550/go.mod h1:yigFU9vqHzYiE8U
 golang.org/x/crypto v0.0.0-20200622213623-75b288015ac9/go.mod h1:LzIPMQfyMNhhGPhUkYOs5KpL4U8rLKemX1yGLhDgUto=
 golang.org/x/crypto v0.0.0-20210921155107-089bfa567519/go.mod h1:GvvjBRRGRdwPK5ydBHafDWAxML/pGHZbMvKqRZ5+Abc=
 golang.org/x/mod v0.3.0/go.mod h1:s0Qsj1ACt9ePp/hMypM3fl4fZqREWJwdYDEqhRiZZUA=
-golang.org/x/mod v0.6.0-dev.0.20220106191415-9b9b3d81d5e3 h1:kQgndtyPBW/JIYERgdxfwMYh3AVStj88WQTlNDi2a+o=
-golang.org/x/mod v0.6.0-dev.0.20220106191415-9b9b3d81d5e3/go.mod h1:3p9vT2HGsQu2K1YbXdKPJLVgG5VJdoTa1poYQBtP1AY=
+golang.org/x/mod v0.6.0-dev.0.20220419223038-86c51ed26bb4 h1:6zppjxzCulZykYSLyVDYbneBfbaBIQPYMevg0bEwv2s=
+golang.org/x/mod v0.6.0-dev.0.20220419223038-86c51ed26bb4/go.mod h1:jJ57K6gSWd91VN4djpZkiMVwK6gcyfeH4XE8wZrZaV4=
 golang.org/x/net v0.0.0-20180906233101-161cd47e91fd/go.mod h1:mL1N/T3taQHkDXs73rZJwtUhF3w3ftmwwsq0BUmARs4=
 golang.org/x/net v0.0.0-20190404232315-eb5bcb51f2a3/go.mod h1:t9HGtf8HONx5eT2rtn7q6eTqICYqUVnKs3thJo3Qplg=
 golang.org/x/net v0.0.0-20190620200207-3b0461eec859/go.mod h1:z5CRVTTTmAJ677TzLLGU+0bjPO0LkuOLi4/5GtJWs/s=
@@ -83,8 +83,8 @@ golang.org/x/sys v0.0.0-20210423082822-04245dca01da/go.mod h1:h1NjWce9XRLGQEsW7w
 golang.org/x/sys v0.0.0-20210615035016-665e8c7367d1/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
 golang.org/x/sys v0.0.0-20211019181941-9d821ace8654/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
 golang.org/x/sys v0.0.0-20220412211240-33da011f77ad/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
-golang.org/x/sys v0.0.0-20220520151302-bc2c85ada10a h1:dGzPydgVsqGcTRVwiLJ1jVbufYwmzD3LfVPLKsKg+0k=
-golang.org/x/sys v0.0.0-20220520151302-bc2c85ada10a/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
+golang.org/x/sys v0.0.0-20220608164250-635b8c9b7f68 h1:z8Hj/bl9cOV2grsOpEaQFUaly0JWN3i97mo3jXKJNp0=
+golang.org/x/sys v0.0.0-20220608164250-635b8c9b7f68/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
 golang.org/x/term v0.0.0-20201126162022-7de9c90e9dd1/go.mod h1:bj7SfCRtBDWHUb9snDiAeCFNEtKQo2Wmx5Cou7ajbmo=
 golang.org/x/text v0.3.0/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ=
 golang.org/x/text v0.3.3/go.mod h1:5Zoc/QRtKVWzQhOtBMvqHzDpF6irO9z98xDceosuGiQ=
@@ -94,8 +94,8 @@ golang.org/x/text v0.3.7/go.mod h1:u+2+/6zg+i71rQMx5EYifcz6MCKuco9NR6JIITiCfzQ=
 golang.org/x/tools v0.0.0-20180917221912-90fa682c2a6e/go.mod h1:n7NCudcB/nEzxVGmLbDWY5pfWTLqBcC2KZ6jyYvM4mQ=
 golang.org/x/tools v0.0.0-20191119224855-298f0cb1881e/go.mod h1:b+2E5dAYhXwXZwtnZ6UAqBI28+e2cm9otk0dWdXHAEo=
 golang.org/x/tools v0.0.0-20201224043029-2b0845dc783e/go.mod h1:emZCQorbCU4vsT4fOWvOPXz4eW1wZW4PmDk9uLelYpA=
-golang.org/x/tools v0.1.10 h1:QjFRCZxdOhBJ/UNgnBZLbNV13DlbnK0quyivTnXJM20=
-golang.org/x/tools v0.1.10/go.mod h1:Uh6Zz+xoGYZom868N8YTex3t7RhtHDBrE8Gzo9bV56E=
+golang.org/x/tools v0.1.11 h1:loJ25fNOEhSXfHrpoGj91eCUThwdNX6u24rO1xnNteY=
+golang.org/x/tools v0.1.11/go.mod h1:SgwaegtQh8clINPpECJMqnxLv9I09HLqnW3RMqW0CA4=
 golang.org/x/xerrors v0.0.0-20190717185122-a985d3407aa7/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0=
 golang.org/x/xerrors v0.0.0-20191011141410-1b5146add898/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0=
 golang.org/x/xerrors v0.0.0-20191204190536-9bdfabe68543/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0=

diff --git a/helpers.go b/helpers.go
@@ -25,6 +25,7 @@ import (
 	"path/filepath"
 	"regexp"
 	"runtime" // #nosec G702
+	"sort"
 	"strconv"
 	"strings"
 )
@@ -267,7 +268,12 @@ func GetImportedName(path string, ctx *Context) (string, bool) {
 // GetImportPath resolves the full import path of an identifier based on
 // the imports in the current context.
 func GetImportPath(name string, ctx *Context) (string, bool) {
+	paths := make([]string, 0, len(ctx.Imports.Imported))
 	for path := range ctx.Imports.Imported {
+		paths = append(paths, path)
+	}
+	sort.Slice(paths, func(i, j int) bool { return paths[i] < paths[j] })
+	for _, path := range paths {
 		if imported, ok := GetImportedName(path, ctx); ok && imported == name {
 			return path, true
 		}

diff --git a/rules/errors.go b/rules/errors.go
@@ -17,6 +17,7 @@ package rules
 import (
 	"go/ast"
 	"go/types"
+	"sort"
 
 	"github.com/informalsystems/gosec/v2"
 )
@@ -89,8 +90,13 @@ func NewNoErrorCheck(id string, conf gosec.Config) (gosec.Rule, []ast.Node) {
 
 	if configured, ok := conf["G104"]; ok {
 		if whitelisted, ok := configured.(map[string]interface{}); ok {
-			for pkg, funcs := range whitelisted {
-				if funcs, ok := funcs.([]interface{}); ok {
+			pkgs := make([]string, 0, len(whitelisted))
+			for pkg := range whitelisted {
+				pkgs = append(pkgs, pkg)
+			}
+			sort.Slice(pkgs, func(i, j int) bool { return pkgs[i] < pkgs[j] })
+			for _, pkg := range pkgs {
+				if funcs, ok := whitelisted[pkg].([]interface{}); ok {
 					whitelist.AddAll(pkg, toStringSlice(funcs)...)
 				}
 			}

diff --git a/rules/rulelist.go b/rules/rulelist.go
@@ -15,6 +15,8 @@
 package rules
 
 import (
+	"sort"
+
 	"github.com/informalsystems/gosec/v2"
 	"github.com/informalsystems/gosec/v2/rules/sdk"
 )
@@ -32,8 +34,14 @@ type RuleList map[string]RuleDefinition
 
 // Builders returns all the create methods for a given rule list
 func (rl RuleList) Builders() map[string]gosec.RuleBuilder {
+	ids := make([]string, 0, len(rl))
+	for id := range rl {
+		ids = append(ids, id)
+	}
+	sort.Slice(ids, func(i, j int) bool { return ids[i] < ids[j] })
 	builders := make(map[string]gosec.RuleBuilder)
-	for _, def := range rl {
+	for _, id := range ids {
+		def := rl[id]
 		builders[def.ID] = def.Create
 	}
 	return builders