Skip to content

Commit

Permalink
Add support for suppressing the findings
Browse files Browse the repository at this point in the history
  • Loading branch information
Yiwei-Ding committed Dec 9, 2021
1 parent 040327f commit b45f95f
Show file tree
Hide file tree
Showing 15 changed files with 448 additions and 127 deletions.
28 changes: 27 additions & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -269,7 +269,8 @@ gosec -exclude-generated ./...
### Annotating code

As with all automated detection tools, there will be cases of false positives. In cases where gosec reports a failure that has been manually verified as being safe,
it is possible to annotate the code with a `#nosec` comment.
it is possible to annotate the code with a comment that starts with `#nosec`.
The `#nosec` comment should have the format `#nosec [RuleList] [-- Justification]`.

The annotation causes gosec to stop processing any further nodes within the
AST so can apply to a whole block or more granularly to a single expression.
Expand All @@ -294,6 +295,10 @@ When a specific false positive has been identified and verified as safe, you may
within a section of code, while continuing to scan for other problems. To do this, you can list the rule(s) to be suppressed within
the `#nosec` annotation, e.g: `/* #nosec G401 */` or `// #nosec G201 G202 G203`

You could put the description or justification text for the annotation. The
justification should be after the rule(s) to suppress and start with two or
more dashes, e.g: `// #nosec G101 G102 -- This is a false positive`

In some cases you may also want to revisit places where `#nosec` annotations
have been used. To run the scanner and ignore any `#nosec` annotations you
can do the following:
Expand All @@ -302,6 +307,27 @@ can do the following:
gosec -nosec=true ./...
```

### Tracking suppressions

As described above, we could suppress violations externally (using `-include`/
`-exclude`) or inline (using `#nosec` annotations) in gosec. This suppression
inflammation can be used to generate corresponding signals for auditing
purposes.

We could track suppressions by the `-track-suppressions` flag as follows:

```bash
gosec -track-suppressions -exclude=G101 -fmt=sarif -out=results.sarif ./...
```

- For external suppressions, gosec records suppression info where `kind` is
`external` and `justification` is a certain sentence "Globally suppressed".
- For inline suppressions, gosec records suppression info where `kind` is
`inSource` and `justification` is the text after two or more dashes in the
comment.

**Note:** Only SARIF and JSON formats support tracking suppressions.

### Build tags

