From 64d58c2e51338e3a8e4eacb3fb857e293b9615e8 Mon Sep 17 00:00:00 2001 From: Cosmin Cojocar Date: Fri, 28 Sep 2018 11:42:25 +0300 Subject: [PATCH] Refactor the test code sample to support multiple files per sample --- analyzer_test.go | 14 +-- call_list_test.go | 2 +- rules/rules_test.go | 16 +--- testutils/source.go | 202 +++++++++++++++++++++----------------------- 4 files changed, 105 insertions(+), 129 deletions(-) diff --git a/analyzer_test.go b/analyzer_test.go index 5252af1..1942f44 100644 --- a/analyzer_test.go +++ b/analyzer_test.go @@ -98,7 +98,7 @@ var _ = Describe("Analyzer", func() { // Rule for MD5 weak crypto usage sample := testutils.SampleCodeG401[0] - source := sample.Code + source := sample.Code[0] analyzer.LoadRules(rules.Generate(rules.NewRuleFilter(false, "G401")).Builders()) controlPackage := testutils.NewTestPackage() @@ -114,7 +114,7 @@ var _ = Describe("Analyzer", func() { It("should not report errors when a nosec comment is present", func() { // Rule for MD5 weak crypto usage sample := testutils.SampleCodeG401[0] - source := sample.Code + source := sample.Code[0] analyzer.LoadRules(rules.Generate(rules.NewRuleFilter(false, "G401")).Builders()) nosecPackage := testutils.NewTestPackage() @@ -131,7 +131,7 @@ var _ = Describe("Analyzer", func() { 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] - source := sample.Code + source := sample.Code[0] analyzer.LoadRules(rules.Generate(rules.NewRuleFilter(false, "G401")).Builders()) nosecPackage := testutils.NewTestPackage() @@ -148,7 +148,7 @@ var _ = Describe("Analyzer", func() { It("should report errors when an exclude comment is present for a different rule", func() { // Rule for MD5 weak crypto usage sample := testutils.SampleCodeG401[0] - source := sample.Code + source := sample.Code[0] analyzer.LoadRules(rules.Generate(rules.NewRuleFilter(false, "G401")).Builders()) nosecPackage := testutils.NewTestPackage() @@ -165,7 +165,7 @@ var _ = Describe("Analyzer", func() { It("should not report errors when an exclude comment is present for multiple rules, including the correct rule", func() { // Rule for MD5 weak crypto usage sample := testutils.SampleCodeG401[0] - source := sample.Code + source := sample.Code[0] analyzer.LoadRules(rules.Generate(rules.NewRuleFilter(false, "G401")).Builders()) nosecPackage := testutils.NewTestPackage() @@ -181,7 +181,7 @@ var _ = Describe("Analyzer", func() { It("should pass the build tags", func() { sample := testutils.SampleCode601[0] - source := sample.Code + source := sample.Code[0] analyzer.LoadRules(rules.Generate().Builders()) pkg := testutils.NewTestPackage() defer pkg.Close() @@ -197,7 +197,7 @@ var _ = Describe("Analyzer", func() { // Rule for MD5 weak crypto usage sample := testutils.SampleCodeG401[0] - source := sample.Code + source := sample.Code[0] // overwrite nosec option nosecIgnoreConfig := gosec.NewConfig() diff --git a/call_list_test.go b/call_list_test.go index 284e2e8..2f1cf60 100644 --- a/call_list_test.go +++ b/call_list_test.go @@ -61,7 +61,7 @@ var _ = Describe("call list", func() { // Create file to be scanned pkg := testutils.NewTestPackage() defer pkg.Close() - pkg.AddFile("md5.go", testutils.SampleCodeG401[0].Code) + pkg.AddFile("md5.go", testutils.SampleCodeG401[0].Code[0]) ctx := pkg.CreateContext("md5.go") diff --git a/rules/rules_test.go b/rules/rules_test.go index d617a60..71657c5 100644 --- a/rules/rules_test.go +++ b/rules/rules_test.go @@ -28,25 +28,13 @@ var _ = Describe("gosec rules", func() { analyzer = gosec.NewAnalyzer(config, logger) runner = func(rule string, samples []testutils.CodeSample) { analyzer.LoadRules(rules.Generate(rules.NewRuleFilter(false, rule)).Builders()) - - supportingFiles := []string{} - for _, sample := range samples { - if sample.SupportingCode { - supportingFiles = append(supportingFiles, sample.Code) - } - } - for n, sample := range samples { - if sample.SupportingCode { - continue - } analyzer.Reset() pkg := testutils.NewTestPackage() defer pkg.Close() - for n, supportingCode := range supportingFiles { - pkg.AddFile(fmt.Sprintf("supporting_sample_%d.go", n), supportingCode) + for i, code := range sample.Code { + pkg.AddFile(fmt.Sprintf("sample_%d_%d.go", n, i), code) } - pkg.AddFile(fmt.Sprintf("sample_%d.go", n), sample.Code) err := pkg.Build() Expect(err).ShouldNot(HaveOccurred()) err = analyzer.Process(buildTags, pkg.Path) diff --git a/testutils/source.go b/testutils/source.go index 4a692dd..6a67ee7 100644 --- a/testutils/source.go +++ b/testutils/source.go @@ -1,25 +1,21 @@ package testutils // CodeSample encapsulates a snippet of source code that compiles, and how many errors should be detected -// Setting SupportingCode to true means that that snippet will not be scanned against -// the rules, but will be added to the same package with the other snippets. -// See SampleCodeG202 for an example. type CodeSample struct { - Code string - Errors int - SupportingCode bool + Code []string + Errors int } var ( // SampleCodeG101 code snippets for hardcoded credentials - SampleCodeG101 = []CodeSample{{` + SampleCodeG101 = []CodeSample{{[]string{` package main import "fmt" func main() { username := "admin" password := "f62e5bcda4fae4f82370da0c6f20697b8f8447ef" fmt.Println("Doing something with: ", username, password) -}`, 1, false}, {` +}`}, 1}, {[]string{` // Entropy check should not report this error by default package main import "fmt" @@ -27,21 +23,21 @@ func main() { username := "admin" password := "secret" fmt.Println("Doing something with: ", username, password) -}`, 0, false}, {` +}`}, 0}, {[]string{` package main import "fmt" var password = "f62e5bcda4fae4f82370da0c6f20697b8f8447ef" func main() { username := "admin" fmt.Println("Doing something with: ", username, password) -}`, 1, false}, {` +}`}, 1}, {[]string{` package main import "fmt" const password = "f62e5bcda4fae4f82370da0c6f20697b8f8447ef" func main() { username := "admin" fmt.Println("Doing something with: ", username, password) -}`, 1, false}, {` +}`}, 1}, {[]string{` package main import "fmt" const ( @@ -50,12 +46,12 @@ const ( ) func main() { fmt.Println("Doing something with: ", username, password) -}`, 1, false}, {` +}`}, 1}, {[]string{` package main var password string func init() { password = "f62e5bcda4fae4f82370da0c6f20697b8f8447ef" -}`, 1, false}, {` +}`}, 1}, {[]string{` package main const ( ATNStateSomethingElse = 1 @@ -63,19 +59,19 @@ const ( ) func main() { println(ATNStateTokenStart) -}`, 0, false}, {` +}`}, 0}, {[]string{` package main const ( ATNStateTokenStart = "f62e5bcda4fae4f82370da0c6f20697b8f8447ef" ) func main() { println(ATNStateTokenStart) -}`, 1, false}} +}`}, 1}} // SampleCodeG102 code snippets for network binding SampleCodeG102 = []CodeSample{ // Bind to all networks explicitly - {` + {[]string{` package main import ( "log" @@ -87,10 +83,10 @@ func main() { log.Fatal(err) } defer l.Close() -}`, 1, false}, +}`}, 1}, // Bind to all networks implicitly (default if host omitted) - {` + {[]string{` package main import ( "log" @@ -102,11 +98,11 @@ func main() { log.Fatal(err) } defer l.Close() -}`, 1, false}, +}`}, 1}, } // SampleCodeG103 find instances of unsafe blocks for auditing purposes SampleCodeG103 = []CodeSample{ - {` + {[]string{` package main import ( "fmt" @@ -124,11 +120,11 @@ func main() { addressHolder := uintptr(unsafe.Pointer(intPtr)) + unsafe.Sizeof(intArray[0]) intPtr = (*int)(unsafe.Pointer(addressHolder)) fmt.Printf("\nintPtr=%p, *intPtr=%d.\n\n", intPtr, *intPtr) -}`, 3, false}} +}`}, 3}} // SampleCodeG104 finds errors that aren't being handled SampleCodeG104 = []CodeSample{ - {` + {[]string{` package main import "fmt" func test() (int,error) { @@ -137,7 +133,7 @@ func test() (int,error) { func main() { v, _ := test() fmt.Println(v) -}`, 1, false}, {` +}`}, 1}, {[]string{` package main import ( "io/ioutil" @@ -159,7 +155,7 @@ func main() { a() b() c() -}`, 3, false}, {` +}`}, 3}, {[]string{` package main import "fmt" func test() error { @@ -168,10 +164,10 @@ func test() error { func main() { e := test() fmt.Println(e) -}`, 0, false}} +}`}, 0}} // SampleCodeG105 - bignum overflow - SampleCodeG105 = []CodeSample{{` + SampleCodeG105 = []CodeSample{{[]string{` package main import ( "math/big" @@ -185,20 +181,20 @@ func main() { m := new(big.Int) m = m.SetUint64(0) z = z.Exp(x, y, m) -}`, 1, false}} +}`}, 1}} // SampleCodeG106 - ssh InsecureIgnoreHostKey - SampleCodeG106 = []CodeSample{{` + SampleCodeG106 = []CodeSample{{[]string{` package main import ( "golang.org/x/crypto/ssh" ) func main() { _ = ssh.InsecureIgnoreHostKey() -}`, 1, false}} +}`}, 1}} // SampleCodeG107 - SSRF via http requests with variable url - SampleCodeG107 = []CodeSample{{` + SampleCodeG107 = []CodeSample{{[]string{` package main import ( "net/http" @@ -218,7 +214,7 @@ func main() { panic(err) } fmt.Printf("%s", body) -}`, 1, false}, {` +}`}, 1}, {[]string{` package main import ( @@ -232,17 +228,16 @@ func main() { fmt.Println(err) } fmt.Println(resp.Status) -}`, 0, false}} +}`}, 0}} // SampleCodeG201 - SQL injection via format string SampleCodeG201 = []CodeSample{ - {` + {[]string{` // Format string without proper quoting package main import ( "database/sql" "fmt" "os" - //_ "github.com/mattn/go-sqlite3" ) func main(){ @@ -256,14 +251,13 @@ func main(){ panic(err) } defer rows.Close() -}`, 1, false}, {` +}`}, 1}, {[]string{` // Format string false positive, safe string spec. package main import ( "database/sql" "fmt" "os" - //_ "github.com/mattn/go-sqlite3" ) func main(){ @@ -277,26 +271,24 @@ func main(){ panic(err) } defer rows.Close() -}`, 0, false}, { - ` +}`}, 0}, {[]string{` // Format string false positive package main import ( "database/sql" - //_ "github.com/mattn/go-sqlite3" ) -var staticQuery = "SELECT * FROM foo WHERE age < 32" +const staticQuery = "SELECT * FROM foo WHERE age < 32" func main(){ db, err := sql.Open("sqlite3", ":memory:") if err != nil { - panic(err) + panic(err) } rows, err := db.Query(staticQuery) if err != nil { - panic(err) + panic(err) } defer rows.Close() -}`, 0, false}, {` +}`}, 0}, {[]string{` // Format string false positive, quoted formatter argument. package main import ( @@ -317,15 +309,14 @@ func main(){ panic(err) } defer rows.Close() -}`, 0, false}} +}`}, 0}} // SampleCodeG202 - SQL query string building via string concatenation SampleCodeG202 = []CodeSample{ - {` + {[]string{` package main import ( "database/sql" - //_ "github.com/mattn/go-sqlite3" "os" ) func main(){ @@ -338,12 +329,11 @@ func main(){ panic(err) } defer rows.Close() -}`, 1, false}, {` +}`}, 1}, {[]string{` // false positive package main import ( "database/sql" - //_ "github.com/mattn/go-sqlite3" ) var staticQuery = "SELECT * FROM foo WHERE age < " func main(){ @@ -356,11 +346,10 @@ func main(){ panic(err) } defer rows.Close() -}`, 0, false}, {` +}`}, 0}, {[]string{` package main import ( "database/sql" - //_ "github.com/mattn/go-sqlite3" ) const age = "32" var staticQuery = "SELECT * FROM foo WHERE age < " @@ -375,14 +364,13 @@ func main(){ } defer rows.Close() } -`, 0, false}, {` +`}, 0}, {[]string{` package main const gender = "M" -`, 0, true}, {` +`, ` package main import ( "database/sql" - //_ "github.com/mattn/go-sqlite3" ) const age = "32" var staticQuery = "SELECT * FROM foo WHERE age < " @@ -397,11 +385,11 @@ func main(){ } defer rows.Close() } -`, 0, false}} +`}, 0}} // SampleCodeG203 - Template checks SampleCodeG203 = []CodeSample{ - {` + {[]string{` // We assume that hardcoded template strings are safe as the programmer would // need to be explicitly shooting themselves in the foot (as below) package main @@ -417,7 +405,7 @@ func main() { "Body": template.HTML(""), } t.Execute(os.Stdout, v) -}`, 0, false}, { +}`}, 0}, {[]string{ ` // Using a variable to initialize could potentially be dangerous. Under the // current model this will likely produce some false positives. @@ -435,7 +423,7 @@ func main() { "Body": template.HTML(a), } t.Execute(os.Stdout, v) -}`, 1, false}, { +}`}, 1}, {[]string{ ` package main import ( @@ -451,7 +439,7 @@ func main() { "Body": template.JS(a), } t.Execute(os.Stdout, v) -}`, 1, false}, { +}`}, 1}, {[]string{ ` package main import ( @@ -467,15 +455,15 @@ func main() { "Body": template.URL(a), } t.Execute(os.Stdout, v) -}`, 1, false}} +}`}, 1}} // SampleCodeG204 - Subprocess auditing - SampleCodeG204 = []CodeSample{{` + SampleCodeG204 = []CodeSample{{[]string{` package main import "syscall" func main() { syscall.Exec("/bin/cat", []string{ "/etc/passwd" }, nil) -}`, 1, false}, {` +}`}, 1}, {[]string{` package main import ( "log" @@ -490,7 +478,7 @@ func main() { log.Printf("Waiting for command to finish...") err = cmd.Wait() log.Printf("Command finished with error: %v", err) -}`, 1, false}, {` +}`}, 1}, {[]string{` package main import ( "log" @@ -503,7 +491,7 @@ func main() { log.Fatal(err) } log.Printf("Command finished with error: %v", err) -}`, 1, false}, {` +}`}, 1}, {[]string{` package main import ( "log" @@ -520,20 +508,20 @@ func main() { log.Printf("Waiting for command to finish...") err = cmd.Wait() log.Printf("Command finished with error: %v", err) -}`, 1, false}} +}`}, 1}} // SampleCodeG301 - mkdir permission check - SampleCodeG301 = []CodeSample{{` + SampleCodeG301 = []CodeSample{{[]string{` package main import "os" func main() { os.Mkdir("/tmp/mydir", 0777) os.Mkdir("/tmp/mydir", 0600) os.MkdirAll("/tmp/mydir/mysubidr", 0775) -}`, 2, false}} +}`}, 2}} // SampleCodeG302 - file create / chmod permissions check - SampleCodeG302 = []CodeSample{{` + SampleCodeG302 = []CodeSample{{[]string{` package main import "os" func main() { @@ -541,10 +529,10 @@ func main() { os.Chmod("/tmp/someotherfile", 0600) os.OpenFile("/tmp/thing", os.O_CREATE|os.O_WRONLY, 0666) os.OpenFile("/tmp/thing", os.O_CREATE|os.O_WRONLY, 0600) -}`, 2, false}} +}`}, 2}} // SampleCodeG303 - bad tempfile permissions & hardcoded shared path - SampleCodeG303 = []CodeSample{{` + SampleCodeG303 = []CodeSample{{[]string{` package samples import ( "io/ioutil" @@ -554,10 +542,10 @@ func main() { file1, _ := os.Create("/tmp/demo1") defer file1.Close() ioutil.WriteFile("/tmp/demo2", []byte("This is some data"), 0644) -}`, 2, false}} +}`}, 2}} // SampleCodeG304 - potential file inclusion vulnerability - SampleCodeG304 = []CodeSample{{` + SampleCodeG304 = []CodeSample{{[]string{` package main import ( "os" @@ -572,7 +560,7 @@ if err != nil { } log.Print(body) -}`, 1, false}, {` +}`}, 1}, {[]string{` package main import ( @@ -596,7 +584,7 @@ func main() { fmt.Fprintf(w, "%s", body) }) log.Fatal(http.ListenAndServe(":3000", nil)) -}`, 1, false}, {` +}`}, 1}, {[]string{` package main import ( @@ -612,7 +600,7 @@ import ( log.Printf("Error: %v\n", err) } log.Print(body) - }`, 1, false}, {` + }`}, 1}, {[]string{` package main import ( @@ -636,7 +624,7 @@ func main() { fmt.Printf("Error: %v\n", err) } fmt.Println(string(contents)) -}`, 1, false}, {` +}`}, 1}, {[]string{` package main import ( @@ -655,10 +643,10 @@ func main() { log.Printf("Error: %v\n", err) } log.Print(body) -}`, 1, false}} +}`}, 1}} // SampleCodeG305 - File path traversal when extracting zip archives - SampleCodeG305 = []CodeSample{{` + SampleCodeG305 = []CodeSample{{[]string{` package unzip import ( @@ -703,7 +691,7 @@ func unzip(archive, target string) error { } return nil -}`, 1, false}, {` +}`}, 1}, {[]string{` package unzip import ( @@ -749,11 +737,11 @@ func unzip(archive, target string) error { } return nil -}`, 1, false}} +}`}, 1}} // SampleCodeG401 - Use of weak crypto MD5 SampleCodeG401 = []CodeSample{ - {` + {[]string{` package main import ( "crypto/md5" @@ -774,11 +762,11 @@ func main() { log.Fatal(err) } fmt.Printf("%x", h.Sum(nil)) -}`, 1, false}} +}`}, 1}} // SampleCodeG401b - Use of weak crypto SHA1 SampleCodeG401b = []CodeSample{ - {` + {[]string{` package main import ( "crypto/sha1" @@ -799,10 +787,10 @@ func main() { log.Fatal(err) } fmt.Printf("%x", h.Sum(nil)) -}`, 1, false}} +}`}, 1}} // SampleCodeG402 - TLS settings - SampleCodeG402 = []CodeSample{{` + SampleCodeG402 = []CodeSample{{[]string{` // InsecureSkipVerify package main import ( @@ -820,7 +808,7 @@ func main() { if err != nil { fmt.Println(err) } -}`, 1, false}, { +}`}, 1}, {[]string{ ` // Insecure minimum version package main @@ -838,7 +826,7 @@ func main() { if err != nil { fmt.Println(err) } -}`, 1, false}, {` +}`}, 1}, {[]string{` // Insecure max version package main import ( @@ -856,8 +844,8 @@ func main() { fmt.Println(err) } } -`, 1, false}, { - ` +`}, 1}, { + []string{` // Insecure ciphersuite selection package main import ( @@ -877,11 +865,11 @@ func main() { if err != nil { fmt.Println(err) } -}`, 1, false}} +}`}, 1}} // SampleCodeG403 - weak key strength SampleCodeG403 = []CodeSample{ - {` + {[]string{` package main import ( "crypto/rand" @@ -895,23 +883,23 @@ func main() { fmt.Println(err) } fmt.Println(pvk) -}`, 1, false}} +}`}, 1}} // SampleCodeG404 - weak random number SampleCodeG404 = []CodeSample{ - {` + {[]string{` package main import "crypto/rand" func main() { good, _ := rand.Read(nil) println(good) -}`, 0, false}, {` +}`}, 0}, {[]string{` package main import "math/rand" func main() { bad := rand.Int() println(bad) -}`, 1, false}, {` +}`}, 1}, {[]string{` package main import ( "crypto/rand" @@ -922,11 +910,11 @@ func main() { println(good) i := mrand.Int31() println(i) -}`, 0, false}} +}`}, 0}} // SampleCodeG501 - Blacklisted import MD5 SampleCodeG501 = []CodeSample{ - {` + {[]string{` package main import ( "crypto/md5" @@ -937,11 +925,11 @@ func main() { for _, arg := range os.Args { fmt.Printf("%x - %s\n", md5.Sum([]byte(arg)), arg) } -}`, 1, false}} +}`}, 1}} // SampleCodeG502 - Blacklisted import DES SampleCodeG502 = []CodeSample{ - {` + {[]string{` package main import ( "crypto/cipher" @@ -965,10 +953,10 @@ func main() { stream := cipher.NewCFBEncrypter(block, iv) stream.XORKeyStream(ciphertext[des.BlockSize:], plaintext) fmt.Println("Secret message is: %s", hex.EncodeToString(ciphertext)) -}`, 1, false}} +}`}, 1}} // SampleCodeG503 - Blacklisted import RC4 - SampleCodeG503 = []CodeSample{{` + SampleCodeG503 = []CodeSample{{[]string{` package main import ( "crypto/rc4" @@ -984,10 +972,10 @@ func main() { ciphertext := make([]byte, len(plaintext)) cipher.XORKeyStream(ciphertext, plaintext) fmt.Println("Secret message is: %s", hex.EncodeToString(ciphertext)) -}`, 1, false}} +}`}, 1}} // SampleCodeG504 - Blacklisted import CGI - SampleCodeG504 = []CodeSample{{` + SampleCodeG504 = []CodeSample{{[]string{` package main import ( "net/http/cgi" @@ -995,10 +983,10 @@ import ( ) func main() { cgi.Serve(http.FileServer(http.Dir("/usr/share/doc"))) -}`, 1, false}} +}`}, 1}} // SampleCodeG505 - Blacklisted import SHA1 SampleCodeG505 = []CodeSample{ - {` + {[]string{` package main import ( "crypto/sha1" @@ -1009,13 +997,13 @@ func main() { for _, arg := range os.Args { fmt.Printf("%x - %s\n", sha1.Sum([]byte(arg)), arg) } -}`, 1, false}} +}`}, 1}} // SampleCode601 - Go build tags - SampleCode601 = []CodeSample{{` + SampleCode601 = []CodeSample{{[]string{` // +build test package main func main() { fmt.Println("no package imported error") -}`, 1, false}} +}`}, 1}} )