Skip to content

Commit

Permalink
Split the G401 rule into two separate ones
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
Dimitar Banchev committed Jun 20, 2024
1 parent 2f74a03 commit 89f3249
Show file tree
Hide file tree
Showing 11 changed files with 480 additions and 14 deletions.
3 changes: 2 additions & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
307 changes: 307 additions & 0 deletions analyzer_test.go

Large diffs are not rendered by default.

26 changes: 26 additions & 0 deletions call_list_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down
1 change: 1 addition & 0 deletions issue/issue.go
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,7 @@ var ruleToCWE = map[string]string{
"G402": "295",
"G403": "310",
"G404": "338",
"G405": "327",
"G501": "327",
"G502": "327",
"G503": "327",
Expand Down
4 changes: 2 additions & 2 deletions report/formatter_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand Down
3 changes: 2 additions & 1 deletion rules/rulelist.go
Original file line number Diff line number Diff line change
Expand Up @@ -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},
Expand Down
8 changes: 8 additions & 0 deletions rules/rules_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
})
Expand Down
14 changes: 6 additions & 8 deletions rules/weakcrypto.go → rules/weakcryptoencryption.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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,
Expand Down
57 changes: 57 additions & 0 deletions rules/weakcryptohash.go
Original file line number Diff line number Diff line change
@@ -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)}
}
4 changes: 2 additions & 2 deletions testutils/g401_samples.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
67 changes: 67 additions & 0 deletions testutils/g405_samples .go
Original file line number Diff line number Diff line change
@@ -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()},
}
)

0 comments on commit 89f3249

Please sign in to comment.