gosec is able to pass your [Go build tags](https://golang.org/pkg/go/build/) to the analyzer.
Expand Down
132 changes: 85 additions & 47 deletions analyzer.go
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,10 @@ const LoadMode = packages.NeedName |
packages.NeedTypesInfo |
packages.NeedSyntax

const externalSuppressionJustification = "Globally suppressed."

const aliasOfAllRules = "*"

var generatedCodePattern = regexp.MustCompile(`^// Code generated .* DO NOT EDIT\.$`)

// The Context is populated with data parsed from the source code as it is scanned.
Expand All @@ -57,7 +61,7 @@ type Context struct {
Root *ast.File
Config Config
Imports *ImportTracker
Ignores []map[string]bool
Ignores []map[string][]SuppressionInfo
PassedValues map[string]interface{}
}

Expand All @@ -72,21 +76,29 @@ type Metrics struct {
// Analyzer object is the main object of gosec. It has methods traverse an AST
// and invoke the correct checking rules as on each node as required.
type Analyzer struct {
ignoreNosec bool
ruleset RuleSet
context *Context
config Config
logger *log.Logger
issues []*Issue
stats *Metrics
errors map[string][]Error // keys are file paths; values are the golang errors in those files
tests bool
excludeGenerated bool
showIgnored bool
ignoreNosec bool
ruleset RuleSet
context *Context
config Config
logger *log.Logger
issues []*Issue
stats *Metrics
errors map[string][]Error // keys are file paths; values are the golang errors in those files
tests bool
excludeGenerated bool
showIgnored bool
trackSuppressions bool
}

// SuppressionInfo object is to record the kind and the justification that used
// to suppress violations.
type SuppressionInfo struct {
Kind string `json:"kind"`
Justification string `json:"justification"`
}

// NewAnalyzer builds a new analyzer.
func NewAnalyzer(conf Config, tests bool, excludeGenerated bool, logger *log.Logger) *Analyzer {
func NewAnalyzer(conf Config, tests bool, excludeGenerated bool, trackSuppressions bool, logger *log.Logger) *Analyzer {
ignoreNoSec := false
if enabled, err := conf.IsGlobalEnabled(Nosec); err == nil {
ignoreNoSec = enabled
Expand All @@ -99,17 +111,18 @@ func NewAnalyzer(conf Config, tests bool, excludeGenerated bool, logger *log.Log
logger = log.New(os.Stderr, "[gosec]", log.LstdFlags)
}
return &Analyzer{
ignoreNosec: ignoreNoSec,
showIgnored: showIgnored,
ruleset: make(RuleSet),
context: &Context{},
config: conf,
logger: logger,
issues: make([]*Issue, 0, 16),
stats: &Metrics{},
errors: make(map[string][]Error),
tests: tests,
excludeGenerated: excludeGenerated,
ignoreNosec: ignoreNoSec,
showIgnored: showIgnored,
ruleset: NewRuleSet(),
context: &Context{},
config: conf,
logger: logger,
issues: make([]*Issue, 0, 16),
stats: &Metrics{},
errors: make(map[string][]Error),
tests: tests,
excludeGenerated: excludeGenerated,
trackSuppressions: trackSuppressions,
}
}

Expand All @@ -125,10 +138,10 @@ func (gosec *Analyzer) Config() Config {

// LoadRules instantiates all the rules to be used when analyzing source
// packages
func (gosec *Analyzer) LoadRules(ruleDefinitions map[string]RuleBuilder) {
func (gosec *Analyzer) LoadRules(ruleDefinitions map[string]RuleBuilder, ruleSuppressed map[string]bool) {
for id, def := range ruleDefinitions {
r, nodes := def(id, gosec.config)
gosec.ruleset.Register(r, nodes...)
gosec.ruleset.Register(r, ruleSuppressed[id], nodes...)
}
}

Expand Down Expand Up @@ -295,7 +308,7 @@ func (gosec *Analyzer) AppendError(file string, err error) {
}

// ignore a node (and sub-tree) if it is tagged with a nosec tag comment
func (gosec *Analyzer) ignore(n ast.Node) ([]string, bool) {
func (gosec *Analyzer) ignore(n ast.Node) map[string]SuppressionInfo {
if groups, ok := gosec.context.Comments[n]; ok && !gosec.ignoreNosec {

// Checks if an alternative for #nosec is set and, if not, uses the default.
Expand All @@ -307,31 +320,44 @@ func (gosec *Analyzer) ignore(n ast.Node) ([]string, bool) {

for _, group := range groups {

foundDefaultTag := strings.Contains(group.Text(), noSecDefaultTag)
foundAlternativeTag := strings.Contains(group.Text(), noSecAlternativeTag)
foundDefaultTag := strings.HasPrefix(group.Text(), noSecDefaultTag)
foundAlternativeTag := strings.HasPrefix(group.Text(), noSecAlternativeTag)

if foundDefaultTag || foundAlternativeTag {
gosec.stats.NumNosec++

// Extract the directive and the justification.
justification := ""
commentParts := regexp.MustCompile(`-{2,}`).Split(group.Text(), 2)
directive := commentParts[0]
if len(commentParts) > 1 {
justification = strings.TrimSpace(strings.TrimRight(commentParts[1], "\n"))
}

// Pull out the specific rules that are listed to be ignored.
re := regexp.MustCompile(`(G\d{3})`)
matches := re.FindAllStringSubmatch(group.Text(), -1)
matches := re.FindAllStringSubmatch(directive, -1)

// If no specific rules were given, ignore everything.
if len(matches) == 0 {
return nil, true
suppression := SuppressionInfo{
Kind: "inSource",
Justification: justification,
}

// Find the rule IDs to ignore.
var ignores []string
ignores := make(map[string]SuppressionInfo)
for _, v := range matches {
ignores = append(ignores, v[1])
ignores[v[1]] = suppression
}

// If no specific rules were given, ignore everything.
if len(matches) == 0 {
ignores[aliasOfAllRules] = suppression
}
return ignores, false
return ignores
}
}
}
return nil, false
return nil
}

// Visit runs the gosec visitor logic over an AST created by parsing go code.
Expand All @@ -346,31 +372,40 @@ func (gosec *Analyzer) Visit(n ast.Node) ast.Visitor {
}

// Get any new rule exclusions.
ignoredRules, ignoreAll := gosec.ignore(n)
if ignoreAll {
return nil
}
ignoredRules := gosec.ignore(n)

// Now create the union of exclusions.
ignores := map[string]bool{}
ignores := map[string][]SuppressionInfo{}
if len(gosec.context.Ignores) > 0 {
for k, v := range gosec.context.Ignores[0] {
ignores[k] = v
}
}

for _, v := range ignoredRules {
ignores[v] = true
for ruleID, suppression := range ignoredRules {
ignores[ruleID] = append(ignores[ruleID], suppression)
}

// Push the new set onto the stack.
gosec.context.Ignores = append([]map[string]bool{ignores}, gosec.context.Ignores...)
gosec.context.Ignores = append([]map[string][]SuppressionInfo{ignores}, gosec.context.Ignores...)

// Track aliased and initialization imports
gosec.context.Imports.TrackImport(n)

for _, rule := range gosec.ruleset.RegisteredFor(n) {
_, ignored := ignores[rule.ID()]
// Check if all rules are ignored.
suppressions, ignored := ignores[aliasOfAllRules]
if !ignored {
suppressions, ignored = ignores[rule.ID()]
}
// Track external suppressions.
if gosec.ruleset.IsRuleSuppressed(rule.ID()) {
ignored = true
suppressions = append(suppressions, SuppressionInfo{
Kind: "external",
Justification: externalSuppressionJustification,
})
}

issue, err := rule.Match(n, gosec.context)
if err != nil {
Expand All @@ -385,7 +420,10 @@ func (gosec *Analyzer) Visit(n ast.Node) ast.Visitor {
if !ignored || !gosec.showIgnored {
gosec.stats.NumFound++
}
if !ignored || gosec.showIgnored || gosec.ignoreNosec {
if ignored && gosec.trackSuppressions {
issue.WithSuppressions(suppressions)
gosec.issues = append(gosec.issues, issue)
} else if !ignored || gosec.showIgnored || gosec.ignoreNosec {
gosec.issues = append(gosec.issues, issue)
}
}
Expand Down

0 comments on commit b45f95f

Please sign in to comment.