Fix #743 (#748)

* Check if nosec tag is in front of a line * Use \n instead of a whitespace in a test case
securego · Jan 3, 2022 · 72f1145 · 72f1145
1 parent 63a8e78
commit 72f1145
Show file tree

Hide file tree

Showing 2 changed files with 77 additions and 2 deletions.
diff --git a/analyzer.go b/analyzer.go
@@ -325,12 +325,19 @@ func (gosec *Analyzer) ignore(n ast.Node) map[string]SuppressionInfo {
 
 		for _, group := range groups {
 			comment := strings.TrimSpace(group.Text())
-			foundDefaultTag := strings.HasPrefix(comment, noSecDefaultTag)
-			foundAlternativeTag := strings.HasPrefix(comment, noSecAlternativeTag)
+			foundDefaultTag := strings.HasPrefix(comment, noSecDefaultTag) || regexp.MustCompile("\n *"+noSecDefaultTag).Match([]byte(comment))
+			foundAlternativeTag := strings.HasPrefix(comment, noSecAlternativeTag) || regexp.MustCompile("\n *"+noSecAlternativeTag).Match([]byte(comment))
 
 			if foundDefaultTag || foundAlternativeTag {
 				gosec.stats.NumNosec++
 
+				// Discard what's in front of the nosec tag.
+				if foundDefaultTag {
+					comment = strings.SplitN(comment, noSecDefaultTag, 2)[1]
+				} else {
+					comment = strings.SplitN(comment, noSecAlternativeTag, 2)[1]
+				}
+
 				// Extract the directive and the justification.
 				justification := ""
 				commentParts := regexp.MustCompile(`-{2,}`).Split(comment, 2)

diff --git a/analyzer_test.go b/analyzer_test.go
@@ -303,6 +303,74 @@ var _ = Describe("Analyzer", func() {
 			Expect(metrics.NumNosec).Should(Equal(1))
 		})
 
+		It("should not report errors when nosec tag is in front of a line", 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()", "//Some description\n//#nosec G401\nh := md5.New()", 1)
+			nosecPackage.AddFile("md5.go", nosecSource)
+			err := nosecPackage.Build()
+			Expect(err).ShouldNot(HaveOccurred())
+			err = analyzer.Process(buildTags, nosecPackage.Path)
+			Expect(err).ShouldNot(HaveOccurred())
+			nosecIssues, _, _ := analyzer.Report()
+			Expect(nosecIssues).Should(BeEmpty())
+		})
+
+		It("should report errors when nosec tag is not in front of a line", 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()", "//Some description\n//Another description #nosec G401\nh := md5.New()", 1)
+			nosecPackage.AddFile("md5.go", nosecSource)
+			err := nosecPackage.Build()
+			Expect(err).ShouldNot(HaveOccurred())
+			err = analyzer.Process(buildTags, nosecPackage.Path)
+			Expect(err).ShouldNot(HaveOccurred())
+			nosecIssues, _, _ := analyzer.Report()
+			Expect(nosecIssues).Should(HaveLen(sample.Errors))
+		})
+
+		It("should not report errors when rules are in front of nosec tag even rules are wrong", 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()", "//G301\n//#nosec\nh := md5.New()", 1)
+			nosecPackage.AddFile("md5.go", nosecSource)
+			err := nosecPackage.Build()
+			Expect(err).ShouldNot(HaveOccurred())
+			err = analyzer.Process(buildTags, nosecPackage.Path)
+			Expect(err).ShouldNot(HaveOccurred())
+			nosecIssues, _, _ := analyzer.Report()
+			Expect(nosecIssues).Should(BeEmpty())
+		})
+
+		It("should report errors when there are nosec tags after a #nosec WrongRuleList annotation", 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()", "//#nosec\n//G301\n//#nosec\nh := md5.New()", 1)
+			nosecPackage.AddFile("md5.go", nosecSource)
+			err := nosecPackage.Build()
+			Expect(err).ShouldNot(HaveOccurred())
+			err = analyzer.Process(buildTags, nosecPackage.Path)
+			Expect(err).ShouldNot(HaveOccurred())
+			nosecIssues, _, _ := analyzer.Report()
+			Expect(nosecIssues).Should(HaveLen(sample.Errors))
+		})
+
 		It("should be possible to use an alternative nosec tag", func() {
 			// Rule for MD5 weak crypto usage
 			sample := testutils.SampleCodeG401[0]