Detect the unhandled errors even though they are explicitly ignored if the 'audit: enabled' setting is defined in the global configuration (#274)

* Define more explicit the global options in the configuration

* Detect in audit mode the unhandled errors even thought they are explicitly ignored

Author: Cosmin Cojocar

analyzer.go

@@ -41,7 +41,7 @@ type Context struct {
 	Pkg      *types.Package
 	PkgFiles []*ast.File
 	Root     *ast.File
-	Config   map[string]interface{}
+	Config   Config
 	Imports  *ImportTracker
 	Ignores  []map[string]bool
 }
@@ -69,8 +69,8 @@ type Analyzer struct {
 // NewAnalyzer builds a new analyzer.
 func NewAnalyzer(conf Config, logger *log.Logger) *Analyzer {
 	ignoreNoSec := false
-	if setting, err := conf.GetGlobal("nosec"); err == nil {
-		ignoreNoSec = setting == "true" || setting == "enabled"
+	if enabled, err := conf.IsGlobalEnabled(Nosec); err == nil {
+		ignoreNoSec = enabled
 	}
 	if logger == nil {
 		logger = log.New(os.Stderr, "[gosec]", log.LstdFlags)

analyzer_test.go

@@ -201,7 +201,7 @@ var _ = Describe("Analyzer", func() {
 
 		// overwrite nosec option
 		nosecIgnoreConfig := gosec.NewConfig()
-		nosecIgnoreConfig.SetGlobal("nosec", "true")
+		nosecIgnoreConfig.SetGlobal(gosec.Nosec, "true")
 		customAnalyzer := gosec.NewAnalyzer(nosecIgnoreConfig, logger)
 		customAnalyzer.LoadRules(rules.Generate(rules.NewRuleFilter(false, "G401")).Builders())
 

cmd/gosec/main.go

@@ -137,7 +137,7 @@ func loadConfig(configFile string) (gosec.Config, error) {
 		}
 	}
 	if *flagIgnoreNoSec {
-		config.SetGlobal("nosec", "true")
+		config.SetGlobal(gosec.Nosec, "true")
 	}
 	return config, nil
 }

config.go

@@ -14,6 +14,16 @@ const (
 	Globals = "global"
 )
 
+// GlobalOption defines the name of the global options
+type GlobalOption string
+
+const (
+	// Nosec global option for #nosec directive
+	Nosec GlobalOption = "nosec"
+	// Audit global option which indicates that gosec runs in audit mode
+	Audit GlobalOption = "audit"
+)
+
 // Config is used to provide configuration and customization to each of the rules.
 type Config map[string]interface{}
 
@@ -22,7 +32,7 @@ type Config map[string]interface{}
 // or from a *os.File.
 func NewConfig() Config {
 	cfg := make(Config)
-	cfg[Globals] = make(map[string]string)
+	cfg[Globals] = make(map[GlobalOption]string)
 	return cfg
 }
 
@@ -65,9 +75,9 @@ func (c Config) Set(section string, value interface{}) {
 }
 
 // GetGlobal returns value associated with global configuration option
-func (c Config) GetGlobal(option string) (string, error) {
+func (c Config) GetGlobal(option GlobalOption) (string, error) {
 	if globals, ok := c[Globals]; ok {
-		if settings, ok := globals.(map[string]string); ok {
+		if settings, ok := globals.(map[GlobalOption]string); ok {
 			if value, ok := settings[option]; ok {
 				return value, nil
 			}
@@ -79,10 +89,19 @@ func (c Config) GetGlobal(option string) (string, error) {
 }
 
 // SetGlobal associates a value with a global configuration option
-func (c Config) SetGlobal(option, value string) {
+func (c Config) SetGlobal(option GlobalOption, value string) {
 	if globals, ok := c[Globals]; ok {
-		if settings, ok := globals.(map[string]string); ok {
+		if settings, ok := globals.(map[GlobalOption]string); ok {
 			settings[option] = value
 		}
 	}
 }
+
+// IsGlobalEnabled checks if a global option is enabled
+func (c Config) IsGlobalEnabled(option GlobalOption) (bool, error) {
+	value, err := c.GetGlobal(option)
+	if err != nil {
+		return false, err
+	}
+	return (value == "true" || value == "enabled"), nil
+}

config_test.go

@@ -81,23 +81,31 @@ var _ = Describe("Configuration", func() {
 		It("should have a default global section", func() {
 			settings, err := configuration.Get("global")
 			Expect(err).Should(BeNil())
-			expectedType := make(map[string]string)
+			expectedType := make(map[gosec.GlobalOption]string)
 			Expect(settings).Should(BeAssignableToTypeOf(expectedType))
 		})
 
 		It("should save global settings to correct section", func() {
-			configuration.SetGlobal("nosec", "enabled")
+			configuration.SetGlobal(gosec.Nosec, "enabled")
 			settings, err := configuration.Get("global")
 			Expect(err).Should(BeNil())
-			if globals, ok := settings.(map[string]string); ok {
+			if globals, ok := settings.(map[gosec.GlobalOption]string); ok {
 				Expect(globals["nosec"]).Should(MatchRegexp("enabled"))
 			} else {
 				Fail("globals are not defined as map")
 			}
 
-			setValue, err := configuration.GetGlobal("nosec")
+			setValue, err := configuration.GetGlobal(gosec.Nosec)
 			Expect(err).Should(BeNil())
 			Expect(setValue).Should(MatchRegexp("enabled"))
 		})
+
+		It("should find global settings which are enabled", func() {
+			configuration.SetGlobal(gosec.Nosec, "enabled")
+			enabled, err := configuration.IsGlobalEnabled(gosec.Nosec)
+			Expect(err).Should(BeNil())
+			Expect(enabled).Should(BeTrue())
+		})
 	})
+
 })

rules/errors.go

@@ -51,6 +51,21 @@ func returnsError(callExpr *ast.CallExpr, ctx *gosec.Context) int {
 
 func (r *noErrorCheck) Match(n ast.Node, ctx *gosec.Context) (*gosec.Issue, error) {
 	switch stmt := n.(type) {
+	case *ast.AssignStmt:
+		cfg := ctx.Config
+		if enabled, err := cfg.IsGlobalEnabled(gosec.Audit); err == nil && enabled {
+			for _, expr := range stmt.Rhs {
+				if callExpr, ok := expr.(*ast.CallExpr); ok && r.whitelist.ContainsCallExpr(expr, ctx, false) == nil {
+					pos := returnsError(callExpr, ctx)
+					if pos < 0 || pos >= len(stmt.Lhs) {
+						return nil, nil
+					}
+					if id, ok := stmt.Lhs[pos].(*ast.Ident); ok && id.Name == "_" {
+						return gosec.NewIssue(ctx, n, r.ID(), r.What, r.Severity, r.Confidence), nil
+					}
+				}
+			}
+		}
 	case *ast.ExprStmt:
 		if callExpr, ok := stmt.X.(*ast.CallExpr); ok && r.whitelist.ContainsCallExpr(stmt.X, ctx, false) == nil {
 			pos := returnsError(callExpr, ctx)

rules/rules_test.go

@@ -12,21 +12,29 @@ import (
 	"github.com/securego/gosec/testutils"
 )
 
+type option struct {
+	name  gosec.GlobalOption
+	value string
+}
+
 var _ = Describe("gosec rules", func() {
 
 	var (
 		logger    *log.Logger
 		config    gosec.Config
 		analyzer  *gosec.Analyzer
-		runner    func(string, []testutils.CodeSample)
+		runner    func(string, []testutils.CodeSample, ...option)
 		buildTags []string
 	)
 
 	BeforeEach(func() {
 		logger, _ = testutils.NewLogger()
 		config = gosec.NewConfig()
 		analyzer = gosec.NewAnalyzer(config, logger)
-		runner = func(rule string, samples []testutils.CodeSample) {
+		runner = func(rule string, samples []testutils.CodeSample, options ...option) {
+			for _, o := range options {
+				config.SetGlobal(o.name, o.value)
+			}
 			analyzer.LoadRules(rules.Generate(rules.NewRuleFilter(false, rule)).Builders())
 			for n, sample := range samples {
 				analyzer.Reset()
@@ -61,10 +69,14 @@ var _ = Describe("gosec rules", func() {
 			runner("G103", testutils.SampleCodeG103)
 		})
 
-		It("should errors not being checked", func() {
+		It("should detect errors not being checked", func() {
 			runner("G104", testutils.SampleCodeG104)
 		})
 
+		It("should detect errors not being checked in audit mode", func() {
+			runner("G104", testutils.SampleCodeG104Audit, option{name: gosec.Audit, value: "enabled"})
+		})
+
 		It("should detect of big.Exp function", func() {
 			runner("G105", testutils.SampleCodeG105)
 		})

testutils/source.go

@@ -231,6 +231,63 @@ package main
 func dummy(){}
 `}, 0}}
 
+	// SampleCodeG104Audit finds errors that aren't being handled in audit mode
+	SampleCodeG104Audit = []CodeSample{
+		{[]string{`
+package main
+import "fmt"
+func test() (int,error) {
+	return 0, nil
+}
+func main() {
+	v, _ := test()
+	fmt.Println(v)
+}`}, 1}, {[]string{`
+package main
+import (
+	"io/ioutil"
+	"os"
+	"fmt"
+)
+func a() error {
+	return fmt.Errorf("This is an error")
+}
+func b() {
+	fmt.Println("b")
+	ioutil.WriteFile("foo.txt", []byte("bar"), os.ModeExclusive)
+}
+func c() string {
+	return fmt.Sprintf("This isn't anything")
+}
+func main() {
+	_ = a()
+	a()
+	b()
+	c()
+}`}, 3}, {[]string{`
+package main
+import "fmt"
+func test() error {
+	return nil
+}
+func main() {
+	e := test()
+	fmt.Println(e)
+}`}, 0}, {[]string{`
+// +build go1.10
+
+package main
+import "strings"
+func main() {
+	var buf strings.Builder
+	_, err := buf.WriteString("test string")
+	if err != nil {
+		panic(err)
+	}
+}`, `
+package main
+func dummy(){}
+`}, 0}}
 	// SampleCodeG105 - bignum overflow
 	SampleCodeG105 = []CodeSample{{[]string{`
 package main