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.

Author: Dimitar Banchev

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

analyzer_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()

call_list_test.go

@@ -82,6 +82,7 @@ var ruleToCWE = map[string]string{
 	"G402": "295",
 	"G403": "310",
 	"G404": "338",
+	"G405": "327",
 	"G501": "327",
 	"G502": "327",
 	"G503": "327",

issue/issue.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() {

report/formatter_test.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},

rules/rulelist.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)
 		})

rules/rules_test.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,

rules/weakcrypto.go renamed to rules/weakcryptoencryption.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)}
+}

rules/weakcryptohash.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

testutils/g401_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()},
+	}
+)