Skip to content

Commit

Permalink
Detect the unhandled errors even though they are explicitly ignored i…
Browse files Browse the repository at this point in the history
…f 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
  • Loading branch information
Cosmin Cojocar authored and gcmurphy committed Jan 14, 2019
1 parent 14ed63d commit f87af5f
Show file tree
Hide file tree
Showing 8 changed files with 128 additions and 17 deletions.
6 changes: 3 additions & 3 deletions analyzer.go
Expand Up @@ -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
}
Expand Down Expand Up @@ -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)
Expand Down
2 changes: 1 addition & 1 deletion analyzer_test.go
Expand Up @@ -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())

Expand Down
2 changes: 1 addition & 1 deletion cmd/gosec/main.go
Expand Up @@ -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
}
Expand Down
29 changes: 24 additions & 5 deletions config.go
Expand Up @@ -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{}

Expand All @@ -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
}

Expand Down Expand Up @@ -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
}
Expand All @@ -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
}
16 changes: 12 additions & 4 deletions config_test.go
Expand Up @@ -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())
})
})

})
15 changes: 15 additions & 0 deletions rules/errors.go
Expand Up @@ -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)
Expand Down
18 changes: 15 additions & 3 deletions rules/rules_test.go
Expand Up @@ -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()
Expand Down Expand Up @@ -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)
})
Expand Down
57 changes: 57 additions & 0 deletions testutils/source.go
Expand Up @@ -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
Expand Down

0 comments on commit f87af5f

Please sign in to comment.