From 58e4fccc1382194f682ee8f97860f5b9c7aa491a Mon Sep 17 00:00:00 2001 From: Dimitar Banchev Date: Thu, 20 Jun 2024 13:02:59 +0200 Subject: [PATCH] Split the G401 rule into two separate ones Now the G401 rule is split into hashing and encryption algorithms. G401 is responsible for checking the usage of MD5 and SHA1, with corresponding CWE of 328. And G405(New rule) is responsible for checking the usege of DES and RC4, with corresponding CWE of 327. --- README.md | 3 +- analyzer_test.go | 307 ++++++++++++++++++ call_list_test.go | 26 ++ issue/issue.go | 1 + report/formatter_test.go | 4 +- rules/rulelist.go | 3 +- rules/rules_test.go | 8 + ...{weakcrypto.go => weakcryptoencryption.go} | 14 +- rules/weakcryptohash.go | 57 ++++ testutils/g401_samples.go | 4 +- testutils/g405_samples .go | 67 ++++ 11 files changed, 480 insertions(+), 14 deletions(-) rename rules/{weakcrypto.go => weakcryptoencryption.go} (75%) create mode 100644 rules/weakcryptohash.go create mode 100644 testutils/g405_samples .go 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()}, + } +)