From fb0dc73a96d625afe04ce6ff1bbabd722e0870cc Mon Sep 17 00:00:00 2001 From: Cosmin Cojocar Date: Wed, 8 Aug 2018 16:38:57 +0200 Subject: [PATCH 1/2] Add sha1 to weak crypto primitives --- rules/blacklist.go | 7 +++++++ rules/rulelist.go | 1 + rules/rules_test.go | 8 ++++++++ rules/weakcrypto.go | 1 + testutils/source.go | 39 +++++++++++++++++++++++++++++++++++++++ 5 files changed, 56 insertions(+) diff --git a/rules/blacklist.go b/rules/blacklist.go index 74a769c..24fbf12 100644 --- a/rules/blacklist.go +++ b/rules/blacklist.go @@ -85,3 +85,10 @@ func NewBlacklistedImportCGI(id string, conf gosec.Config) (gosec.Rule, []ast.No "net/http/cgi": "Blacklisted import net/http/cgi: Go versions < 1.6.3 are vulnerable to Httpoxy attack: (CVE-2016-5386)", }) } + +// NewBlacklistedImportSHA1 fails if SHA1 is imported +func NewBlacklistedImportSHA1(id string, conf gosec.Config) (gosec.Rule, []ast.Node) { + return NewBlacklistedImports(id, conf, map[string]string{ + "crypto/sha1": "Blacklisted import crypto/sha1: weak cryptographic primitive", + }) +} diff --git a/rules/rulelist.go b/rules/rulelist.go index 16cb273..8c76a32 100644 --- a/rules/rulelist.go +++ b/rules/rulelist.go @@ -90,6 +90,7 @@ func Generate(filters ...RuleFilter) RuleList { {"G502", "Import blacklist: crypto/des", NewBlacklistedImportDES}, {"G503", "Import blacklist: crypto/rc4", NewBlacklistedImportRC4}, {"G504", "Import blacklist: net/http/cgi", NewBlacklistedImportCGI}, + {"G505", "Import blacklist: crypto/sha1", NewBlacklistedImportSHA1}, } ruleMap := make(map[string]RuleDefinition) diff --git a/rules/rules_test.go b/rules/rules_test.go index 958932f..c41e532 100644 --- a/rules/rules_test.go +++ b/rules/rules_test.go @@ -6,6 +6,7 @@ import ( . "github.com/onsi/ginkgo" . "github.com/onsi/gomega" + "github.com/securego/gosec" "github.com/securego/gosec/rules" "github.com/securego/gosec/testutils" @@ -110,6 +111,10 @@ var _ = Describe("gosec rules", func() { runner("G401", testutils.SampleCodeG401) }) + It("should detect weak crypto algorithms", func() { + runner("G401", testutils.SampleCodeG401b) + }) + It("should find insecure tls settings", func() { runner("G402", testutils.SampleCodeG402) }) @@ -137,6 +142,9 @@ var _ = Describe("gosec rules", func() { It("should detect blacklisted imports - CGI (httpoxy)", func() { runner("G504", testutils.SampleCodeG504) }) + It("should detect blacklisted imports - SHA1", func() { + runner("G505", testutils.SampleCodeG505) + }) }) diff --git a/rules/weakcrypto.go b/rules/weakcrypto.go index db1ada7..60c63aa 100644 --- a/rules/weakcrypto.go +++ b/rules/weakcrypto.go @@ -43,6 +43,7 @@ func NewUsesWeakCryptography(id string, conf gosec.Config) (gosec.Rule, []ast.No 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{ blacklist: calls, diff --git a/testutils/source.go b/testutils/source.go index 024d438..d3464b4 100644 --- a/testutils/source.go +++ b/testutils/source.go @@ -633,6 +633,31 @@ func main() { fmt.Printf("%x", h.Sum(nil)) }`, 1}} + // SampleCodeG401b - Use of weak crypto SHA1 + SampleCodeG401b = []CodeSample{ + {` +package main +import ( + "crypto/sha1" + "fmt" + "io" + "log" + "os" +) +func main() { + f, err := os.Open("file.txt") + if err != nil { + log.Fatal(err) + } + defer f.Close() + + h := sha1.New() + if _, err := io.Copy(h, f); err != nil { + log.Fatal(err) + } + fmt.Printf("%x", h.Sum(nil)) +}`, 1}} + // SampleCodeG402 - TLS settings SampleCodeG402 = []CodeSample{{` // InsecureSkipVerify @@ -827,6 +852,20 @@ import ( ) func main() { cgi.Serve(http.FileServer(http.Dir("/usr/share/doc"))) +}`, 1}} + // SampleCodeG505 - Blacklisted import SHA1 + SampleCodeG505 = []CodeSample{ + {` +package main +import ( + "crypto/sha1" + "fmt" + "os" +) +func main() { + for _, arg := range os.Args { + fmt.Printf("%x - %s\n", sha1.Sum([]byte(arg)), arg) + } }`, 1}} // SampleCode601 - Go build tags SampleCode601 = []CodeSample{{` From 8dfa8dc0150755c479ed115c1747574f0af55c43 Mon Sep 17 00:00:00 2001 From: Cosmin Cojocar Date: Wed, 8 Aug 2018 16:41:34 +0200 Subject: [PATCH 2/2] Update README --- README.md | 3 ++- rules/rulelist.go | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index afb51db..7ca8098 100644 --- a/README.md +++ b/README.md @@ -50,7 +50,7 @@ or to specify a set of rules to explicitly exclude using the '-exclude=' flag. - G303: Creating tempfile using a predictable path - G304: File path provided as taint input - G305: File traversal when extracting zip archive - - G401: Detect the usage of DES, RC4, or MD5 + - G401: Detect the usage of DES, RC4, 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) @@ -58,6 +58,7 @@ or to specify a set of rules to explicitly exclude using the '-exclude=' flag. - G502: Import blacklist: crypto/des - G503: Import blacklist: crypto/rc4 - G504: Import blacklist: net/http/cgi + - G505: Import blacklist: crypto/sha1 ``` diff --git a/rules/rulelist.go b/rules/rulelist.go index 8c76a32..e9685b9 100644 --- a/rules/rulelist.go +++ b/rules/rulelist.go @@ -80,7 +80,7 @@ func Generate(filters ...RuleFilter) RuleList { {"G305", "File path traversal when extracting zip archive", NewArchive}, // crypto - {"G401", "Detect the usage of DES, RC4, or MD5", NewUsesWeakCryptography}, + {"G401", "Detect the usage of DES, RC4, MD5 or SHA1", NewUsesWeakCryptography}, {"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},