diff --git a/README.md b/README.md index 59323d5..ffaa027 100644 --- a/README.md +++ b/README.md @@ -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. @@ -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: @@ -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. diff --git a/analyzer.go b/analyzer.go index 3809bdc..0efb9a9 100644 --- a/analyzer.go +++ b/analyzer.go @@ -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. @@ -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{} } @@ -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 @@ -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, } } @@ -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...) } } @@ -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. @@ -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 } - return ignores, false + + // If no specific rules were given, ignore everything. + if len(matches) == 0 { + ignores[aliasOfAllRules] = suppression + } + return ignores } } } - return nil, false + return nil } // Visit runs the gosec visitor logic over an AST created by parsing go code. @@ -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 { @@ -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) } } diff --git a/analyzer_test.go b/analyzer_test.go index 2f42fe4..21e9af4 100644 --- a/analyzer_test.go +++ b/analyzer_test.go @@ -24,12 +24,12 @@ var _ = Describe("Analyzer", func() { ) BeforeEach(func() { logger, _ = testutils.NewLogger() - analyzer = gosec.NewAnalyzer(nil, tests, false, logger) + analyzer = gosec.NewAnalyzer(nil, tests, false, false, logger) }) Context("when processing a package", func() { It("should not report an error if the package contains no Go files", func() { - analyzer.LoadRules(rules.Generate().Builders()) + analyzer.LoadRules(rules.Generate(false).RulesInfo()) dir, err := ioutil.TempDir("", "empty") defer os.RemoveAll(dir) Expect(err).ShouldNot(HaveOccurred()) @@ -40,7 +40,7 @@ var _ = Describe("Analyzer", func() { }) It("should report an error if the package fails to build", func() { - analyzer.LoadRules(rules.Generate().Builders()) + analyzer.LoadRules(rules.Generate(false).RulesInfo()) pkg := testutils.NewTestPackage() defer pkg.Close() pkg.AddFile("wonky.go", `func main(){ println("forgot the package")}`) @@ -56,7 +56,7 @@ var _ = Describe("Analyzer", func() { }) It("should be able to analyze multiple Go files", func() { - analyzer.LoadRules(rules.Generate().Builders()) + analyzer.LoadRules(rules.Generate(false).RulesInfo()) pkg := testutils.NewTestPackage() defer pkg.Close() pkg.AddFile("foo.go", ` @@ -78,7 +78,7 @@ var _ = Describe("Analyzer", func() { }) It("should be able to analyze multiple Go packages", func() { - analyzer.LoadRules(rules.Generate().Builders()) + analyzer.LoadRules(rules.Generate(false).RulesInfo()) pkg1 := testutils.NewTestPackage() pkg2 := testutils.NewTestPackage() defer pkg1.Close() @@ -104,7 +104,7 @@ var _ = Describe("Analyzer", func() { It("should find errors when nosec is not in use", func() { sample := testutils.SampleCodeG401[0] source := sample.Code[0] - analyzer.LoadRules(rules.Generate(rules.NewRuleFilter(false, "G401")).Builders()) + analyzer.LoadRules(rules.Generate(false, rules.NewRuleFilter(false, "G401")).RulesInfo()) controlPackage := testutils.NewTestPackage() defer controlPackage.Close() @@ -118,7 +118,7 @@ var _ = Describe("Analyzer", func() { }) It("should report Go build errors and invalid files", func() { - analyzer.LoadRules(rules.Generate().Builders()) + analyzer.LoadRules(rules.Generate(false).RulesInfo()) pkg := testutils.NewTestPackage() defer pkg.Close() pkg.AddFile("foo.go", ` @@ -142,7 +142,7 @@ var _ = Describe("Analyzer", func() { It("should not report errors when a nosec comment is present", func() { sample := testutils.SampleCodeG401[0] source := sample.Code[0] - analyzer.LoadRules(rules.Generate(rules.NewRuleFilter(false, "G401")).Builders()) + analyzer.LoadRules(rules.Generate(false, rules.NewRuleFilter(false, "G401")).RulesInfo()) nosecPackage := testutils.NewTestPackage() defer nosecPackage.Close() @@ -160,7 +160,7 @@ var _ = Describe("Analyzer", func() { // Rule for MD5 weak crypto usage sample := testutils.SampleCodeG401[0] source := sample.Code[0] - analyzer.LoadRules(rules.Generate(rules.NewRuleFilter(false, "G401")).Builders()) + analyzer.LoadRules(rules.Generate(false, rules.NewRuleFilter(false, "G401")).RulesInfo()) nosecPackage := testutils.NewTestPackage() defer nosecPackage.Close() @@ -177,7 +177,7 @@ var _ = Describe("Analyzer", func() { It("should report errors when an exclude comment is present for a different rule", func() { sample := testutils.SampleCodeG401[0] source := sample.Code[0] - analyzer.LoadRules(rules.Generate(rules.NewRuleFilter(false, "G401")).Builders()) + analyzer.LoadRules(rules.Generate(false, rules.NewRuleFilter(false, "G401")).RulesInfo()) nosecPackage := testutils.NewTestPackage() defer nosecPackage.Close() @@ -194,7 +194,7 @@ var _ = Describe("Analyzer", func() { It("should not report errors when an exclude comment is present for multiple rules, including the correct rule", func() { sample := testutils.SampleCodeG401[0] source := sample.Code[0] - analyzer.LoadRules(rules.Generate(rules.NewRuleFilter(false, "G401")).Builders()) + analyzer.LoadRules(rules.Generate(false, rules.NewRuleFilter(false, "G401")).RulesInfo()) nosecPackage := testutils.NewTestPackage() defer nosecPackage.Close() @@ -213,7 +213,7 @@ var _ = Describe("Analyzer", func() { It("should pass the build tags", func() { sample := testutils.SampleCodeBuildTag[0] source := sample.Code[0] - analyzer.LoadRules(rules.Generate().Builders()) + analyzer.LoadRules(rules.Generate(false).RulesInfo()) pkg := testutils.NewTestPackage() defer pkg.Close() pkg.AddFile("tags.go", source) @@ -223,7 +223,7 @@ var _ = Describe("Analyzer", func() { }) It("should process an empty package with test file", func() { - analyzer.LoadRules(rules.Generate().Builders()) + analyzer.LoadRules(rules.Generate(false).RulesInfo()) pkg := testutils.NewTestPackage() defer pkg.Close() pkg.AddFile("foo_test.go", ` @@ -245,8 +245,8 @@ var _ = Describe("Analyzer", func() { // overwrite nosec option nosecIgnoreConfig := gosec.NewConfig() nosecIgnoreConfig.SetGlobal(gosec.Nosec, "true") - customAnalyzer := gosec.NewAnalyzer(nosecIgnoreConfig, tests, false, logger) - customAnalyzer.LoadRules(rules.Generate(rules.NewRuleFilter(false, "G401")).Builders()) + customAnalyzer := gosec.NewAnalyzer(nosecIgnoreConfig, tests, false, false, logger) + customAnalyzer.LoadRules(rules.Generate(false, rules.NewRuleFilter(false, "G401")).RulesInfo()) nosecPackage := testutils.NewTestPackage() defer nosecPackage.Close() @@ -269,8 +269,8 @@ var _ = Describe("Analyzer", func() { nosecIgnoreConfig := gosec.NewConfig() nosecIgnoreConfig.SetGlobal(gosec.Nosec, "true") nosecIgnoreConfig.SetGlobal(gosec.ShowIgnored, "true") - customAnalyzer := gosec.NewAnalyzer(nosecIgnoreConfig, tests, false, logger) - customAnalyzer.LoadRules(rules.Generate(rules.NewRuleFilter(false, "G401")).Builders()) + customAnalyzer := gosec.NewAnalyzer(nosecIgnoreConfig, tests, false, false, logger) + customAnalyzer.LoadRules(rules.Generate(false, rules.NewRuleFilter(false, "G401")).RulesInfo()) nosecPackage := testutils.NewTestPackage() defer nosecPackage.Close() @@ -294,8 +294,8 @@ var _ = Describe("Analyzer", func() { // overwrite nosec option nosecIgnoreConfig := gosec.NewConfig() nosecIgnoreConfig.SetGlobal(gosec.NoSecAlternative, "#falsePositive") - customAnalyzer := gosec.NewAnalyzer(nosecIgnoreConfig, tests, false, logger) - customAnalyzer.LoadRules(rules.Generate(rules.NewRuleFilter(false, "G401")).Builders()) + customAnalyzer := gosec.NewAnalyzer(nosecIgnoreConfig, tests, false, false, logger) + customAnalyzer.LoadRules(rules.Generate(false, rules.NewRuleFilter(false, "G401")).RulesInfo()) nosecPackage := testutils.NewTestPackage() defer nosecPackage.Close() @@ -317,8 +317,8 @@ var _ = Describe("Analyzer", func() { // overwrite nosec option nosecIgnoreConfig := gosec.NewConfig() nosecIgnoreConfig.SetGlobal(gosec.NoSecAlternative, "#falsePositive") - customAnalyzer := gosec.NewAnalyzer(nosecIgnoreConfig, tests, false, logger) - customAnalyzer.LoadRules(rules.Generate(rules.NewRuleFilter(false, "G401")).Builders()) + customAnalyzer := gosec.NewAnalyzer(nosecIgnoreConfig, tests, false, false, logger) + customAnalyzer.LoadRules(rules.Generate(false, rules.NewRuleFilter(false, "G401")).RulesInfo()) nosecPackage := testutils.NewTestPackage() defer nosecPackage.Close() @@ -333,8 +333,8 @@ var _ = Describe("Analyzer", func() { }) It("should be able to analyze Go test package", func() { - customAnalyzer := gosec.NewAnalyzer(nil, true, false, logger) - customAnalyzer.LoadRules(rules.Generate().Builders()) + customAnalyzer := gosec.NewAnalyzer(nil, true, false, false, logger) + customAnalyzer.LoadRules(rules.Generate(false).RulesInfo()) pkg := testutils.NewTestPackage() defer pkg.Close() pkg.AddFile("foo.go", ` @@ -358,8 +358,8 @@ var _ = Describe("Analyzer", func() { Expect(issues).Should(HaveLen(1)) }) It("should be able to scan generated files if NOT excluded", func() { - customAnalyzer := gosec.NewAnalyzer(nil, true, false, logger) - customAnalyzer.LoadRules(rules.Generate().Builders()) + customAnalyzer := gosec.NewAnalyzer(nil, true, false, false, logger) + customAnalyzer.LoadRules(rules.Generate(false).RulesInfo()) pkg := testutils.NewTestPackage() defer pkg.Close() pkg.AddFile("foo.go", ` @@ -379,8 +379,8 @@ var _ = Describe("Analyzer", func() { Expect(issues).Should(HaveLen(1)) }) It("should be able to skip generated files if excluded", func() { - customAnalyzer := gosec.NewAnalyzer(nil, true, true, logger) - customAnalyzer.LoadRules(rules.Generate().Builders()) + customAnalyzer := gosec.NewAnalyzer(nil, true, true, false, logger) + customAnalyzer.LoadRules(rules.Generate(false).RulesInfo()) pkg := testutils.NewTestPackage() defer pkg.Close() pkg.AddFile("foo.go", ` @@ -401,7 +401,7 @@ var _ = Describe("Analyzer", func() { }) }) It("should be able to analyze Cgo files", func() { - analyzer.LoadRules(rules.Generate().Builders()) + analyzer.LoadRules(rules.Generate(false).RulesInfo()) sample := testutils.SampleCodeCgo[0] source := sample.Code[0] @@ -583,4 +583,106 @@ var _ = Describe("Analyzer", func() { } }) }) + + Context("when tracking suppressions", func() { + BeforeEach(func() { + analyzer = gosec.NewAnalyzer(nil, tests, false, true, logger) + }) + + It("should not report an error if the violation is suppressed", func() { + sample := testutils.SampleCodeG401[0] + source := sample.Code[0] + analyzer.LoadRules(rules.Generate(false, rules.NewRuleFilter(false, "G401")).RulesInfo()) + + nosecPackage := testutils.NewTestPackage() + defer nosecPackage.Close() + nosecSource := strings.Replace(source, "h := md5.New()", "h := md5.New() // #nosec G401 -- Justification", 1) + nosecPackage.AddFile("md5.go", nosecSource) + err := nosecPackage.Build() + Expect(err).ShouldNot(HaveOccurred()) + err = analyzer.Process(buildTags, nosecPackage.Path) + Expect(err).ShouldNot(HaveOccurred()) + issues, _, _ := analyzer.Report() + Expect(issues).To(HaveLen(1)) + Expect(issues[0].Suppressions).To(HaveLen(1)) + Expect(issues[0].Suppressions[0].Kind).To(Equal("inSource")) + Expect(issues[0].Suppressions[0].Justification).To(Equal("Justification")) + }) + + It("should not report an error if the violation is suppressed without certain rules", func() { + sample := testutils.SampleCodeG401[0] + source := sample.Code[0] + analyzer.LoadRules(rules.Generate(false, rules.NewRuleFilter(false, "G401")).RulesInfo()) + + nosecPackage := testutils.NewTestPackage() + defer nosecPackage.Close() + nosecSource := strings.Replace(source, "h := md5.New()", "h := md5.New() // #nosec", 1) + nosecPackage.AddFile("md5.go", nosecSource) + err := nosecPackage.Build() + Expect(err).ShouldNot(HaveOccurred()) + err = analyzer.Process(buildTags, nosecPackage.Path) + Expect(err).ShouldNot(HaveOccurred()) + issues, _, _ := analyzer.Report() + Expect(issues).To(HaveLen(1)) + Expect(issues[0].Suppressions).To(HaveLen(1)) + Expect(issues[0].Suppressions[0].Kind).To(Equal("inSource")) + Expect(issues[0].Suppressions[0].Justification).To(Equal("")) + }) + + It("should not report an error if the rule is not included", func() { + sample := testutils.SampleCodeG101[0] + source := sample.Code[0] + analyzer.LoadRules(rules.Generate(true, rules.NewRuleFilter(false, "G401")).RulesInfo()) + + controlPackage := testutils.NewTestPackage() + defer controlPackage.Close() + controlPackage.AddFile("pwd.go", source) + err := controlPackage.Build() + Expect(err).ShouldNot(HaveOccurred()) + err = analyzer.Process(buildTags, controlPackage.Path) + Expect(err).ShouldNot(HaveOccurred()) + controlIssues, _, _ := analyzer.Report() + Expect(controlIssues).Should(HaveLen(sample.Errors)) + Expect(controlIssues[0].Suppressions).To(HaveLen(1)) + Expect(controlIssues[0].Suppressions[0].Kind).To(Equal("external")) + Expect(controlIssues[0].Suppressions[0].Justification).To(Equal("Globally suppressed.")) + }) + + It("should not report an error if the rule is excluded", func() { + sample := testutils.SampleCodeG101[0] + source := sample.Code[0] + analyzer.LoadRules(rules.Generate(true, rules.NewRuleFilter(true, "G101")).RulesInfo()) + + controlPackage := testutils.NewTestPackage() + defer controlPackage.Close() + controlPackage.AddFile("pwd.go", source) + err := controlPackage.Build() + Expect(err).ShouldNot(HaveOccurred()) + err = analyzer.Process(buildTags, controlPackage.Path) + Expect(err).ShouldNot(HaveOccurred()) + issues, _, _ := analyzer.Report() + Expect(issues).Should(HaveLen(sample.Errors)) + Expect(issues[0].Suppressions).To(HaveLen(1)) + Expect(issues[0].Suppressions[0].Kind).To(Equal("external")) + Expect(issues[0].Suppressions[0].Justification).To(Equal("Globally suppressed.")) + }) + + It("should track multiple suppressions if the violation is multiply suppressed", func() { + sample := testutils.SampleCodeG101[0] + source := sample.Code[0] + analyzer.LoadRules(rules.Generate(true, rules.NewRuleFilter(true, "G101")).RulesInfo()) + + nosecPackage := testutils.NewTestPackage() + defer nosecPackage.Close() + nosecSource := strings.Replace(source, "}", "} // #nosec G101 -- Justification", 1) + nosecPackage.AddFile("pwd.go", nosecSource) + err := nosecPackage.Build() + Expect(err).ShouldNot(HaveOccurred()) + err = analyzer.Process(buildTags, nosecPackage.Path) + Expect(err).ShouldNot(HaveOccurred()) + issues, _, _ := analyzer.Report() + Expect(issues).Should(HaveLen(sample.Errors)) + Expect(issues[0].Suppressions).To(HaveLen(2)) + }) + }) }) diff --git a/cmd/gosec/main.go b/cmd/gosec/main.go index c007e08..844f41f 100644 --- a/cmd/gosec/main.go +++ b/cmd/gosec/main.go @@ -132,6 +132,9 @@ var ( // overrides the output format when stdout the results while saving them in the output file flagVerbose = flag.String("verbose", "", "Overrides the output format when stdout the results while saving them in the output file.\nValid options are: json, yaml, csv, junit-xml, html, sonarqube, golint, sarif or text") + // output suppression information for auditing purposes + flagTrackSuppressions = flag.Bool("track-suppressions", false, "Output suppression information, including its kind and justification") + // exlude the folders from scan flagDirsExclude arrayFlags @@ -147,14 +150,14 @@ func usage() { fmt.Fprint(os.Stderr, "\n\nRULES:\n\n") // sorted rule list for ease of reading - rl := rules.Generate() - keys := make([]string, 0, len(rl)) - for key := range rl { + rl := rules.Generate(*flagTrackSuppressions) + keys := make([]string, 0, len(rl.Rules)) + for key := range rl.Rules { keys = append(keys, key) } sort.Strings(keys) for _, k := range keys { - v := rl[k] + v := rl.Rules[k] fmt.Fprintf(os.Stderr, "\t%s: %s\n", k, v.Description) } fmt.Fprint(os.Stderr, "\n") @@ -202,7 +205,7 @@ func loadRules(include, exclude string) rules.RuleList { } else { logger.Println("Excluding rules: default") } - return rules.Generate(filters...) + return rules.Generate(*flagTrackSuppressions, filters...) } func getRootPaths(paths []string) []string { @@ -267,7 +270,7 @@ func filterIssues(issues []*gosec.Issue, severity gosec.Score, confidence gosec. for _, issue := range issues { if issue.Severity >= severity && issue.Confidence >= confidence { result = append(result, issue) - if !issue.NoSec || !*flagShowIgnored { + if (!issue.NoSec || !*flagShowIgnored) && len(issue.Suppressions) == 0 { trueIssues++ } } @@ -345,14 +348,14 @@ func main() { } // Load enabled rule definitions - ruleDefinitions := loadRules(*flagRulesInclude, flagRulesExclude.String()) - if len(ruleDefinitions) == 0 { + ruleList := loadRules(*flagRulesInclude, flagRulesExclude.String()) + if len(ruleList.Rules) == 0 { logger.Fatal("No rules are configured") } // Create the analyzer - analyzer := gosec.NewAnalyzer(config, *flagScanTests, *flagExcludeGenerated, logger) - analyzer.LoadRules(ruleDefinitions.Builders()) + analyzer := gosec.NewAnalyzer(config, *flagScanTests, *flagExcludeGenerated, *flagTrackSuppressions, logger) + analyzer.LoadRules(ruleList.RulesInfo()) excludedDirs := gosec.ExcludedDirsRegExp(flagDirsExclude) var packages []string diff --git a/cmd/gosecutil/tools.go b/cmd/gosecutil/tools.go index 11d36c6..2816323 100644 --- a/cmd/gosecutil/tools.go +++ b/cmd/gosecutil/tools.go @@ -104,7 +104,7 @@ func dumpAst(files ...string) { continue } - // Print the AST. #nosec + // #nosec -- Print the AST. ast.Print(fset, f) } } diff --git a/issue.go b/issue.go index 00b1937..cdc748e 100644 --- a/issue.go +++ b/issue.go @@ -88,16 +88,17 @@ var ruleToCWE = map[string]string{ // Issue is returned by a gosec rule if it discovers an issue with the scanned code. type Issue struct { - Severity Score `json:"severity"` // issue severity (how problematic it is) - Confidence Score `json:"confidence"` // issue confidence (how sure we are we found it) - Cwe *cwe.Weakness `json:"cwe"` // Cwe associated with RuleID - RuleID string `json:"rule_id"` // Human readable explanation - What string `json:"details"` // Human readable explanation - File string `json:"file"` // File name we found it in - Code string `json:"code"` // Impacted code line - Line string `json:"line"` // Line number in file - Col string `json:"column"` // Column number in line - NoSec bool `json:"nosec"` // true if the issue is nosec + Severity Score `json:"severity"` // issue severity (how problematic it is) + Confidence Score `json:"confidence"` // issue confidence (how sure we are we found it) + Cwe *cwe.Weakness `json:"cwe"` // Cwe associated with RuleID + RuleID string `json:"rule_id"` // Human readable explanation + What string `json:"details"` // Human readable explanation + File string `json:"file"` // File name we found it in + Code string `json:"code"` // Impacted code line + Line string `json:"line"` // Line number in file + Col string `json:"column"` // Column number in line + NoSec bool `json:"nosec"` // true if the issue is nosec + Suppressions []SuppressionInfo `json:"suppressions"` // Suppression info of the issue } // FileLocation point out the file path and line number in file @@ -200,3 +201,9 @@ func NewIssue(ctx *Context, node ast.Node, ruleID, desc string, severity Score, Cwe: GetCweByRule(ruleID), } } + +// WithSuppressions set the suppressions of the issue +func (i *Issue) WithSuppressions(suppressions []SuppressionInfo) *Issue { + i.Suppressions = suppressions + return i +} diff --git a/report/formatter.go b/report/formatter.go index e3ea734..14c90b9 100644 --- a/report/formatter.go +++ b/report/formatter.go @@ -53,6 +53,9 @@ const ( // the specified format. The formats currently accepted are: json, yaml, csv, junit-xml, html, sonarqube, golint and text. func CreateReport(w io.Writer, format string, enableColor bool, rootPaths []string, data *gosec.ReportInfo) error { var err error + if format != "json" && format != "sarif" { + data.Issues = filterOutSuppressedIssues(data.Issues) + } switch format { case "json": err = json.WriteReport(w, data) @@ -77,3 +80,13 @@ func CreateReport(w io.Writer, format string, enableColor bool, rootPaths []stri } return err } + +func filterOutSuppressedIssues(issues []*gosec.Issue) []*gosec.Issue { + nonSuppressedIssues := []*gosec.Issue{} + for _, issue := range issues { + if len(issue.Suppressions) == 0 { + nonSuppressedIssues = append(nonSuppressedIssues, issue) + } + } + return nonSuppressedIssues +} diff --git a/report/formatter_test.go b/report/formatter_test.go index ee37405..3fed7f2 100644 --- a/report/formatter_test.go +++ b/report/formatter_test.go @@ -486,4 +486,53 @@ var _ = Describe("Formatter", func() { } }) }) + + Context("When converting suppressed issues", func() { + ruleID := "G101" + cwe := gosec.GetCweByRule(ruleID) + suppressions := []gosec.SuppressionInfo{ + { + Kind: "kind", + Justification: "justification", + }, + } + suppressedIssue := createIssue(ruleID, cwe) + suppressedIssue.WithSuppressions(suppressions) + + It("text formatted report should contain the suppressed issues", func() { + error := map[string][]gosec.Error{} + reportInfo := gosec.NewReportInfo([]*gosec.Issue{&suppressedIssue}, &gosec.Metrics{}, error) + + buf := new(bytes.Buffer) + err := CreateReport(buf, "text", false, []string{}, reportInfo) + Expect(err).ShouldNot(HaveOccurred()) + + result := stripString(buf.String()) + Expect(result).To(ContainSubstring("Results:Summary")) + }) + + It("sarif formatted report should contain the suppressed issues", func() { + error := map[string][]gosec.Error{} + reportInfo := gosec.NewReportInfo([]*gosec.Issue{&suppressedIssue}, &gosec.Metrics{}, error) + + buf := new(bytes.Buffer) + err := CreateReport(buf, "sarif", false, []string{}, reportInfo) + Expect(err).ShouldNot(HaveOccurred()) + + result := stripString(buf.String()) + Expect(result).To(ContainSubstring(`"results":[{`)) + }) + + It("json formatted report should contain the suppressed issues", func() { + error := map[string][]gosec.Error{} + reportInfo := gosec.NewReportInfo([]*gosec.Issue{&suppressedIssue}, &gosec.Metrics{}, error) + + buf := new(bytes.Buffer) + err := CreateReport(buf, "json", false, []string{}, reportInfo) + Expect(err).ShouldNot(HaveOccurred()) + + result := stripString(buf.String()) + Expect(result).To(ContainSubstring(`"Issues":[{`)) + }) + }) }) diff --git a/report/sarif/builder.go b/report/sarif/builder.go index 8845d7f..7a99135 100644 --- a/report/sarif/builder.go +++ b/report/sarif/builder.go @@ -79,12 +79,13 @@ func NewTool(driver *ToolComponent) *Tool { } // NewResult instantiate a Result -func NewResult(ruleID string, ruleIndex int, level Level, message string) *Result { +func NewResult(ruleID string, ruleIndex int, level Level, message string, suppressions []*Suppression) *Result { return &Result{ - RuleID: ruleID, - RuleIndex: ruleIndex, - Level: level, - Message: NewMessage(message), + RuleID: ruleID, + RuleIndex: ruleIndex, + Level: level, + Message: NewMessage(message), + Suppressions: suppressions, } } @@ -199,3 +200,11 @@ func NewToolComponentReference(name string) *ToolComponentReference { GUID: uuid3(name), } } + +// NewSuppression instantiate a Suppression +func NewSuppression(kind string, justification string) *Suppression { + return &Suppression{ + Kind: kind, + Justification: justification, + } +} diff --git a/report/sarif/formatter.go b/report/sarif/formatter.go index f23230b..d2b6e56 100644 --- a/report/sarif/formatter.go +++ b/report/sarif/formatter.go @@ -48,7 +48,7 @@ func GenerateReport(rootPaths []string, data *gosec.ReportInfo) (*Report, error) return nil, err } - result := NewResult(r.rule.ID, r.index, getSarifLevel(issue.Severity.String()), issue.What). + result := NewResult(r.rule.ID, r.index, getSarifLevel(issue.Severity.String()), issue.What, buildSarifSuppressions(issue.Suppressions)). WithLocations(location) results = append(results, result) @@ -199,3 +199,11 @@ func getSarifLevel(s string) Level { return Note } } + +func buildSarifSuppressions(suppressions []gosec.SuppressionInfo) []*Suppression { + var sarifSuppressionList []*Suppression + for _, s := range suppressions { + sarifSuppressionList = append(sarifSuppressionList, NewSuppression(s.Kind, s.Justification)) + } + return sarifSuppressionList +} diff --git a/report/sarif/sarif_test.go b/report/sarif/sarif_test.go index e227941..ed35d49 100644 --- a/report/sarif/sarif_test.go +++ b/report/sarif/sarif_test.go @@ -2,6 +2,7 @@ package sarif_test import ( "bytes" + "regexp" . "github.com/onsi/ginkgo" . "github.com/onsi/gomega" @@ -21,5 +22,39 @@ var _ = Describe("Sarif Formatter", func() { Expect(err).ShouldNot(HaveOccurred()) Expect(result).To(ContainSubstring("\"results\": [")) }) + + It("sarif formatted report should contain the suppressed results", func() { + ruleID := "G101" + cwe := gosec.GetCweByRule(ruleID) + suppressedIssue := gosec.Issue{ + File: "/home/src/project/test.go", + Line: "1", + Col: "1", + RuleID: ruleID, + What: "test", + Confidence: gosec.High, + Severity: gosec.High, + Code: "1: testcode", + Cwe: cwe, + Suppressions: []gosec.SuppressionInfo{ + { + Kind: "kind", + Justification: "justification", + }, + }, + } + + reportInfo := gosec.NewReportInfo([]*gosec.Issue{&suppressedIssue}, &gosec.Metrics{}, map[string][]gosec.Error{}).WithVersion("v2.7.0") + buf := new(bytes.Buffer) + err := sarif.WriteReport(buf, reportInfo, []string{}) + result := buf.String() + Expect(err).ShouldNot(HaveOccurred()) + + hasResults, _ := regexp.MatchString(`"results": \[(\s*){`, result) + Expect(hasResults).To(BeTrue()) + + hasSuppressions, _ := regexp.MatchString(`"suppressions": \[(\s*){`, result) + Expect(hasSuppressions).To(BeTrue()) + }) }) }) diff --git a/rule.go b/rule.go index fbba089..c0429c4 100644 --- a/rule.go +++ b/rule.go @@ -26,34 +26,45 @@ type Rule interface { // RuleBuilder is used to register a rule definition with the analyzer type RuleBuilder func(id string, c Config) (Rule, []ast.Node) -// A RuleSet maps lists of rules to the type of AST node they should be run on. +// A RuleSet contains a mapping of lists of rules to the type of AST node they +// should be run on and a mapping of rule ID's to whether the rule are +// suppressed. // The analyzer will only invoke rules contained in the list associated with the // type of AST node it is currently visiting. -type RuleSet map[reflect.Type][]Rule +type RuleSet struct { + Rules map[reflect.Type][]Rule + RuleSuppressedMap map[string]bool +} // NewRuleSet constructs a new RuleSet func NewRuleSet() RuleSet { - return make(RuleSet) + return RuleSet{make(map[reflect.Type][]Rule), make(map[string]bool)} } // Register adds a trigger for the supplied rule for the the // specified ast nodes. -func (r RuleSet) Register(rule Rule, nodes ...ast.Node) { +func (r RuleSet) Register(rule Rule, isSuppressed bool, nodes ...ast.Node) { for _, n := range nodes { t := reflect.TypeOf(n) - if rules, ok := r[t]; ok { - r[t] = append(rules, rule) + if rules, ok := r.Rules[t]; ok { + r.Rules[t] = append(rules, rule) } else { - r[t] = []Rule{rule} + r.Rules[t] = []Rule{rule} } } + r.RuleSuppressedMap[rule.ID()] = isSuppressed } // RegisteredFor will return all rules that are registered for a // specified ast node. func (r RuleSet) RegisteredFor(n ast.Node) []Rule { - if rules, found := r[reflect.TypeOf(n)]; found { + if rules, found := r.Rules[reflect.TypeOf(n)]; found { return rules } return []Rule{} } + +// IsRuleSuppressed will return whether the rule is suppressed. +func (r RuleSet) IsRuleSuppressed(ruleID string) bool { + return r.RuleSuppressedMap[ruleID] +} diff --git a/rule_test.go b/rule_test.go index 074429f..c5ce61a 100644 --- a/rule_test.go +++ b/rule_test.go @@ -59,26 +59,36 @@ var _ = Describe("Rule", func() { registeredNodeB := (*ast.AssignStmt)(nil) unregisteredNode := (*ast.BinaryExpr)(nil) - ruleset.Register(dummyIssueRule, registeredNodeA, registeredNodeB) + ruleset.Register(dummyIssueRule, false, registeredNodeA, registeredNodeB) Expect(ruleset.RegisteredFor(unregisteredNode)).Should(BeEmpty()) Expect(ruleset.RegisteredFor(registeredNodeA)).Should(ContainElement(dummyIssueRule)) Expect(ruleset.RegisteredFor(registeredNodeB)).Should(ContainElement(dummyIssueRule)) + Expect(ruleset.IsRuleSuppressed(dummyIssueRule.ID())).Should(BeFalse()) }) It("should not register a rule when no ast.Nodes are specified", func() { - ruleset.Register(dummyErrorRule) - Expect(ruleset).Should(BeEmpty()) + ruleset.Register(dummyErrorRule, false) + Expect(ruleset.Rules).Should(BeEmpty()) }) It("should be possible to retrieve a list of rules for a given node type", func() { registeredNode := (*ast.CallExpr)(nil) unregisteredNode := (*ast.AssignStmt)(nil) - ruleset.Register(dummyErrorRule, registeredNode) - ruleset.Register(dummyIssueRule, registeredNode) + ruleset.Register(dummyErrorRule, false, registeredNode) + ruleset.Register(dummyIssueRule, false, registeredNode) Expect(ruleset.RegisteredFor(unregisteredNode)).Should(BeEmpty()) Expect(ruleset.RegisteredFor(registeredNode)).Should(HaveLen(2)) Expect(ruleset.RegisteredFor(registeredNode)).Should(ContainElement(dummyErrorRule)) Expect(ruleset.RegisteredFor(registeredNode)).Should(ContainElement(dummyIssueRule)) }) + + It("should register a suppressed rule", func() { + registeredNode := (*ast.CallExpr)(nil) + unregisteredNode := (*ast.AssignStmt)(nil) + ruleset.Register(dummyIssueRule, true, registeredNode) + Expect(ruleset.RegisteredFor(registeredNode)).Should(ContainElement(dummyIssueRule)) + Expect(ruleset.RegisteredFor(unregisteredNode)).Should(BeEmpty()) + Expect(ruleset.IsRuleSuppressed(dummyIssueRule.ID())).Should(BeTrue()) + }) }) }) diff --git a/rules/rulelist.go b/rules/rulelist.go index a3d9ca2..dc9f149 100644 --- a/rules/rulelist.go +++ b/rules/rulelist.go @@ -24,16 +24,21 @@ type RuleDefinition struct { Create gosec.RuleBuilder } -// RuleList is a mapping of rule ID's to rule definitions -type RuleList map[string]RuleDefinition +// RuleList contains a mapping of rule ID's to rule definitions and a mapping +// of rule ID's to whether rules are suppressed. +type RuleList struct { + Rules map[string]RuleDefinition + RuleSuppressed map[string]bool +} -// Builders returns all the create methods for a given rule list -func (rl RuleList) Builders() map[string]gosec.RuleBuilder { +// RulesInfo returns all the create methods and the rule suppressed map for a +// given list +func (rl RuleList) RulesInfo() (map[string]gosec.RuleBuilder, map[string]bool) { builders := make(map[string]gosec.RuleBuilder) - for _, def := range rl { + for _, def := range rl.Rules { builders[def.ID] = def.Create } - return builders + return builders, rl.RuleSuppressed } // RuleFilter can be used to include or exclude a rule depending on the return @@ -56,7 +61,7 @@ func NewRuleFilter(action bool, ruleIDs ...string) RuleFilter { } // Generate the list of rules to use -func Generate(filters ...RuleFilter) RuleList { +func Generate(trackSuppressions bool, filters ...RuleFilter) RuleList { rules := []RuleDefinition{ // misc {"G101", "Look for hardcoded credentials", NewHardcodedCredentials}, @@ -102,15 +107,20 @@ func Generate(filters ...RuleFilter) RuleList { } ruleMap := make(map[string]RuleDefinition) + ruleSuppressedMap := make(map[string]bool) RULES: for _, rule := range rules { + ruleSuppressedMap[rule.ID] = false for _, filter := range filters { if filter(rule.ID) { - continue RULES + ruleSuppressedMap[rule.ID] = true + if !trackSuppressions { + continue RULES + } } } ruleMap[rule.ID] = rule } - return ruleMap + return RuleList{ruleMap, ruleSuppressedMap} } diff --git a/rules/rules_test.go b/rules/rules_test.go index 2479fa2..9117911 100644 --- a/rules/rules_test.go +++ b/rules/rules_test.go @@ -24,12 +24,12 @@ var _ = Describe("gosec rules", func() { BeforeEach(func() { logger, _ = testutils.NewLogger() config = gosec.NewConfig() - analyzer = gosec.NewAnalyzer(config, tests, false, logger) + analyzer = gosec.NewAnalyzer(config, tests, false, false, logger) runner = func(rule string, samples []testutils.CodeSample) { for n, sample := range samples { analyzer.Reset() analyzer.SetConfig(sample.Config) - analyzer.LoadRules(rules.Generate(rules.NewRuleFilter(false, rule)).Builders()) + analyzer.LoadRules(rules.Generate(false, rules.NewRuleFilter(false, rule)).RulesInfo()) pkg := testutils.NewTestPackage() defer pkg.Close() for i, code := range sample.Code {