diff --git a/README.md b/README.md index 4992a946b2..51937c4441 100644 --- a/README.md +++ b/README.md @@ -151,10 +151,11 @@ directory you can supply `./...` as the input argument. - G305: File traversal when extracting zip/tar archive - G306: Poor file permissions used when writing to a new file - G307: Poor file permissions used when creating a file with os.Create -- G401: Detect the usage of DES, RC4, MD5 or SHA1 +- G401: Detect the usage of MD5 or SHA1 - G402: Look for bad TLS connection settings - G403: Ensure minimum RSA key length of 2048 bits - G404: Insecure random number source (rand) +- G405: Detect the usage of DES or RC4 - G501: Import blocklist: crypto/md5 - G502: Import blocklist: crypto/des - G503: Import blocklist: crypto/rc4 diff --git a/analyzer_test.go b/analyzer_test.go index c8cf7db20d..879db8c656 100644 --- a/analyzer_test.go +++ b/analyzer_test.go @@ -140,6 +140,22 @@ var _ = Describe("Analyzer", func() { Expect(controlIssues).Should(HaveLen(sample.Errors)) }) + It("should find errors when nosec is not in use", func() { + sample := testutils.SampleCodeG405[0] + source := sample.Code[0] + analyzer.LoadRules(rules.Generate(false, rules.NewRuleFilter(false, "G405")).RulesInfo()) + + controlPackage := testutils.NewTestPackage() + defer controlPackage.Close() + controlPackage.AddFile("cipher.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)) + }) + It("should report Go build errors and invalid files", func() { analyzer.LoadRules(rules.Generate(false).RulesInfo()) pkg := testutils.NewTestPackage() @@ -185,6 +201,23 @@ var _ = Describe("Analyzer", func() { Expect(nosecIssues).Should(BeEmpty()) }) + It("should not report errors when a nosec line comment is present", func() { + sample := testutils.SampleCodeG405[0] + source := sample.Code[0] + analyzer.LoadRules(rules.Generate(false, rules.NewRuleFilter(false, "G405")).RulesInfo()) + + nosecPackage := testutils.NewTestPackage() + defer nosecPackage.Close() + nosecSource := strings.Replace(source, "c, e := des.NewCipher([]byte(\"mySecret\"))", "c, e := des.NewCipher([]byte(\"mySecret\")) //#nosec", 1) + nosecPackage.AddFile("cipher.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 not report errors when a nosec block comment is present", func() { sample := testutils.SampleCodeG401[0] source := sample.Code[0] @@ -202,6 +235,23 @@ var _ = Describe("Analyzer", func() { Expect(nosecIssues).Should(BeEmpty()) }) + It("should not report errors when a nosec block comment is present", func() { + sample := testutils.SampleCodeG405[0] + source := sample.Code[0] + analyzer.LoadRules(rules.Generate(false, rules.NewRuleFilter(false, "G405")).RulesInfo()) + + nosecPackage := testutils.NewTestPackage() + defer nosecPackage.Close() + nosecSource := strings.Replace(source, "c, e := des.NewCipher([]byte(\"mySecret\"))", "c, e := des.NewCipher([]byte(\"mySecret\")) /* #nosec */", 1) + nosecPackage.AddFile("cipher.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 not report errors when an exclude comment is present for the correct rule", func() { // Rule for MD5 weak crypto usage sample := testutils.SampleCodeG401[0] @@ -220,6 +270,24 @@ var _ = Describe("Analyzer", func() { Expect(nosecIssues).Should(BeEmpty()) }) + It("should not report errors when an exclude comment is present for the correct rule", func() { + // Rule for DES weak crypto usage + sample := testutils.SampleCodeG405[0] + source := sample.Code[0] + analyzer.LoadRules(rules.Generate(false, rules.NewRuleFilter(false, "G405")).RulesInfo()) + + nosecPackage := testutils.NewTestPackage() + defer nosecPackage.Close() + nosecSource := strings.Replace(source, "c, e := des.NewCipher([]byte(\"mySecret\"))", "c, e := des.NewCipher([]byte(\"mySecret\")) //#nosec G405", 1) + nosecPackage.AddFile("cipher.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 not report errors when a nosec block and line comment are present", func() { sample := testutils.SampleCodeG101[23] source := sample.Code[0] @@ -283,6 +351,23 @@ var _ = Describe("Analyzer", func() { Expect(nosecIssues).Should(HaveLen(sample.Errors)) }) + It("should report errors when an exclude comment is present for a different rule", func() { + sample := testutils.SampleCodeG405[0] + source := sample.Code[0] + analyzer.LoadRules(rules.Generate(false, rules.NewRuleFilter(false, "G405")).RulesInfo()) + + nosecPackage := testutils.NewTestPackage() + defer nosecPackage.Close() + nosecSource := strings.Replace(source, "c, e := des.NewCipher([]byte(\"mySecret\"))", "c, e := des.NewCipher([]byte(\"mySecret\")) //#nosec G301", 1) + nosecPackage.AddFile("cipher.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 an exclude comment is present for multiple rules, including the correct rule", func() { sample := testutils.SampleCodeG401[0] source := sample.Code[0] @@ -302,6 +387,25 @@ var _ = Describe("Analyzer", func() { Expect(nosecIssues).Should(BeEmpty()) }) + It("should not report errors when an exclude comment is present for multiple rules, including the correct rule", func() { + sample := testutils.SampleCodeG405[0] + source := sample.Code[0] + analyzer.LoadRules(rules.Generate(false, rules.NewRuleFilter(false, "G405")).RulesInfo()) + + nosecPackage := testutils.NewTestPackage() + defer nosecPackage.Close() + nosecSource := strings.Replace(source, "c, e := des.NewCipher([]byte(\"mySecret\"))", "c, e := des.NewCipher([]byte(\"mySecret\")) //#nosec G301 G405", 1) + nosecPackage.AddFile("cipher.go", nosecSource) + err := nosecPackage.Build() + Expect(err).ShouldNot(HaveOccurred()) + err = analyzer.Process(buildTags, nosecPackage.Path) + Expect(err).ShouldNot(HaveOccurred()) + err = analyzer.Process(buildTags, nosecPackage.Path) + Expect(err).ShouldNot(HaveOccurred()) + nosecIssues, _, _ := analyzer.Report() + Expect(nosecIssues).Should(BeEmpty()) + }) + It("should pass the build tags", func() { sample := testutils.SampleCodeBuildTag[0] source := sample.Code[0] @@ -352,6 +456,29 @@ var _ = Describe("Analyzer", func() { Expect(nosecIssues).Should(HaveLen(sample.Errors)) }) + It("should be possible to overwrite nosec comments, and report issues", func() { + // Rule for DES weak crypto usage + sample := testutils.SampleCodeG405[0] + source := sample.Code[0] + + // overwrite nosec option + nosecIgnoreConfig := gosec.NewConfig() + nosecIgnoreConfig.SetGlobal(gosec.Nosec, "true") + customAnalyzer := gosec.NewAnalyzer(nosecIgnoreConfig, tests, false, false, 1, logger) + customAnalyzer.LoadRules(rules.Generate(false, rules.NewRuleFilter(false, "G405")).RulesInfo()) + + nosecPackage := testutils.NewTestPackage() + defer nosecPackage.Close() + nosecSource := strings.Replace(source, "c, e := des.NewCipher([]byte(\"mySecret\"))", "c, e := des.NewCipher([]byte(\"mySecret\")) //#nosec", 1) + nosecPackage.AddFile("cipher.go", nosecSource) + err := nosecPackage.Build() + Expect(err).ShouldNot(HaveOccurred()) + err = customAnalyzer.Process(buildTags, nosecPackage.Path) + Expect(err).ShouldNot(HaveOccurred()) + nosecIssues, _, _ := customAnalyzer.Report() + Expect(nosecIssues).Should(HaveLen(sample.Errors)) + }) + It("should be possible to overwrite nosec comments, and report issues but they should not be counted", func() { // Rule for MD5 weak crypto usage sample := testutils.SampleCodeG401[0] @@ -378,6 +505,32 @@ var _ = Describe("Analyzer", func() { Expect(metrics.NumNosec).Should(Equal(1)) }) + It("should be possible to overwrite nosec comments, and report issues but they should not be counted", func() { + // Rule for DES weak crypto usage + sample := testutils.SampleCodeG405[0] + source := sample.Code[0] + + // overwrite nosec option + nosecIgnoreConfig := gosec.NewConfig() + nosecIgnoreConfig.SetGlobal(gosec.Nosec, "mynosec") + nosecIgnoreConfig.SetGlobal(gosec.ShowIgnored, "true") + customAnalyzer := gosec.NewAnalyzer(nosecIgnoreConfig, tests, false, false, 1, logger) + customAnalyzer.LoadRules(rules.Generate(false, rules.NewRuleFilter(false, "G405")).RulesInfo()) + + nosecPackage := testutils.NewTestPackage() + defer nosecPackage.Close() + nosecSource := strings.Replace(source, "c, e := des.NewCipher([]byte(\"mySecret\"))", "c, e := des.NewCipher([]byte(\"mySecret\")) // #mynosec", 1) + nosecPackage.AddFile("cipher.go", nosecSource) + err := nosecPackage.Build() + Expect(err).ShouldNot(HaveOccurred()) + err = customAnalyzer.Process(buildTags, nosecPackage.Path) + Expect(err).ShouldNot(HaveOccurred()) + nosecIssues, metrics, _ := customAnalyzer.Report() + Expect(nosecIssues).Should(HaveLen(sample.Errors)) + Expect(metrics.NumFound).Should(Equal(0)) + 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] @@ -395,6 +548,23 @@ var _ = Describe("Analyzer", func() { Expect(nosecIssues).Should(BeEmpty()) }) + It("should not report errors when nosec tag is in front of a line", func() { + sample := testutils.SampleCodeG405[0] + source := sample.Code[0] + analyzer.LoadRules(rules.Generate(false, rules.NewRuleFilter(false, "G405")).RulesInfo()) + + nosecPackage := testutils.NewTestPackage() + defer nosecPackage.Close() + nosecSource := strings.Replace(source, "c, e := des.NewCipher([]byte(\"mySecret\"))", "//Some description\n//#nosec G405\nc, e := des.NewCipher([]byte(\"mySecret\"))", 1) + nosecPackage.AddFile("cipher.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] @@ -412,6 +582,23 @@ var _ = Describe("Analyzer", func() { Expect(nosecIssues).Should(HaveLen(sample.Errors)) }) + It("should report errors when nosec tag is not in front of a line", func() { + sample := testutils.SampleCodeG405[0] + source := sample.Code[0] + analyzer.LoadRules(rules.Generate(false, rules.NewRuleFilter(false, "G405")).RulesInfo()) + + nosecPackage := testutils.NewTestPackage() + defer nosecPackage.Close() + nosecSource := strings.Replace(source, "c, e := des.NewCipher([]byte(\"mySecret\"))", "//Some description\n//Another description #nosec G405\nc, e := des.NewCipher([]byte(\"mySecret\"))", 1) + nosecPackage.AddFile("cipher.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] @@ -429,6 +616,23 @@ var _ = Describe("Analyzer", func() { Expect(nosecIssues).Should(BeEmpty()) }) + It("should not report errors when rules are in front of nosec tag even rules are wrong", func() { + sample := testutils.SampleCodeG405[0] + source := sample.Code[0] + analyzer.LoadRules(rules.Generate(false, rules.NewRuleFilter(false, "G405")).RulesInfo()) + + nosecPackage := testutils.NewTestPackage() + defer nosecPackage.Close() + nosecSource := strings.Replace(source, "c, e := des.NewCipher([]byte(\"mySecret\"))", "//G301\n//#nosec\nc, e := des.NewCipher([]byte(\"mySecret\"))", 1) + nosecPackage.AddFile("cipher.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] @@ -446,6 +650,23 @@ var _ = Describe("Analyzer", func() { Expect(nosecIssues).Should(HaveLen(sample.Errors)) }) + It("should report errors when there are nosec tags after a #nosec WrongRuleList annotation", func() { + sample := testutils.SampleCodeG405[0] + source := sample.Code[0] + analyzer.LoadRules(rules.Generate(false, rules.NewRuleFilter(false, "G405")).RulesInfo()) + + nosecPackage := testutils.NewTestPackage() + defer nosecPackage.Close() + nosecSource := strings.Replace(source, "c, e := des.NewCipher([]byte(\"mySecret\"))", "//#nosec\n//G301\n//#nosec\nc, e := des.NewCipher([]byte(\"mySecret\"))", 1) + nosecPackage.AddFile("cipher.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] @@ -469,6 +690,29 @@ var _ = Describe("Analyzer", func() { Expect(nosecIssues).Should(BeEmpty()) }) + It("should be possible to use an alternative nosec tag", func() { + // Rule for DES weak crypto usage + sample := testutils.SampleCodeG405[0] + source := sample.Code[0] + + // overwrite nosec option + nosecIgnoreConfig := gosec.NewConfig() + nosecIgnoreConfig.SetGlobal(gosec.NoSecAlternative, "falsePositive") + customAnalyzer := gosec.NewAnalyzer(nosecIgnoreConfig, tests, false, false, 1, logger) + customAnalyzer.LoadRules(rules.Generate(false, rules.NewRuleFilter(false, "G405")).RulesInfo()) + + nosecPackage := testutils.NewTestPackage() + defer nosecPackage.Close() + nosecSource := strings.Replace(source, "c, e := des.NewCipher([]byte(\"mySecret\"))", "c, e := des.NewCipher([]byte(\"mySecret\")) // #falsePositive", 1) + nosecPackage.AddFile("cipher.go", nosecSource) + err := nosecPackage.Build() + Expect(err).ShouldNot(HaveOccurred()) + err = customAnalyzer.Process(buildTags, nosecPackage.Path) + Expect(err).ShouldNot(HaveOccurred()) + nosecIssues, _, _ := customAnalyzer.Report() + Expect(nosecIssues).Should(BeEmpty()) + }) + It("should ignore vulnerabilities when the default tag is found", func() { // Rule for MD5 weak crypto usage sample := testutils.SampleCodeG401[0] @@ -492,6 +736,29 @@ var _ = Describe("Analyzer", func() { Expect(nosecIssues).Should(BeEmpty()) }) + It("should ignore vulnerabilities when the default tag is found", func() { + // Rule for DES weak crypto usage + sample := testutils.SampleCodeG405[0] + source := sample.Code[0] + + // overwrite nosec option + nosecIgnoreConfig := gosec.NewConfig() + nosecIgnoreConfig.SetGlobal(gosec.NoSecAlternative, "falsePositive") + customAnalyzer := gosec.NewAnalyzer(nosecIgnoreConfig, tests, false, false, 1, logger) + customAnalyzer.LoadRules(rules.Generate(false, rules.NewRuleFilter(false, "G405")).RulesInfo()) + + nosecPackage := testutils.NewTestPackage() + defer nosecPackage.Close() + nosecSource := strings.Replace(source, "c, e := des.NewCipher([]byte(\"mySecret\"))", "c, e := des.NewCipher([]byte(\"mySecret\")) //#nosec", 1) + nosecPackage.AddFile("cipher.go", nosecSource) + err := nosecPackage.Build() + Expect(err).ShouldNot(HaveOccurred()) + err = customAnalyzer.Process(buildTags, nosecPackage.Path) + Expect(err).ShouldNot(HaveOccurred()) + nosecIssues, _, _ := customAnalyzer.Report() + Expect(nosecIssues).Should(BeEmpty()) + }) + It("should be able to analyze Go test package", func() { customAnalyzer := gosec.NewAnalyzer(nil, true, false, false, 1, logger) customAnalyzer.LoadRules(rules.Generate(false).RulesInfo()) @@ -813,6 +1080,26 @@ var _ = Describe("Analyzer", func() { Expect(issues[0].Suppressions[0].Justification).To(Equal("Justification")) }) + It("should not report an error if the violation is suppressed", func() { + sample := testutils.SampleCodeG405[0] + source := sample.Code[0] + analyzer.LoadRules(rules.Generate(false, rules.NewRuleFilter(false, "G405")).RulesInfo()) + + nosecPackage := testutils.NewTestPackage() + defer nosecPackage.Close() + nosecSource := strings.Replace(source, "c, e := des.NewCipher([]byte(\"mySecret\"))", "c, e := des.NewCipher([]byte(\"mySecret\")) //#nosec G405 -- Justification", 1) + nosecPackage.AddFile("cipher.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(sample.Errors)) + 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] @@ -833,6 +1120,26 @@ var _ = Describe("Analyzer", func() { Expect(issues[0].Suppressions[0].Justification).To(Equal("")) }) + It("should not report an error if the violation is suppressed without certain rules", func() { + sample := testutils.SampleCodeG405[0] + source := sample.Code[0] + analyzer.LoadRules(rules.Generate(false, rules.NewRuleFilter(false, "G405")).RulesInfo()) + + nosecPackage := testutils.NewTestPackage() + defer nosecPackage.Close() + nosecSource := strings.Replace(source, "c, e := des.NewCipher([]byte(\"mySecret\"))", "c, e := des.NewCipher([]byte(\"mySecret\")) //#nosec", 1) + nosecPackage.AddFile("cipher.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(sample.Errors)) + 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] diff --git a/call_list_test.go b/call_list_test.go index 940bc05c9c..498fb78a2a 100644 --- a/call_list_test.go +++ b/call_list_test.go @@ -94,6 +94,32 @@ var _ = Describe("Call List", func() { Expect(matched).Should(Equal(1)) }) + It("should match a package call expression", func() { + // Create file to be scanned + pkg := testutils.NewTestPackage() + defer pkg.Close() + pkg.AddFile("cipher.go", testutils.SampleCodeG405[0].Code[0]) + + ctx := pkg.CreateContext("cipher.go") + + // Search for des.NewCipher() + calls.Add("crypto/des", "NewCipher") + + // Stub out visitor and count number of matched call expr + matched := 0 + v := testutils.NewMockVisitor() + v.Context = ctx + v.Callback = func(n ast.Node, ctx *gosec.Context) bool { + if _, ok := n.(*ast.CallExpr); ok && calls.ContainsPkgCallExpr(n, ctx, false) != nil { + matched++ + } + return true + } + ast.Walk(v, ctx.Root) + Expect(matched).Should(Equal(1)) + }) + + It("should match a call expression", func() { // Create file to be scanned pkg := testutils.NewTestPackage() diff --git a/issue/issue.go b/issue/issue.go index a2de0dcc29..d10f9506ee 100644 --- a/issue/issue.go +++ b/issue/issue.go @@ -82,6 +82,7 @@ var ruleToCWE = map[string]string{ "G402": "295", "G403": "310", "G404": "338", + "G405": "327", "G501": "327", "G502": "327", "G503": "327", diff --git a/report/formatter_test.go b/report/formatter_test.go index 38af8bd50c..acaa84b7a5 100644 --- a/report/formatter_test.go +++ b/report/formatter_test.go @@ -281,8 +281,8 @@ var _ = Describe("Formatter", func() { "G101", "G102", "G103", "G104", "G106", "G107", "G109", "G110", "G111", "G112", "G113", "G201", "G202", "G203", "G204", "G301", "G302", "G303", "G304", "G305", "G401", - "G402", "G403", "G404", "G501", "G502", "G503", "G504", - "G505", "G601", + "G402", "G403", "G404", "G405", "G501", "G502", "G503", + "G504", "G505", "G601", } It("csv formatted report should contain the CWE mapping", func() { diff --git a/rules/rulelist.go b/rules/rulelist.go index f9ca4f52c4..cb905f4108 100644 --- a/rules/rulelist.go +++ b/rules/rulelist.go @@ -94,10 +94,11 @@ func Generate(trackSuppressions bool, filters ...RuleFilter) RuleList { {"G307", "Poor file permissions used when creating a file with os.Create", NewOsCreatePerms}, // crypto - {"G401", "Detect the usage of DES, RC4, MD5 or SHA1", NewUsesWeakCryptography}, + {"G401", "Detect the usage of MD5 or SHA1", NewUsesWeakCryptographyHash}, {"G402", "Look for bad TLS connection settings", NewIntermediateTLSCheck}, {"G403", "Ensure minimum RSA key length of 2048 bits", NewWeakKeyStrength}, {"G404", "Insecure random number source (rand)", NewWeakRandCheck}, + {"G405", "Detect the usage of DES or RC4", NewUsesWeakCryptographyEncryption}, // blocklist {"G501", "Import blocklist: crypto/md5", NewBlocklistedImportMD5}, diff --git a/rules/rules_test.go b/rules/rules_test.go index aa2e2a4834..1d3e3dd617 100644 --- a/rules/rules_test.go +++ b/rules/rules_test.go @@ -175,6 +175,14 @@ var _ = Describe("gosec rules", func() { runner("G404", testutils.SampleCodeG404) }) + It("should detect weak crypto algorithms", func() { + runner("G405", testutils.SampleCodeG405) + }) + + It("should detect weak crypto algorithms", func() { + runner("G405", testutils.SampleCodeG405b) + }) + It("should detect blocklisted imports - MD5", func() { runner("G501", testutils.SampleCodeG501) }) diff --git a/rules/weakcrypto.go b/rules/weakcryptoencryption.go similarity index 75% rename from rules/weakcrypto.go rename to rules/weakcryptoencryption.go index 4f2ab11d15..143f67d4e8 100644 --- a/rules/weakcrypto.go +++ b/rules/weakcryptoencryption.go @@ -21,16 +21,16 @@ import ( "github.com/securego/gosec/v2/issue" ) -type usesWeakCryptography struct { +type usesWeakCryptographyEncryption struct { issue.MetaData blocklist map[string][]string } -func (r *usesWeakCryptography) ID() string { +func (r *usesWeakCryptographyEncryption) ID() string { return r.MetaData.ID } -func (r *usesWeakCryptography) Match(n ast.Node, c *gosec.Context) (*issue.Issue, error) { +func (r *usesWeakCryptographyEncryption) Match(n ast.Node, c *gosec.Context) (*issue.Issue, error) { for pkg, funcs := range r.blocklist { if _, matched := gosec.MatchCallByPackage(n, c, pkg, funcs...); matched { return c.NewIssue(n, r.ID(), r.What, r.Severity, r.Confidence), nil @@ -39,14 +39,12 @@ func (r *usesWeakCryptography) Match(n ast.Node, c *gosec.Context) (*issue.Issue return nil, nil } -// NewUsesWeakCryptography detects uses of des.* md5.* or rc4.* -func NewUsesWeakCryptography(id string, _ gosec.Config) (gosec.Rule, []ast.Node) { +// NewUsesWeakCryptographyEncryption detects uses of des.*, rc4.* +func NewUsesWeakCryptographyEncryption(id string, _ gosec.Config) (gosec.Rule, []ast.Node) { calls := make(map[string][]string) calls["crypto/des"] = []string{"NewCipher", "NewTripleDESCipher"} - calls["crypto/md5"] = []string{"New", "Sum"} - calls["crypto/sha1"] = []string{"New", "Sum"} calls["crypto/rc4"] = []string{"NewCipher"} - rule := &usesWeakCryptography{ + rule := &usesWeakCryptographyEncryption{ blocklist: calls, MetaData: issue.MetaData{ ID: id, diff --git a/rules/weakcryptohash.go b/rules/weakcryptohash.go new file mode 100644 index 0000000000..3282b5a3c2 --- /dev/null +++ b/rules/weakcryptohash.go @@ -0,0 +1,57 @@ +// (c) Copyright 2016 Hewlett Packard Enterprise Development LP +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package rules + +import ( + "go/ast" + + "github.com/securego/gosec/v2" + "github.com/securego/gosec/v2/issue" +) + +type usesWeakCryptographyHash struct { + issue.MetaData + blocklist map[string][]string +} + +func (r *usesWeakCryptographyHash) ID() string { + return r.MetaData.ID +} + +func (r *usesWeakCryptographyHash) Match(n ast.Node, c *gosec.Context) (*issue.Issue, error) { + for pkg, funcs := range r.blocklist { + if _, matched := gosec.MatchCallByPackage(n, c, pkg, funcs...); matched { + return c.NewIssue(n, r.ID(), r.What, r.Severity, r.Confidence), nil + } + } + return nil, nil +} + +// NewUsesWeakCryptographyHash detects uses of md5.*, sha1.* +func NewUsesWeakCryptographyHash(id string, _ gosec.Config) (gosec.Rule, []ast.Node) { + calls := make(map[string][]string) + calls["crypto/md5"] = []string{"New", "Sum"} + calls["crypto/sha1"] = []string{"New", "Sum"} + rule := &usesWeakCryptographyHash{ + blocklist: calls, + MetaData: issue.MetaData{ + ID: id, + Severity: issue.Medium, + Confidence: issue.High, + What: "Use of weak cryptographic primitive", + }, + } + return rule, []ast.Node{(*ast.CallExpr)(nil)} +} diff --git a/testutils/g401_samples.go b/testutils/g401_samples.go index 86bf23b315..90a22c4018 100644 --- a/testutils/g401_samples.go +++ b/testutils/g401_samples.go @@ -3,7 +3,7 @@ package testutils import "github.com/securego/gosec/v2" var ( - // SampleCodeG401 - Use of weak crypto MD5 + // SampleCodeG401 - Use of weak crypto hash MD5 SampleCodeG401 = []CodeSample{ {[]string{` package main @@ -39,7 +39,7 @@ func main() { `}, 1, gosec.NewConfig()}, } - // SampleCodeG401b - Use of weak crypto SHA1 + // SampleCodeG401b - Use of weak crypto hash SHA1 SampleCodeG401b = []CodeSample{ {[]string{` package main diff --git a/testutils/g405_samples .go b/testutils/g405_samples .go new file mode 100644 index 0000000000..05dc648cd0 --- /dev/null +++ b/testutils/g405_samples .go @@ -0,0 +1,67 @@ +package testutils + +import "github.com/securego/gosec/v2" + +var ( + // SampleCodeG405 - Use of weak crypto encryption DES + SampleCodeG405 = []CodeSample{ + {[]string{` +package main + +import ( + "crypto/des" + "fmt" +) + +func main() { + // Weakness: Usage of weak encryption algorithm + + c, e := des.NewCipher([]byte("mySecret")) + + if e != nil { + panic("We have a problem: " + e.Error()) + } + + data := []byte("hello world") + fmt.Println("Plain", string(data)) + c.Encrypt(data, data) + + fmt.Println("Encrypted", string(data)) + c.Decrypt(data, data) + + fmt.Println("Plain Decrypted", string(data)) +} + +`}, 1, gosec.NewConfig()}, + } + + // SampleCodeG405b - Use of weak crypto encryption RC4 + SampleCodeG405b = []CodeSample{ + {[]string{` +package main + +import ( + "crypto/rc4" + "fmt" +) + +func main() { + // Weakness: Usage of weak encryption algorithm + + c, _ := rc4.NewCipher([]byte("mySecret")) + + data := []byte("hello world") + fmt.Println("Plain", string(data)) + c.XORKeyStream(data, data) + + cryptCipher2, _ := rc4.NewCipher([]byte("mySecret")) + + fmt.Println("Encrypted", string(data)) + cryptCipher2.XORKeyStream(data, data) + + fmt.Println("Plain Decrypted", string(data)) +} + +`}, 1, gosec.NewConfig()}, + } +)