From 37cada13f352da90265a227527b2f4e5aa7dd900 Mon Sep 17 00:00:00 2001 From: Jon McClintock Date: Thu, 5 Oct 2017 21:32:03 +0000 Subject: [PATCH 01/11] Add support for #excluding specific rules --- core/analyzer.go | 121 +++++++++++++------ core/call_list_test.go | 5 + core/helpers_test.go | 4 + core/issue.go | 1 + rulelist.go | 54 +++++---- rules/big.go | 10 +- rules/big_test.go | 2 +- rules/bind.go | 7 +- rules/bind_test.go | 4 +- rules/blacklist.go | 16 ++- rules/blacklist_test.go | 5 +- rules/errors.go | 10 +- rules/errors_test.go | 8 +- rules/fileperms.go | 10 +- rules/fileperms_test.go | 4 +- rules/hardcoded_credentials.go | 12 +- rules/hardcoded_credentials_test.go | 18 +-- rules/httpoxy_test.go | 2 +- rules/nosec_test.go | 179 +++++++++++++++++++++++++++- rules/rand.go | 7 +- rules/rand_test.go | 6 +- rules/rsa.go | 7 +- rules/rsa_test.go | 2 +- rules/sql.go | 14 ++- rules/sql_test.go | 20 ++-- rules/subproc.go | 16 ++- rules/subproc_test.go | 8 +- rules/tempfiles.go | 7 +- rules/tempfiles_test.go | 2 +- rules/templates.go | 7 +- rules/templates_test.go | 8 +- rules/tls.go | 20 +++- rules/tls_test.go | 10 +- rules/unsafe.go | 10 +- rules/unsafe_test.go | 2 +- rules/weakcrypto.go | 7 +- rules/weakcrypto_test.go | 12 +- 37 files changed, 487 insertions(+), 150 deletions(-) diff --git a/core/analyzer.go b/core/analyzer.go index 116cf78..02753b6 100644 --- a/core/analyzer.go +++ b/core/analyzer.go @@ -25,6 +25,7 @@ import ( "os" "path" "reflect" + "regexp" "strings" ) @@ -54,10 +55,12 @@ type Context struct { Root *ast.File Config map[string]interface{} Imports *ImportInfo + Ignores []map[string]bool } // The Rule interface used by all rules supported by GAS. type Rule interface { + ID() string Match(ast.Node, *Context) (*Issue, error) } @@ -93,7 +96,7 @@ func NewAnalyzer(conf map[string]interface{}, logger *log.Logger) Analyzer { a := Analyzer{ ignoreNosec: conf["ignoreNosec"].(bool), ruleset: make(RuleSet), - context: &Context{nil, nil, nil, nil, nil, nil, nil}, + context: &Context{nil, nil, nil, nil, nil, nil, nil, nil}, logger: logger, Issues: make([]*Issue, 0, 16), Stats: &Metrics{0, 0, 0, 0}, @@ -180,56 +183,98 @@ func (gas *Analyzer) ProcessSource(filename string, source string) error { } // ignore a node (and sub-tree) if it is tagged with a "#nosec" comment -func (gas *Analyzer) ignore(n ast.Node) bool { +func (gas *Analyzer) ignore(n ast.Node) ([]string, bool) { if groups, ok := gas.context.Comments[n]; ok && !gas.ignoreNosec { for _, group := range groups { if strings.Contains(group.Text(), "#nosec") { + return nil, true + } + + if strings.Contains(group.Text(), "#exclude") { gas.Stats.NumNosec++ - return true + + // Pull out the specific rules that are listed to be ignored. + re := regexp.MustCompile("!(G\\d{3})") + matches := re.FindAllStringSubmatch(group.Text(), -1) + + // Find the rule IDs to ignore. + ignores := make([]string, 0) + for _, v := range matches { + ignores = append(ignores, v[1]) + } + return ignores, false } } } - return false + return nil, false } // Visit runs the GAS visitor logic over an AST created by parsing go code. // Rule methods added with AddRule will be invoked as necessary. func (gas *Analyzer) Visit(n ast.Node) ast.Visitor { - if !gas.ignore(n) { - - // Track aliased and initialization imports - if imported, ok := n.(*ast.ImportSpec); ok { - path := strings.Trim(imported.Path.Value, `"`) - if imported.Name != nil { - if imported.Name.Name == "_" { - // Initialization import - gas.context.Imports.InitOnly[path] = true - } else { - // Aliased import - gas.context.Imports.Aliased[path] = imported.Name.Name - } - } - // unsafe is not included in Package.Imports() - if path == "unsafe" { - gas.context.Imports.Imported[path] = path - } - } - - if val, ok := gas.ruleset[reflect.TypeOf(n)]; ok { - for _, rule := range val { - ret, err := rule.Match(n, gas.context) - if err != nil { - file, line := GetLocation(n, gas.context) - file = path.Base(file) - gas.logger.Printf("Rule error: %v => %s (%s:%d)\n", reflect.TypeOf(rule), err, file, line) - } - if ret != nil { - gas.Issues = append(gas.Issues, ret) - gas.Stats.NumFound++ - } - } + // If we've reached the end of this branch, pop off the ignores stack. + if n == nil { + if len(gas.context.Ignores) > 0 { + gas.context.Ignores = gas.context.Ignores[1:] } return gas } - return nil + + // Get any new rule exclusions. + ignoredRules, ignoreAll := gas.ignore(n) + if ignoreAll { + return nil + } + + // Now create the union of exclusions. + ignores := make(map[string]bool, 0) + if len(gas.context.Ignores) > 0 { + for k, v := range gas.context.Ignores[0] { + ignores[k] = v + } + } + + for _, v := range ignoredRules { + ignores[v] = true + } + + // Push the new set onto the stack. + gas.context.Ignores = append([]map[string]bool{ignores}, gas.context.Ignores...) + + // Track aliased and initialization imports + if imported, ok := n.(*ast.ImportSpec); ok { + path := strings.Trim(imported.Path.Value, `"`) + if imported.Name != nil { + if imported.Name.Name == "_" { + // Initialization import + gas.context.Imports.InitOnly[path] = true + } else { + // Aliased import + gas.context.Imports.Aliased[path] = imported.Name.Name + } + } + // unsafe is not included in Package.Imports() + if path == "unsafe" { + gas.context.Imports.Imported[path] = path + } + } + + if val, ok := gas.ruleset[reflect.TypeOf(n)]; ok { + for _, rule := range val { + if _, ok := ignores[rule.ID()]; ok { + continue + } + ret, err := rule.Match(n, gas.context) + if err != nil { + file, line := GetLocation(n, gas.context) + file = path.Base(file) + gas.logger.Printf("Rule error: %v => %s (%s:%d)\n", reflect.TypeOf(rule), err, file, line) + } + if ret != nil { + gas.Issues = append(gas.Issues, ret) + gas.Stats.NumFound++ + } + } + } + return gas } diff --git a/core/call_list_test.go b/core/call_list_test.go index ef58293..25dfeef 100644 --- a/core/call_list_test.go +++ b/core/call_list_test.go @@ -11,6 +11,10 @@ type callListRule struct { matched int } +func (r *callListRule) ID() string { + return r.MetaData.ID +} + func (r *callListRule) Match(n ast.Node, c *Context) (gi *Issue, err error) { if r.callList.ContainsCallExpr(n, c) { r.matched += 1 @@ -25,6 +29,7 @@ func TestCallListContainsCallExpr(t *testing.T) { calls.AddAll("bytes.Buffer", "Write", "WriteTo") rule := &callListRule{ MetaData: MetaData{ + ID: "TEST", Severity: Low, Confidence: Low, What: "A dummy rule", diff --git a/core/helpers_test.go b/core/helpers_test.go index 1a7bcda..f90047c 100644 --- a/core/helpers_test.go +++ b/core/helpers_test.go @@ -16,6 +16,10 @@ type dummyRule struct { matched int } +func (r *dummyRule) ID() string { + return r.MetaData.ID +} + func (r *dummyRule) Match(n ast.Node, c *Context) (gi *Issue, err error) { if callexpr, matched := r.callback(n, c, r.pkgOrType, r.funcsOrMethods...); matched { r.matched += 1 diff --git a/core/issue.go b/core/issue.go index 22c5044..f368fd7 100644 --- a/core/issue.go +++ b/core/issue.go @@ -42,6 +42,7 @@ type Issue struct { // MetaData is embedded in all GAS rules. The Severity, Confidence and What message // will be passed tbhrough to reported issues. type MetaData struct { + ID string Severity Score Confidence Score What string diff --git a/rulelist.go b/rulelist.go index 285c1a1..e6ce155 100644 --- a/rulelist.go +++ b/rulelist.go @@ -22,43 +22,49 @@ import ( ) type RuleInfo struct { + id string description string - build func(map[string]interface{}) (gas.Rule, []ast.Node) + build func(string, map[string]interface{}) (gas.Rule, []ast.Node) } // GetFullRuleList get the full list of all rules available to GAS func GetFullRuleList() map[string]RuleInfo { - return map[string]RuleInfo{ + rules := []RuleInfo{ // misc - "G101": RuleInfo{"Look for hardcoded credentials", rules.NewHardcodedCredentials}, - "G102": RuleInfo{"Bind to all interfaces", rules.NewBindsToAllNetworkInterfaces}, - "G103": RuleInfo{"Audit the use of unsafe block", rules.NewUsingUnsafe}, - "G104": RuleInfo{"Audit errors not checked", rules.NewNoErrorCheck}, - "G105": RuleInfo{"Audit the use of big.Exp function", rules.NewUsingBigExp}, + RuleInfo{"G101", "Look for hardcoded credentials", rules.NewHardcodedCredentials}, + RuleInfo{"G102", "Bind to all interfaces", rules.NewBindsToAllNetworkInterfaces}, + RuleInfo{"G103", "Audit the use of unsafe block", rules.NewUsingUnsafe}, + RuleInfo{"G104", "Audit errors not checked", rules.NewNoErrorCheck}, + RuleInfo{"G105", "Audit the use of big.Exp function", rules.NewUsingBigExp}, // injection - "G201": RuleInfo{"SQL query construction using format string", rules.NewSqlStrFormat}, - "G202": RuleInfo{"SQL query construction using string concatenation", rules.NewSqlStrConcat}, - "G203": RuleInfo{"Use of unescaped data in HTML templates", rules.NewTemplateCheck}, - "G204": RuleInfo{"Audit use of command execution", rules.NewSubproc}, + RuleInfo{"G201", "SQL query construction using format string", rules.NewSqlStrFormat}, + RuleInfo{"G202", "SQL query construction using string concatenation", rules.NewSqlStrConcat}, + RuleInfo{"G203", "Use of unescaped data in HTML templates", rules.NewTemplateCheck}, + RuleInfo{"G204", "Audit use of command execution", rules.NewSubproc}, // filesystem - "G301": RuleInfo{"Poor file permissions used when creating a directory", rules.NewMkdirPerms}, - "G302": RuleInfo{"Poor file permisions used when creation file or using chmod", rules.NewFilePerms}, - "G303": RuleInfo{"Creating tempfile using a predictable path", rules.NewBadTempFile}, + RuleInfo{"G301", "Poor file permissions used when creating a directory", rules.NewMkdirPerms}, + RuleInfo{"G302", "Poor file permisions used when creation file or using chmod", rules.NewFilePerms}, + RuleInfo{"G303", "Creating tempfile using a predictable path", rules.NewBadTempFile}, // crypto - "G401": RuleInfo{"Detect the usage of DES, RC4, or MD5", rules.NewUsesWeakCryptography}, - "G402": RuleInfo{"Look for bad TLS connection settings", rules.NewIntermediateTlsCheck}, - "G403": RuleInfo{"Ensure minimum RSA key length of 2048 bits", rules.NewWeakKeyStrength}, - "G404": RuleInfo{"Insecure random number source (rand)", rules.NewWeakRandCheck}, + RuleInfo{"G401", "Detect the usage of DES, RC4, or MD5", rules.NewUsesWeakCryptography}, + RuleInfo{"G402", "Look for bad TLS connection settings", rules.NewIntermediateTlsCheck}, + RuleInfo{"G403", "Ensure minimum RSA key length of 2048 bits", rules.NewWeakKeyStrength}, + RuleInfo{"G404", "Insecure random number source (rand)", rules.NewWeakRandCheck}, // blacklist - "G501": RuleInfo{"Import blacklist: crypto/md5", rules.NewBlacklist_crypto_md5}, - "G502": RuleInfo{"Import blacklist: crypto/des", rules.NewBlacklist_crypto_des}, - "G503": RuleInfo{"Import blacklist: crypto/rc4", rules.NewBlacklist_crypto_rc4}, - "G504": RuleInfo{"Import blacklist: net/http/cgi", rules.NewBlacklist_net_http_cgi}, + RuleInfo{"G501", "Import blacklist: crypto/md5", rules.NewBlacklist_crypto_md5}, + RuleInfo{"G502", "Import blacklist: crypto/des", rules.NewBlacklist_crypto_des}, + RuleInfo{"G503", "Import blacklist: crypto/rc4", rules.NewBlacklist_crypto_rc4}, + RuleInfo{"G504", "Import blacklist: net/http/cgi", rules.NewBlacklist_net_http_cgi}, } + ruleMap := make(map[string]RuleInfo) + for _, v := range rules { + ruleMap[v.id] = v + } + return ruleMap } func AddRules(analyzer *gas.Analyzer, conf map[string]interface{}) { @@ -85,7 +91,7 @@ func AddRules(analyzer *gas.Analyzer, conf map[string]interface{}) { delete(all, v) } - for _, v := range all { - analyzer.AddRule(v.build(conf)) + for k, v := range all { + analyzer.AddRule(v.build(k, conf)) } } diff --git a/rules/big.go b/rules/big.go index 8ac0d42..dbc4cfe 100644 --- a/rules/big.go +++ b/rules/big.go @@ -15,8 +15,9 @@ package rules import ( - gas "github.com/GoASTScanner/gas/core" "go/ast" + + gas "github.com/GoASTScanner/gas/core" ) type UsingBigExp struct { @@ -25,17 +26,22 @@ type UsingBigExp struct { calls []string } +func (r *UsingBigExp) ID() string { + return r.MetaData.ID +} + func (r *UsingBigExp) Match(n ast.Node, c *gas.Context) (gi *gas.Issue, err error) { if _, matched := gas.MatchCallByType(n, c, r.pkg, r.calls...); matched { return gas.NewIssue(c, n, r.What, r.Severity, r.Confidence), nil } return nil, nil } -func NewUsingBigExp(conf map[string]interface{}) (gas.Rule, []ast.Node) { +func NewUsingBigExp(id string, conf map[string]interface{}) (gas.Rule, []ast.Node) { return &UsingBigExp{ pkg: "*math/big.Int", calls: []string{"Exp"}, MetaData: gas.MetaData{ + ID: id, What: "Use of math/big.Int.Exp function should be audited for modulus == 0", Severity: gas.Low, Confidence: gas.High, diff --git a/rules/big_test.go b/rules/big_test.go index b533e66..cb79416 100644 --- a/rules/big_test.go +++ b/rules/big_test.go @@ -23,7 +23,7 @@ import ( func TestBigExp(t *testing.T) { config := map[string]interface{}{"ignoreNosec": false} analyzer := gas.NewAnalyzer(config, nil) - analyzer.AddRule(NewUsingBigExp(config)) + analyzer.AddRule(NewUsingBigExp("TEST", config)) issues := gasTestRunner(` package main diff --git a/rules/bind.go b/rules/bind.go index ba91ba6..c1d85e0 100644 --- a/rules/bind.go +++ b/rules/bind.go @@ -28,6 +28,10 @@ type BindsToAllNetworkInterfaces struct { pattern *regexp.Regexp } +func (r *BindsToAllNetworkInterfaces) ID() string { + return r.MetaData.ID +} + func (r *BindsToAllNetworkInterfaces) Match(n ast.Node, c *gas.Context) (gi *gas.Issue, err error) { if node := gas.MatchCall(n, r.call); node != nil { if arg, err := gas.GetString(node.Args[1]); err == nil { @@ -39,11 +43,12 @@ func (r *BindsToAllNetworkInterfaces) Match(n ast.Node, c *gas.Context) (gi *gas return } -func NewBindsToAllNetworkInterfaces(conf map[string]interface{}) (gas.Rule, []ast.Node) { +func NewBindsToAllNetworkInterfaces(id string, conf map[string]interface{}) (gas.Rule, []ast.Node) { return &BindsToAllNetworkInterfaces{ call: regexp.MustCompile(`^(net|tls)\.Listen$`), pattern: regexp.MustCompile(`^(0.0.0.0|:).*$`), MetaData: gas.MetaData{ + ID: id, Severity: gas.Medium, Confidence: gas.High, What: "Binds to all network interfaces", diff --git a/rules/bind_test.go b/rules/bind_test.go index 16bc389..59bf629 100644 --- a/rules/bind_test.go +++ b/rules/bind_test.go @@ -23,7 +23,7 @@ import ( func TestBind0000(t *testing.T) { config := map[string]interface{}{"ignoreNosec": false} analyzer := gas.NewAnalyzer(config, nil) - analyzer.AddRule(NewBindsToAllNetworkInterfaces(config)) + analyzer.AddRule(NewBindsToAllNetworkInterfaces("TEST", config)) issues := gasTestRunner(` package main @@ -45,7 +45,7 @@ func TestBind0000(t *testing.T) { func TestBindEmptyHost(t *testing.T) { config := map[string]interface{}{"ignoreNosec": false} analyzer := gas.NewAnalyzer(config, nil) - analyzer.AddRule(NewBindsToAllNetworkInterfaces(config)) + analyzer.AddRule(NewBindsToAllNetworkInterfaces("TEST", config)) issues := gasTestRunner(` package main diff --git a/rules/blacklist.go b/rules/blacklist.go index 747eb4b..74c3b9e 100644 --- a/rules/blacklist.go +++ b/rules/blacklist.go @@ -25,6 +25,10 @@ type BlacklistImport struct { Path string } +func (r *BlacklistImport) ID() string { + return r.MetaData.ID +} + func (r *BlacklistImport) Match(n ast.Node, c *gas.Context) (gi *gas.Issue, err error) { if node, ok := n.(*ast.ImportSpec); ok { if r.Path == node.Path.Value && node.Name.String() != "_" { @@ -34,9 +38,10 @@ func (r *BlacklistImport) Match(n ast.Node, c *gas.Context) (gi *gas.Issue, err return nil, nil } -func NewBlacklist_crypto_md5(conf map[string]interface{}) (gas.Rule, []ast.Node) { +func NewBlacklist_crypto_md5(id string, conf map[string]interface{}) (gas.Rule, []ast.Node) { return &BlacklistImport{ MetaData: gas.MetaData{ + ID: id, Severity: gas.High, Confidence: gas.High, What: "Use of weak cryptographic primitive", @@ -45,9 +50,10 @@ func NewBlacklist_crypto_md5(conf map[string]interface{}) (gas.Rule, []ast.Node) }, []ast.Node{(*ast.ImportSpec)(nil)} } -func NewBlacklist_crypto_des(conf map[string]interface{}) (gas.Rule, []ast.Node) { +func NewBlacklist_crypto_des(id string, conf map[string]interface{}) (gas.Rule, []ast.Node) { return &BlacklistImport{ MetaData: gas.MetaData{ + ID: id, Severity: gas.High, Confidence: gas.High, What: "Use of weak cryptographic primitive", @@ -56,9 +62,10 @@ func NewBlacklist_crypto_des(conf map[string]interface{}) (gas.Rule, []ast.Node) }, []ast.Node{(*ast.ImportSpec)(nil)} } -func NewBlacklist_crypto_rc4(conf map[string]interface{}) (gas.Rule, []ast.Node) { +func NewBlacklist_crypto_rc4(id string, conf map[string]interface{}) (gas.Rule, []ast.Node) { return &BlacklistImport{ MetaData: gas.MetaData{ + ID: id, Severity: gas.High, Confidence: gas.High, What: "Use of weak cryptographic primitive", @@ -67,9 +74,10 @@ func NewBlacklist_crypto_rc4(conf map[string]interface{}) (gas.Rule, []ast.Node) }, []ast.Node{(*ast.ImportSpec)(nil)} } -func NewBlacklist_net_http_cgi(conf map[string]interface{}) (gas.Rule, []ast.Node) { +func NewBlacklist_net_http_cgi(id string, conf map[string]interface{}) (gas.Rule, []ast.Node) { return &BlacklistImport{ MetaData: gas.MetaData{ + ID: id, Severity: gas.High, Confidence: gas.High, What: "Go versions < 1.6.3 are vulnerable to Httpoxy attack: (CVE-2016-5386)", diff --git a/rules/blacklist_test.go b/rules/blacklist_test.go index 110afd4..fa3536e 100644 --- a/rules/blacklist_test.go +++ b/rules/blacklist_test.go @@ -13,8 +13,9 @@ package rules import ( - gas "github.com/GoASTScanner/gas/core" "testing" + + gas "github.com/GoASTScanner/gas/core" ) const initOnlyImportSrc = ` @@ -33,7 +34,7 @@ func main() { func TestInitOnlyImport(t *testing.T) { config := map[string]interface{}{"ignoreNosec": false} analyzer := gas.NewAnalyzer(config, nil) - analyzer.AddRule(NewBlacklist_crypto_md5(config)) + analyzer.AddRule(NewBlacklist_crypto_md5("TEST", config)) issues := gasTestRunner(initOnlyImportSrc, analyzer) checkTestResults(t, issues, 0, "") } diff --git a/rules/errors.go b/rules/errors.go index 2bf61c9..5cdb509 100644 --- a/rules/errors.go +++ b/rules/errors.go @@ -15,9 +15,10 @@ package rules import ( - gas "github.com/GoASTScanner/gas/core" "go/ast" "go/types" + + gas "github.com/GoASTScanner/gas/core" ) type NoErrorCheck struct { @@ -25,6 +26,10 @@ type NoErrorCheck struct { whitelist gas.CallList } +func (r *NoErrorCheck) ID() string { + return r.MetaData.ID +} + func returnsError(callExpr *ast.CallExpr, ctx *gas.Context) int { if tv := ctx.Info.TypeOf(callExpr); tv != nil { switch t := tv.(type) { @@ -69,7 +74,7 @@ func (r *NoErrorCheck) Match(n ast.Node, ctx *gas.Context) (*gas.Issue, error) { return nil, nil } -func NewNoErrorCheck(conf map[string]interface{}) (gas.Rule, []ast.Node) { +func NewNoErrorCheck(id string, conf map[string]interface{}) (gas.Rule, []ast.Node) { // TODO(gm) Come up with sensible defaults here. Or flip it to use a // black list instead. @@ -87,6 +92,7 @@ func NewNoErrorCheck(conf map[string]interface{}) (gas.Rule, []ast.Node) { } return &NoErrorCheck{ MetaData: gas.MetaData{ + ID: id, Severity: gas.Low, Confidence: gas.High, What: "Errors unhandled.", diff --git a/rules/errors_test.go b/rules/errors_test.go index a0d82c9..a71e163 100644 --- a/rules/errors_test.go +++ b/rules/errors_test.go @@ -23,7 +23,7 @@ import ( func TestErrorsMulti(t *testing.T) { config := map[string]interface{}{"ignoreNosec": false} analyzer := gas.NewAnalyzer(config, nil) - analyzer.AddRule(NewNoErrorCheck(config)) + analyzer.AddRule(NewNoErrorCheck("TEST", config)) issues := gasTestRunner( `package main @@ -47,7 +47,7 @@ func TestErrorsMulti(t *testing.T) { func TestErrorsSingle(t *testing.T) { config := map[string]interface{}{"ignoreNosec": false} analyzer := gas.NewAnalyzer(config, nil) - analyzer.AddRule(NewNoErrorCheck(config)) + analyzer.AddRule(NewNoErrorCheck("TEST", config)) issues := gasTestRunner( `package main @@ -81,7 +81,7 @@ func TestErrorsSingle(t *testing.T) { func TestErrorsGood(t *testing.T) { config := map[string]interface{}{"ignoreNosec": false} analyzer := gas.NewAnalyzer(config, nil) - analyzer.AddRule(NewNoErrorCheck(config)) + analyzer.AddRule(NewNoErrorCheck("TEST", config)) issues := gasTestRunner( `package main @@ -110,7 +110,7 @@ func TestErrorsWhitelisted(t *testing.T) { }, } analyzer := gas.NewAnalyzer(config, nil) - analyzer.AddRule(NewNoErrorCheck(config)) + analyzer.AddRule(NewNoErrorCheck("TEST", config)) source := `package main import ( "io" diff --git a/rules/fileperms.go b/rules/fileperms.go index 101c7e2..c9d1b44 100644 --- a/rules/fileperms.go +++ b/rules/fileperms.go @@ -29,6 +29,10 @@ type FilePermissions struct { calls []string } +func (r *FilePermissions) ID() string { + return r.MetaData.ID +} + func getConfiguredMode(conf map[string]interface{}, configKey string, defaultMode int64) int64 { var mode int64 = defaultMode if value, ok := conf[configKey]; ok { @@ -56,13 +60,14 @@ func (r *FilePermissions) Match(n ast.Node, c *gas.Context) (*gas.Issue, error) return nil, nil } -func NewFilePerms(conf map[string]interface{}) (gas.Rule, []ast.Node) { +func NewFilePerms(id string, conf map[string]interface{}) (gas.Rule, []ast.Node) { mode := getConfiguredMode(conf, "G302", 0600) return &FilePermissions{ mode: mode, pkg: "os", calls: []string{"OpenFile", "Chmod"}, MetaData: gas.MetaData{ + ID: id, Severity: gas.Medium, Confidence: gas.High, What: fmt.Sprintf("Expect file permissions to be %#o or less", mode), @@ -70,13 +75,14 @@ func NewFilePerms(conf map[string]interface{}) (gas.Rule, []ast.Node) { }, []ast.Node{(*ast.CallExpr)(nil)} } -func NewMkdirPerms(conf map[string]interface{}) (gas.Rule, []ast.Node) { +func NewMkdirPerms(id string, conf map[string]interface{}) (gas.Rule, []ast.Node) { mode := getConfiguredMode(conf, "G301", 0700) return &FilePermissions{ mode: mode, pkg: "os", calls: []string{"Mkdir", "MkdirAll"}, MetaData: gas.MetaData{ + ID: id, Severity: gas.Medium, Confidence: gas.High, What: fmt.Sprintf("Expect directory permissions to be %#o or less", mode), diff --git a/rules/fileperms_test.go b/rules/fileperms_test.go index 278c29e..2728b92 100644 --- a/rules/fileperms_test.go +++ b/rules/fileperms_test.go @@ -23,7 +23,7 @@ import ( func TestChmod(t *testing.T) { config := map[string]interface{}{"ignoreNosec": false} analyzer := gas.NewAnalyzer(config, nil) - analyzer.AddRule(NewFilePerms(config)) + analyzer.AddRule(NewFilePerms("TEST", config)) issues := gasTestRunner(` package main @@ -41,7 +41,7 @@ func TestChmod(t *testing.T) { func TestMkdir(t *testing.T) { config := map[string]interface{}{"ignoreNosec": false} analyzer := gas.NewAnalyzer(config, nil) - analyzer.AddRule(NewMkdirPerms(config)) + analyzer.AddRule(NewMkdirPerms("TEST", config)) issues := gasTestRunner(` package main diff --git a/rules/hardcoded_credentials.go b/rules/hardcoded_credentials.go index 1b4b85a..0f7fcee 100644 --- a/rules/hardcoded_credentials.go +++ b/rules/hardcoded_credentials.go @@ -15,12 +15,13 @@ package rules import ( - gas "github.com/GoASTScanner/gas/core" "go/ast" "go/token" "regexp" - "github.com/nbutton23/zxcvbn-go" + gas "github.com/GoASTScanner/gas/core" + zxcvbn "github.com/nbutton23/zxcvbn-go" + "strconv" ) @@ -33,6 +34,10 @@ type Credentials struct { ignoreEntropy bool } +func (r *Credentials) ID() string { + return r.MetaData.ID +} + func truncate(s string, n int) string { if n > len(s) { return s @@ -100,7 +105,7 @@ func (r *Credentials) matchGenDecl(decl *ast.GenDecl, ctx *gas.Context) (*gas.Is return nil, nil } -func NewHardcodedCredentials(conf map[string]interface{}) (gas.Rule, []ast.Node) { +func NewHardcodedCredentials(id string, conf map[string]interface{}) (gas.Rule, []ast.Node) { pattern := `(?i)passwd|pass|password|pwd|secret|token` entropyThreshold := 80.0 perCharThreshold := 3.0 @@ -140,6 +145,7 @@ func NewHardcodedCredentials(conf map[string]interface{}) (gas.Rule, []ast.Node) ignoreEntropy: ignoreEntropy, truncate: truncateString, MetaData: gas.MetaData{ + ID: id, What: "Potential hardcoded credentials", Confidence: gas.Low, Severity: gas.High, diff --git a/rules/hardcoded_credentials_test.go b/rules/hardcoded_credentials_test.go index 63f3db1..a29b955 100644 --- a/rules/hardcoded_credentials_test.go +++ b/rules/hardcoded_credentials_test.go @@ -23,7 +23,7 @@ import ( func TestHardcoded(t *testing.T) { config := map[string]interface{}{"ignoreNosec": false} analyzer := gas.NewAnalyzer(config, nil) - analyzer.AddRule(NewHardcodedCredentials(config)) + analyzer.AddRule(NewHardcodedCredentials("TEST", config)) issues := gasTestRunner( ` @@ -43,7 +43,7 @@ func TestHardcoded(t *testing.T) { func TestHardcodedWithEntropy(t *testing.T) { config := map[string]interface{}{"ignoreNosec": false} analyzer := gas.NewAnalyzer(config, nil) - analyzer.AddRule(NewHardcodedCredentials(config)) + analyzer.AddRule(NewHardcodedCredentials("TEST", config)) issues := gasTestRunner( ` @@ -68,7 +68,7 @@ func TestHardcodedIgnoreEntropy(t *testing.T) { }, } analyzer := gas.NewAnalyzer(config, nil) - analyzer.AddRule(NewHardcodedCredentials(config)) + analyzer.AddRule(NewHardcodedCredentials("TEST", config)) issues := gasTestRunner( ` @@ -88,7 +88,7 @@ func TestHardcodedIgnoreEntropy(t *testing.T) { func TestHardcodedGlobalVar(t *testing.T) { config := map[string]interface{}{"ignoreNosec": false} analyzer := gas.NewAnalyzer(config, nil) - analyzer.AddRule(NewHardcodedCredentials(config)) + analyzer.AddRule(NewHardcodedCredentials("TEST", config)) issues := gasTestRunner(` package samples @@ -108,7 +108,7 @@ func TestHardcodedGlobalVar(t *testing.T) { func TestHardcodedConstant(t *testing.T) { config := map[string]interface{}{"ignoreNosec": false} analyzer := gas.NewAnalyzer(config, nil) - analyzer.AddRule(NewHardcodedCredentials(config)) + analyzer.AddRule(NewHardcodedCredentials("TEST", config)) issues := gasTestRunner(` package samples @@ -128,7 +128,7 @@ func TestHardcodedConstant(t *testing.T) { func TestHardcodedConstantMulti(t *testing.T) { config := map[string]interface{}{"ignoreNosec": false} analyzer := gas.NewAnalyzer(config, nil) - analyzer.AddRule(NewHardcodedCredentials(config)) + analyzer.AddRule(NewHardcodedCredentials("TEST", config)) issues := gasTestRunner(` package samples @@ -150,7 +150,7 @@ func TestHardcodedConstantMulti(t *testing.T) { func TestHardecodedVarsNotAssigned(t *testing.T) { config := map[string]interface{}{"ignoreNosec": false} analyzer := gas.NewAnalyzer(config, nil) - analyzer.AddRule(NewHardcodedCredentials(config)) + analyzer.AddRule(NewHardcodedCredentials("TEST", config)) issues := gasTestRunner(` package main var password string @@ -163,7 +163,7 @@ func TestHardecodedVarsNotAssigned(t *testing.T) { func TestHardcodedConstInteger(t *testing.T) { config := map[string]interface{}{"ignoreNosec": false} analyzer := gas.NewAnalyzer(config, nil) - analyzer.AddRule(NewHardcodedCredentials(config)) + analyzer.AddRule(NewHardcodedCredentials("TEST", config)) issues := gasTestRunner(` package main @@ -180,7 +180,7 @@ func TestHardcodedConstInteger(t *testing.T) { func TestHardcodedConstString(t *testing.T) { config := map[string]interface{}{"ignoreNosec": false} analyzer := gas.NewAnalyzer(config, nil) - analyzer.AddRule(NewHardcodedCredentials(config)) + analyzer.AddRule(NewHardcodedCredentials("TEST", config)) issues := gasTestRunner(` package main diff --git a/rules/httpoxy_test.go b/rules/httpoxy_test.go index b666fdf..cfd5631 100644 --- a/rules/httpoxy_test.go +++ b/rules/httpoxy_test.go @@ -23,7 +23,7 @@ import ( func TestHttpoxy(t *testing.T) { config := map[string]interface{}{"ignoreNosec": false} analyzer := gas.NewAnalyzer(config, nil) - analyzer.AddRule(NewBlacklist_net_http_cgi(config)) + analyzer.AddRule(NewBlacklist_net_http_cgi("TEST", config)) issues := gasTestRunner(` package main diff --git a/rules/nosec_test.go b/rules/nosec_test.go index dbbc107..377c6f7 100644 --- a/rules/nosec_test.go +++ b/rules/nosec_test.go @@ -23,7 +23,7 @@ import ( func TestNosec(t *testing.T) { config := map[string]interface{}{"ignoreNosec": false} analyzer := gas.NewAnalyzer(config, nil) - analyzer.AddRule(NewSubproc(config)) + analyzer.AddRule(NewSubproc("G001", config)) issues := gasTestRunner( `package main @@ -43,7 +43,7 @@ func TestNosec(t *testing.T) { func TestNosecBlock(t *testing.T) { config := map[string]interface{}{"ignoreNosec": false} analyzer := gas.NewAnalyzer(config, nil) - analyzer.AddRule(NewSubproc(config)) + analyzer.AddRule(NewSubproc("G001", config)) issues := gasTestRunner( `package main @@ -66,7 +66,7 @@ func TestNosecBlock(t *testing.T) { func TestNosecIgnore(t *testing.T) { config := map[string]interface{}{"ignoreNosec": true} analyzer := gas.NewAnalyzer(config, nil) - analyzer.AddRule(NewSubproc(config)) + analyzer.AddRule(NewSubproc("G001", config)) issues := gasTestRunner( `package main @@ -83,3 +83,176 @@ func TestNosecIgnore(t *testing.T) { checkTestResults(t, issues, 1, "Subprocess launching with variable.") } + +func TestNosecExcludeOne(t *testing.T) { + config := map[string]interface{}{"ignoreNosec": false} + analyzer := gas.NewAnalyzer(config, nil) + analyzer.AddRule(NewSubproc("G001", config)) + + issues := gasTestRunner( + `package main + import ( + "os" + "os/exec" + ) + + func main() { + cmd := exec.Command("sh", "-c", os.Getenv("BLAH")) // #exclude !G001 + cmd.Run() + }`, analyzer) + + checkTestResults(t, issues, 0, "None") +} + +func TestNosecExcludeOneNoMatch(t *testing.T) { + config := map[string]interface{}{"ignoreNosec": false} + analyzer := gas.NewAnalyzer(config, nil) + analyzer.AddRule(NewSubproc("G001", config)) + + issues := gasTestRunner( + `package main + import ( + "os" + "os/exec" + ) + + func main() { + cmd := exec.Command("sh", "-c", os.Getenv("BLAH")) // #exclude !G002 + cmd.Run() + }`, analyzer) + + checkTestResults(t, issues, 1, "Subprocess launching with variable.") +} + +func TestNosecExcludeOneMatchNextLine(t *testing.T) { + config := map[string]interface{}{"ignoreNosec": false} + analyzer := gas.NewAnalyzer(config, nil) + analyzer.AddRule(NewSubproc("G001", config)) + + issues := gasTestRunner( + `package main + import ( + "os" + "os/exec" + ) + + func main() { + cmd := exec.Command("sh", "-c", os.Getenv("FOO")) // #exclude !G001 + cmd = exec.Command("sh", "-c", os.Getenv("BAR")) + cmd.Run() + }`, analyzer) + + checkTestResults(t, issues, 1, "Subprocess launching with variable.") +} + +func TestNosecBlockExcludeOne(t *testing.T) { + config := map[string]interface{}{"ignoreNosec": false} + analyzer := gas.NewAnalyzer(config, nil) + analyzer.AddRule(NewSubproc("G001", config)) + + issues := gasTestRunner( + `package main + import ( + "os" + "os/exec" + ) + + func main() { + // #exclude !G001 + if true { + cmd := exec.Command("sh", "-c", os.Getenv("BLAH")) + cmd.Run() + } + }`, analyzer) + + checkTestResults(t, issues, 0, "None") +} + +func TestNosecBlockExcludeOneNoMatch(t *testing.T) { + config := map[string]interface{}{"ignoreNosec": false} + analyzer := gas.NewAnalyzer(config, nil) + analyzer.AddRule(NewSubproc("G001", config)) + + issues := gasTestRunner( + `package main + import ( + "os" + "os/exec" + ) + + func main() { + // #exclude !G002 + if true { + cmd := exec.Command("sh", "-c", os.Getenv("BLAH")) + cmd.Run() + } + }`, analyzer) + + checkTestResults(t, issues, 1, "Subprocess launching with variable.") +} + +func TestNosecExcludeTwoNoMatch(t *testing.T) { + config := map[string]interface{}{"ignoreNosec": false} + analyzer := gas.NewAnalyzer(config, nil) + analyzer.AddRule(NewSubproc("G001", config)) + analyzer.AddRule(NewWeakRandCheck("G002", config)) + + issues := gasTestRunner( + `package main + import ( + "math/rand" + "os" + "os/exec" + ) + + func main() { + cmd := exec.Command("sh", "-c", os.Getenv("BLAH"), string(rand.Int())) // #exclude !G003 !G004 + cmd.Run() + }`, analyzer) + + checkTestResults(t, issues, 2, "") +} + +func TestNosecExcludeTwoOneMatch(t *testing.T) { + config := map[string]interface{}{"ignoreNosec": false} + analyzer := gas.NewAnalyzer(config, nil) + analyzer.AddRule(NewSubproc("G001", config)) + analyzer.AddRule(NewWeakRandCheck("G002", config)) + + issues := gasTestRunner( + `package main + import ( + "math/rand" + "os" + "os/exec" + ) + + func main() { + cmd := exec.Command("sh", "-c", os.Getenv("BLAH"), string(rand.Int())) // #exclude !G001 !G004 + cmd.Run() + }`, analyzer) + + checkTestResults(t, issues, 1, "Use of weak random number generator") +} + +func TestNosecExcludeTwoBothMatch(t *testing.T) { + config := map[string]interface{}{"ignoreNosec": false} + analyzer := gas.NewAnalyzer(config, nil) + analyzer.AddRule(NewSubproc("G001", config)) + analyzer.AddRule(NewWeakRandCheck("G002", config)) + + issues := gasTestRunner( + `package main + import ( + "math/rand" + "os" + "os/exec" + ) + + func main() { + cmd := exec.Command("sh", "-c", os.Getenv("BLAH"), string(rand.Int())) // #exclude !G001 !G002 + cmd.Run() + }`, analyzer) + + checkTestResults(t, issues, 0, "No issues") +} diff --git a/rules/rand.go b/rules/rand.go index 9cc99e4..b267c2d 100644 --- a/rules/rand.go +++ b/rules/rand.go @@ -26,6 +26,10 @@ type WeakRand struct { packagePath string } +func (r *WeakRand) ID() string { + return r.MetaData.ID +} + func (w *WeakRand) Match(n ast.Node, c *gas.Context) (*gas.Issue, error) { for _, funcName := range w.funcNames { if _, matched := gas.MatchCallByPackage(n, c, w.packagePath, funcName); matched { @@ -36,11 +40,12 @@ func (w *WeakRand) Match(n ast.Node, c *gas.Context) (*gas.Issue, error) { return nil, nil } -func NewWeakRandCheck(conf map[string]interface{}) (gas.Rule, []ast.Node) { +func NewWeakRandCheck(id string, conf map[string]interface{}) (gas.Rule, []ast.Node) { return &WeakRand{ funcNames: []string{"Read", "Int"}, packagePath: "math/rand", MetaData: gas.MetaData{ + ID: id, Severity: gas.High, Confidence: gas.Medium, What: "Use of weak random number generator (math/rand instead of crypto/rand)", diff --git a/rules/rand_test.go b/rules/rand_test.go index d6de104..01ac1a2 100644 --- a/rules/rand_test.go +++ b/rules/rand_test.go @@ -23,7 +23,7 @@ import ( func TestRandOk(t *testing.T) { config := map[string]interface{}{"ignoreNosec": false} analyzer := gas.NewAnalyzer(config, nil) - analyzer.AddRule(NewWeakRandCheck(config)) + analyzer.AddRule(NewWeakRandCheck("TEST", config)) issues := gasTestRunner( ` @@ -42,7 +42,7 @@ func TestRandOk(t *testing.T) { func TestRandBad(t *testing.T) { config := map[string]interface{}{"ignoreNosec": false} analyzer := gas.NewAnalyzer(config, nil) - analyzer.AddRule(NewWeakRandCheck(config)) + analyzer.AddRule(NewWeakRandCheck("TEST", config)) issues := gasTestRunner( ` @@ -62,7 +62,7 @@ func TestRandBad(t *testing.T) { func TestRandRenamed(t *testing.T) { config := map[string]interface{}{"ignoreNosec": false} analyzer := gas.NewAnalyzer(config, nil) - analyzer.AddRule(NewWeakRandCheck(config)) + analyzer.AddRule(NewWeakRandCheck("TEST", config)) issues := gasTestRunner( ` diff --git a/rules/rsa.go b/rules/rsa.go index 510ca78..c9adb6d 100644 --- a/rules/rsa.go +++ b/rules/rsa.go @@ -28,6 +28,10 @@ type WeakKeyStrength struct { bits int } +func (r *WeakKeyStrength) ID() string { + return r.MetaData.ID +} + func (w *WeakKeyStrength) Match(n ast.Node, c *gas.Context) (*gas.Issue, error) { if node := gas.MatchCall(n, w.pattern); node != nil { if bits, err := gas.GetInt(node.Args[1]); err == nil && bits < (int64)(w.bits) { @@ -37,12 +41,13 @@ func (w *WeakKeyStrength) Match(n ast.Node, c *gas.Context) (*gas.Issue, error) return nil, nil } -func NewWeakKeyStrength(conf map[string]interface{}) (gas.Rule, []ast.Node) { +func NewWeakKeyStrength(id string, conf map[string]interface{}) (gas.Rule, []ast.Node) { bits := 2048 return &WeakKeyStrength{ pattern: regexp.MustCompile(`^rsa\.GenerateKey$`), bits: bits, MetaData: gas.MetaData{ + ID: id, Severity: gas.Medium, Confidence: gas.High, What: fmt.Sprintf("RSA keys should be at least %d bits", bits), diff --git a/rules/rsa_test.go b/rules/rsa_test.go index 9b0b47b..0c9c6b7 100644 --- a/rules/rsa_test.go +++ b/rules/rsa_test.go @@ -23,7 +23,7 @@ import ( func TestRSAKeys(t *testing.T) { config := map[string]interface{}{"ignoreNosec": false} analyzer := gas.NewAnalyzer(config, nil) - analyzer.AddRule(NewWeakKeyStrength(config)) + analyzer.AddRule(NewWeakKeyStrength("TEST", config)) issues := gasTestRunner( `package main diff --git a/rules/sql.go b/rules/sql.go index 9b8b79f..6dd53fb 100644 --- a/rules/sql.go +++ b/rules/sql.go @@ -30,6 +30,14 @@ type SqlStrConcat struct { SqlStatement } +func (r *SqlStatement) ID() string { + return r.MetaData.ID +} + +func (r *SqlStrConcat) ID() string { + return r.MetaData.ID +} + // see if we can figure out what it is func (s *SqlStrConcat) checkObject(n *ast.Ident) bool { if n.Obj != nil { @@ -56,11 +64,12 @@ func (s *SqlStrConcat) Match(n ast.Node, c *gas.Context) (*gas.Issue, error) { return nil, nil } -func NewSqlStrConcat(conf map[string]interface{}) (gas.Rule, []ast.Node) { +func NewSqlStrConcat(id string, conf map[string]interface{}) (gas.Rule, []ast.Node) { return &SqlStrConcat{ SqlStatement: SqlStatement{ pattern: regexp.MustCompile(`(?)(SELECT|DELETE|INSERT|UPDATE|INTO|FROM|WHERE) `), MetaData: gas.MetaData{ + ID: id, Severity: gas.Medium, Confidence: gas.High, What: "SQL string concatenation", @@ -84,12 +93,13 @@ func (s *SqlStrFormat) Match(n ast.Node, c *gas.Context) (gi *gas.Issue, err err return nil, nil } -func NewSqlStrFormat(conf map[string]interface{}) (gas.Rule, []ast.Node) { +func NewSqlStrFormat(id string, conf map[string]interface{}) (gas.Rule, []ast.Node) { return &SqlStrFormat{ call: regexp.MustCompile(`^fmt\.Sprintf$`), SqlStatement: SqlStatement{ pattern: regexp.MustCompile("(?)(SELECT|DELETE|INSERT|UPDATE|INTO|FROM|WHERE) "), MetaData: gas.MetaData{ + ID: id, Severity: gas.Medium, Confidence: gas.High, What: "SQL string formatting", diff --git a/rules/sql_test.go b/rules/sql_test.go index 8919f7a..c512f8f 100644 --- a/rules/sql_test.go +++ b/rules/sql_test.go @@ -23,7 +23,7 @@ import ( func TestSQLInjectionViaConcatenation(t *testing.T) { config := map[string]interface{}{"ignoreNosec": false} analyzer := gas.NewAnalyzer(config, nil) - analyzer.AddRule(NewSqlStrConcat(config)) + analyzer.AddRule(NewSqlStrConcat("TEST", config)) source := ` package main @@ -51,7 +51,7 @@ func TestSQLInjectionViaConcatenation(t *testing.T) { func TestSQLInjectionViaIntepolation(t *testing.T) { config := map[string]interface{}{"ignoreNosec": false} analyzer := gas.NewAnalyzer(config, nil) - analyzer.AddRule(NewSqlStrFormat(config)) + analyzer.AddRule(NewSqlStrFormat("TEST", config)) source := ` package main @@ -81,8 +81,8 @@ func TestSQLInjectionViaIntepolation(t *testing.T) { func TestSQLInjectionFalsePositiveA(t *testing.T) { config := map[string]interface{}{"ignoreNosec": false} analyzer := gas.NewAnalyzer(config, nil) - analyzer.AddRule(NewSqlStrConcat(config)) - analyzer.AddRule(NewSqlStrFormat(config)) + analyzer.AddRule(NewSqlStrConcat("TEST1", config)) + analyzer.AddRule(NewSqlStrFormat("TEST2", config)) source := ` @@ -115,8 +115,8 @@ func TestSQLInjectionFalsePositiveA(t *testing.T) { func TestSQLInjectionFalsePositiveB(t *testing.T) { config := map[string]interface{}{"ignoreNosec": false} analyzer := gas.NewAnalyzer(config, nil) - analyzer.AddRule(NewSqlStrConcat(config)) - analyzer.AddRule(NewSqlStrFormat(config)) + analyzer.AddRule(NewSqlStrConcat("TEST1", config)) + analyzer.AddRule(NewSqlStrFormat("TEST2", config)) source := ` @@ -149,8 +149,8 @@ func TestSQLInjectionFalsePositiveB(t *testing.T) { func TestSQLInjectionFalsePositiveC(t *testing.T) { config := map[string]interface{}{"ignoreNosec": false} analyzer := gas.NewAnalyzer(config, nil) - analyzer.AddRule(NewSqlStrConcat(config)) - analyzer.AddRule(NewSqlStrFormat(config)) + analyzer.AddRule(NewSqlStrConcat("TEST1", config)) + analyzer.AddRule(NewSqlStrFormat("TEST2", config)) source := ` @@ -183,8 +183,8 @@ func TestSQLInjectionFalsePositiveC(t *testing.T) { func TestSQLInjectionFalsePositiveD(t *testing.T) { config := map[string]interface{}{"ignoreNosec": false} analyzer := gas.NewAnalyzer(config, nil) - analyzer.AddRule(NewSqlStrConcat(config)) - analyzer.AddRule(NewSqlStrFormat(config)) + analyzer.AddRule(NewSqlStrConcat("TEST1", config)) + analyzer.AddRule(NewSqlStrFormat("TEST2", config)) source := ` diff --git a/rules/subproc.go b/rules/subproc.go index b5a6fa2..ed236da 100644 --- a/rules/subproc.go +++ b/rules/subproc.go @@ -23,9 +23,14 @@ import ( ) type Subprocess struct { + gas.MetaData pattern *regexp.Regexp } +func (r *Subprocess) ID() string { + return r.MetaData.ID +} + func (r *Subprocess) Match(n ast.Node, c *gas.Context) (*gas.Issue, error) { if node := gas.MatchCall(n, r.pattern); node != nil { for _, arg := range node.Args { @@ -43,14 +48,19 @@ func (r *Subprocess) Match(n ast.Node, c *gas.Context) (*gas.Issue, error) { } } - what := "Subprocess launching should be audited." - return gas.NewIssue(c, n, what, gas.Low, gas.High), nil + return gas.NewIssue(c, n, r.What, r.Severity, r.Confidence), nil } return nil, nil } -func NewSubproc(conf map[string]interface{}) (gas.Rule, []ast.Node) { +func NewSubproc(id string, conf map[string]interface{}) (gas.Rule, []ast.Node) { return &Subprocess{ pattern: regexp.MustCompile(`^exec\.Command|syscall\.Exec$`), + MetaData: gas.MetaData{ + ID: id, + Severity: gas.Low, + Confidence: gas.High, + What: "Subprocess launching should be audited.", + }, }, []ast.Node{(*ast.CallExpr)(nil)} } diff --git a/rules/subproc_test.go b/rules/subproc_test.go index 13c79df..8f2c427 100644 --- a/rules/subproc_test.go +++ b/rules/subproc_test.go @@ -23,7 +23,7 @@ import ( func TestSubprocess(t *testing.T) { config := map[string]interface{}{"ignoreNosec": false} analyzer := gas.NewAnalyzer(config, nil) - analyzer.AddRule(NewSubproc(config)) + analyzer.AddRule(NewSubproc("TEST", config)) issues := gasTestRunner(` package main @@ -51,7 +51,7 @@ func TestSubprocess(t *testing.T) { func TestSubprocessVar(t *testing.T) { config := map[string]interface{}{"ignoreNosec": false} analyzer := gas.NewAnalyzer(config, nil) - analyzer.AddRule(NewSubproc(config)) + analyzer.AddRule(NewSubproc("TEST", config)) issues := gasTestRunner(` package main @@ -80,7 +80,7 @@ func TestSubprocessVar(t *testing.T) { func TestSubprocessPath(t *testing.T) { config := map[string]interface{}{"ignoreNosec": false} analyzer := gas.NewAnalyzer(config, nil) - analyzer.AddRule(NewSubproc(config)) + analyzer.AddRule(NewSubproc("TEST", config)) issues := gasTestRunner(` package main @@ -107,7 +107,7 @@ func TestSubprocessPath(t *testing.T) { func TestSubprocessSyscall(t *testing.T) { config := map[string]interface{}{"ignoreNosec": false} analyzer := gas.NewAnalyzer(config, nil) - analyzer.AddRule(NewSubproc(config)) + analyzer.AddRule(NewSubproc("TEST", config)) issues := gasTestRunner(` package main diff --git a/rules/tempfiles.go b/rules/tempfiles.go index 8cbd55a..e04c6b9 100644 --- a/rules/tempfiles.go +++ b/rules/tempfiles.go @@ -27,6 +27,10 @@ type BadTempFile struct { call *regexp.Regexp } +func (r *BadTempFile) ID() string { + return r.MetaData.ID +} + func (t *BadTempFile) Match(n ast.Node, c *gas.Context) (gi *gas.Issue, err error) { if node := gas.MatchCall(n, t.call); node != nil { if arg, e := gas.GetString(node.Args[0]); t.args.MatchString(arg) && e == nil { @@ -36,11 +40,12 @@ func (t *BadTempFile) Match(n ast.Node, c *gas.Context) (gi *gas.Issue, err erro return nil, nil } -func NewBadTempFile(conf map[string]interface{}) (gas.Rule, []ast.Node) { +func NewBadTempFile(id string, conf map[string]interface{}) (gas.Rule, []ast.Node) { return &BadTempFile{ call: regexp.MustCompile(`ioutil\.WriteFile|os\.Create`), args: regexp.MustCompile(`^/tmp/.*$|^/var/tmp/.*$`), MetaData: gas.MetaData{ + ID: id, Severity: gas.Medium, Confidence: gas.High, What: "File creation in shared tmp directory without using ioutil.Tempfile", diff --git a/rules/tempfiles_test.go b/rules/tempfiles_test.go index 51709e8..4ac8a67 100644 --- a/rules/tempfiles_test.go +++ b/rules/tempfiles_test.go @@ -23,7 +23,7 @@ import ( func TestTempfiles(t *testing.T) { config := map[string]interface{}{"ignoreNosec": false} analyzer := gas.NewAnalyzer(config, nil) - analyzer.AddRule(NewBadTempFile(config)) + analyzer.AddRule(NewBadTempFile("TEST", config)) source := ` package samples diff --git a/rules/templates.go b/rules/templates.go index 0f1dc24..4003982 100644 --- a/rules/templates.go +++ b/rules/templates.go @@ -26,6 +26,10 @@ type TemplateCheck struct { call *regexp.Regexp } +func (r *TemplateCheck) ID() string { + return r.MetaData.ID +} + func (t *TemplateCheck) Match(n ast.Node, c *gas.Context) (gi *gas.Issue, err error) { if node := gas.MatchCall(n, t.call); node != nil { for _, arg := range node.Args { @@ -37,10 +41,11 @@ func (t *TemplateCheck) Match(n ast.Node, c *gas.Context) (gi *gas.Issue, err er return nil, nil } -func NewTemplateCheck(conf map[string]interface{}) (gas.Rule, []ast.Node) { +func NewTemplateCheck(id string, conf map[string]interface{}) (gas.Rule, []ast.Node) { return &TemplateCheck{ call: regexp.MustCompile(`^template\.(HTML|JS|URL)$`), MetaData: gas.MetaData{ + ID: id, Severity: gas.Medium, Confidence: gas.Low, What: "this method will not auto-escape HTML. Verify data is well formed.", diff --git a/rules/templates_test.go b/rules/templates_test.go index 83dccf1..4b9c05b 100644 --- a/rules/templates_test.go +++ b/rules/templates_test.go @@ -23,7 +23,7 @@ import ( func TestTemplateCheckSafe(t *testing.T) { config := map[string]interface{}{"ignoreNosec": false} analyzer := gas.NewAnalyzer(config, nil) - analyzer.AddRule(NewTemplateCheck(config)) + analyzer.AddRule(NewTemplateCheck("TEST", config)) source := ` package samples @@ -51,7 +51,7 @@ func TestTemplateCheckSafe(t *testing.T) { func TestTemplateCheckBadHTML(t *testing.T) { config := map[string]interface{}{"ignoreNosec": false} analyzer := gas.NewAnalyzer(config, nil) - analyzer.AddRule(NewTemplateCheck(config)) + analyzer.AddRule(NewTemplateCheck("TEST", config)) source := ` package samples @@ -80,7 +80,7 @@ func TestTemplateCheckBadHTML(t *testing.T) { func TestTemplateCheckBadJS(t *testing.T) { config := map[string]interface{}{"ignoreNosec": false} analyzer := gas.NewAnalyzer(config, nil) - analyzer.AddRule(NewTemplateCheck(config)) + analyzer.AddRule(NewTemplateCheck("TEST", config)) source := ` package samples @@ -109,7 +109,7 @@ func TestTemplateCheckBadJS(t *testing.T) { func TestTemplateCheckBadURL(t *testing.T) { config := map[string]interface{}{"ignoreNosec": false} analyzer := gas.NewAnalyzer(config, nil) - analyzer.AddRule(NewTemplateCheck(config)) + analyzer.AddRule(NewTemplateCheck("TEST", config)) source := ` package samples diff --git a/rules/tls.go b/rules/tls.go index a323c8f..29f3d69 100644 --- a/rules/tls.go +++ b/rules/tls.go @@ -24,12 +24,17 @@ import ( ) type InsecureConfigTLS struct { + gas.MetaData MinVersion int16 MaxVersion int16 pattern *regexp.Regexp goodCiphers []string } +func (r *InsecureConfigTLS) ID() string { + return r.MetaData.ID +} + func stringInSlice(a string, list []string) bool { for _, b := range list { if b == a { @@ -121,7 +126,7 @@ func (t *InsecureConfigTLS) Match(n ast.Node, c *gas.Context) (gi *gas.Issue, er return } -func NewModernTlsCheck(conf map[string]interface{}) (gas.Rule, []ast.Node) { +func NewModernTlsCheck(id string, conf map[string]interface{}) (gas.Rule, []ast.Node) { // https://wiki.mozilla.org/Security/Server_Side_TLS#Modern_compatibility return &InsecureConfigTLS{ pattern: regexp.MustCompile(`^tls\.Config$`), @@ -135,10 +140,13 @@ func NewModernTlsCheck(conf map[string]interface{}) (gas.Rule, []ast.Node) { "TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384", "TLS_ECDHE_RSA_WITH_CHACHA20_POLY1305", }, + MetaData: gas.MetaData{ + ID: id, + }, }, []ast.Node{(*ast.CompositeLit)(nil)} } -func NewIntermediateTlsCheck(conf map[string]interface{}) (gas.Rule, []ast.Node) { +func NewIntermediateTlsCheck(id string, conf map[string]interface{}) (gas.Rule, []ast.Node) { // https://wiki.mozilla.org/Security/Server_Side_TLS#Intermediate_compatibility_.28default.29 return &InsecureConfigTLS{ pattern: regexp.MustCompile(`^tls\.Config$`), @@ -161,10 +169,13 @@ func NewIntermediateTlsCheck(conf map[string]interface{}) (gas.Rule, []ast.Node) "TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384", "TLS_ECDHE_ECDSA_WITH_AES_256_GCM_SHA384", }, + MetaData: gas.MetaData{ + ID: id, + }, }, []ast.Node{(*ast.CompositeLit)(nil)} } -func NewCompatTlsCheck(conf map[string]interface{}) (gas.Rule, []ast.Node) { +func NewCompatTlsCheck(id string, conf map[string]interface{}) (gas.Rule, []ast.Node) { // https://wiki.mozilla.org/Security/Server_Side_TLS#Old_compatibility_.28default.29 return &InsecureConfigTLS{ pattern: regexp.MustCompile(`^tls\.Config$`), @@ -189,5 +200,8 @@ func NewCompatTlsCheck(conf map[string]interface{}) (gas.Rule, []ast.Node) { "TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384", "TLS_ECDHE_ECDSA_WITH_AES_256_GCM_SHA384", }, + MetaData: gas.MetaData{ + ID: id, + }, }, []ast.Node{(*ast.CompositeLit)(nil)} } diff --git a/rules/tls_test.go b/rules/tls_test.go index 9b215a3..4b995b2 100644 --- a/rules/tls_test.go +++ b/rules/tls_test.go @@ -23,7 +23,7 @@ import ( func TestInsecureSkipVerify(t *testing.T) { config := map[string]interface{}{"ignoreNosec": false} analyzer := gas.NewAnalyzer(config, nil) - analyzer.AddRule(NewModernTlsCheck(config)) + analyzer.AddRule(NewModernTlsCheck("TEST", config)) issues := gasTestRunner(` package main @@ -52,7 +52,7 @@ func TestInsecureSkipVerify(t *testing.T) { func TestInsecureMinVersion(t *testing.T) { config := map[string]interface{}{"ignoreNosec": false} analyzer := gas.NewAnalyzer(config, nil) - analyzer.AddRule(NewModernTlsCheck(config)) + analyzer.AddRule(NewModernTlsCheck("TEST", config)) issues := gasTestRunner(` package main @@ -81,7 +81,7 @@ func TestInsecureMinVersion(t *testing.T) { func TestInsecureMaxVersion(t *testing.T) { config := map[string]interface{}{"ignoreNosec": false} analyzer := gas.NewAnalyzer(config, nil) - analyzer.AddRule(NewModernTlsCheck(config)) + analyzer.AddRule(NewModernTlsCheck("TEST", config)) issues := gasTestRunner(` package main @@ -110,7 +110,7 @@ func TestInsecureMaxVersion(t *testing.T) { func TestInsecureCipherSuite(t *testing.T) { config := map[string]interface{}{"ignoreNosec": false} analyzer := gas.NewAnalyzer(config, nil) - analyzer.AddRule(NewModernTlsCheck(config)) + analyzer.AddRule(NewModernTlsCheck("TEST", config)) issues := gasTestRunner(` package main @@ -142,7 +142,7 @@ func TestInsecureCipherSuite(t *testing.T) { func TestPreferServerCipherSuites(t *testing.T) { config := map[string]interface{}{"ignoreNosec": false} analyzer := gas.NewAnalyzer(config, nil) - analyzer.AddRule(NewModernTlsCheck(config)) + analyzer.AddRule(NewModernTlsCheck("TEST", config)) issues := gasTestRunner(` package main diff --git a/rules/unsafe.go b/rules/unsafe.go index 861f77c..2c4f2b4 100644 --- a/rules/unsafe.go +++ b/rules/unsafe.go @@ -15,8 +15,9 @@ package rules import ( - gas "github.com/GoASTScanner/gas/core" "go/ast" + + gas "github.com/GoASTScanner/gas/core" ) type UsingUnsafe struct { @@ -25,6 +26,10 @@ type UsingUnsafe struct { calls []string } +func (r *UsingUnsafe) ID() string { + return r.MetaData.ID +} + func (r *UsingUnsafe) Match(n ast.Node, c *gas.Context) (gi *gas.Issue, err error) { if _, matches := gas.MatchCallByPackage(n, c, r.pkg, r.calls...); matches { return gas.NewIssue(c, n, r.What, r.Severity, r.Confidence), nil @@ -32,11 +37,12 @@ func (r *UsingUnsafe) Match(n ast.Node, c *gas.Context) (gi *gas.Issue, err erro return nil, nil } -func NewUsingUnsafe(conf map[string]interface{}) (gas.Rule, []ast.Node) { +func NewUsingUnsafe(id string, conf map[string]interface{}) (gas.Rule, []ast.Node) { return &UsingUnsafe{ pkg: "unsafe", calls: []string{"Alignof", "Offsetof", "Sizeof", "Pointer"}, MetaData: gas.MetaData{ + ID: id, What: "Use of unsafe calls should be audited", Severity: gas.Low, Confidence: gas.High, diff --git a/rules/unsafe_test.go b/rules/unsafe_test.go index f8d7787..b7c9cec 100644 --- a/rules/unsafe_test.go +++ b/rules/unsafe_test.go @@ -23,7 +23,7 @@ import ( func TestUnsafe(t *testing.T) { config := map[string]interface{}{"ignoreNosec": false} analyzer := gas.NewAnalyzer(config, nil) - analyzer.AddRule(NewUsingUnsafe(config)) + analyzer.AddRule(NewUsingUnsafe("TEST", config)) issues := gasTestRunner(` package main diff --git a/rules/weakcrypto.go b/rules/weakcrypto.go index 1c859e9..2f81b1b 100644 --- a/rules/weakcrypto.go +++ b/rules/weakcrypto.go @@ -25,6 +25,10 @@ type UsesWeakCryptography struct { blacklist map[string][]string } +func (r *UsesWeakCryptography) ID() string { + return r.MetaData.ID +} + func (r *UsesWeakCryptography) Match(n ast.Node, c *gas.Context) (*gas.Issue, error) { for pkg, funcs := range r.blacklist { @@ -36,7 +40,7 @@ func (r *UsesWeakCryptography) Match(n ast.Node, c *gas.Context) (*gas.Issue, er } // Uses des.* md5.* or rc4.* -func NewUsesWeakCryptography(conf map[string]interface{}) (gas.Rule, []ast.Node) { +func NewUsesWeakCryptography(id string, conf map[string]interface{}) (gas.Rule, []ast.Node) { calls := make(map[string][]string) calls["crypto/des"] = []string{"NewCipher", "NewTripleDESCipher"} calls["crypto/md5"] = []string{"New", "Sum"} @@ -44,6 +48,7 @@ func NewUsesWeakCryptography(conf map[string]interface{}) (gas.Rule, []ast.Node) rule := &UsesWeakCryptography{ blacklist: calls, MetaData: gas.MetaData{ + ID: id, Severity: gas.Medium, Confidence: gas.High, What: "Use of weak cryptographic primitive", diff --git a/rules/weakcrypto_test.go b/rules/weakcrypto_test.go index 1305c33..0de5255 100644 --- a/rules/weakcrypto_test.go +++ b/rules/weakcrypto_test.go @@ -23,8 +23,8 @@ import ( func TestMD5(t *testing.T) { config := map[string]interface{}{"ignoreNosec": false} analyzer := gas.NewAnalyzer(config, nil) - analyzer.AddRule(NewBlacklist_crypto_md5(config)) - analyzer.AddRule(NewUsesWeakCryptography(config)) + analyzer.AddRule(NewBlacklist_crypto_md5("TEST1", config)) + analyzer.AddRule(NewUsesWeakCryptography("TEST2", config)) issues := gasTestRunner(` package main @@ -45,8 +45,8 @@ func TestMD5(t *testing.T) { func TestDES(t *testing.T) { config := map[string]interface{}{"ignoreNosec": false} analyzer := gas.NewAnalyzer(config, nil) - analyzer.AddRule(NewBlacklist_crypto_des(config)) - analyzer.AddRule(NewUsesWeakCryptography(config)) + analyzer.AddRule(NewBlacklist_crypto_des("TEST1", config)) + analyzer.AddRule(NewUsesWeakCryptography("TEST2", config)) issues := gasTestRunner(` package main @@ -85,8 +85,8 @@ func TestDES(t *testing.T) { func TestRC4(t *testing.T) { config := map[string]interface{}{"ignoreNosec": false} analyzer := gas.NewAnalyzer(config, nil) - analyzer.AddRule(NewBlacklist_crypto_rc4(config)) - analyzer.AddRule(NewUsesWeakCryptography(config)) + analyzer.AddRule(NewBlacklist_crypto_rc4("TEST1", config)) + analyzer.AddRule(NewUsesWeakCryptography("TEST2", config)) issues := gasTestRunner(` package main From 2b2999b48d1fadbfee63197c0a26fcab7f1500d1 Mon Sep 17 00:00:00 2001 From: Jon McClintock Date: Thu, 5 Oct 2017 21:43:54 +0000 Subject: [PATCH 02/11] Add tests for excludes with comments --- rules/nosec_test.go | 46 +++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 46 insertions(+) diff --git a/rules/nosec_test.go b/rules/nosec_test.go index 377c6f7..bad415b 100644 --- a/rules/nosec_test.go +++ b/rules/nosec_test.go @@ -168,6 +168,29 @@ func TestNosecBlockExcludeOne(t *testing.T) { checkTestResults(t, issues, 0, "None") } +func TestNosecBlockExcludeOneWithComment(t *testing.T) { + config := map[string]interface{}{"ignoreNosec": false} + analyzer := gas.NewAnalyzer(config, nil) + analyzer.AddRule(NewSubproc("G001", config)) + + issues := gasTestRunner( + `package main + import ( + "os" + "os/exec" + ) + + func main() { + // #exclude !G001(This rule is bogus) + if true { + cmd := exec.Command("sh", "-c", os.Getenv("BLAH")) + cmd.Run() + } + }`, analyzer) + + checkTestResults(t, issues, 0, "None") +} + func TestNosecBlockExcludeOneNoMatch(t *testing.T) { config := map[string]interface{}{"ignoreNosec": false} analyzer := gas.NewAnalyzer(config, nil) @@ -256,3 +279,26 @@ func TestNosecExcludeTwoBothMatch(t *testing.T) { checkTestResults(t, issues, 0, "No issues") } + +func TestNosecExcludeTwoWithComments(t *testing.T) { + config := map[string]interface{}{"ignoreNosec": false} + analyzer := gas.NewAnalyzer(config, nil) + analyzer.AddRule(NewSubproc("G001", config)) + analyzer.AddRule(NewWeakRandCheck("G002", config)) + + issues := gasTestRunner( + `package main + import ( + "math/rand" + "os" + "os/exec" + ) + + func main() { + // #exclude !G001(The env var is trusted) !G002(Unimportant random number) + cmd := exec.Command("sh", "-c", os.Getenv("BLAH"), string(rand.Int())) + cmd.Run() + }`, analyzer) + + checkTestResults(t, issues, 0, "No issues") +} From 846c9ffc7c27c22798c176c2fce2aff22ce4db48 Mon Sep 17 00:00:00 2001 From: Jon McClintock Date: Mon, 29 Jan 2018 18:33:48 +0000 Subject: [PATCH 03/11] [Issue 159] Allow loader errors so that processing continues if there's a package loading problem. --- analyzer.go | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/analyzer.go b/analyzer.go index f050ab7..e336869 100644 --- a/analyzer.go +++ b/analyzer.go @@ -96,7 +96,11 @@ func (gas *Analyzer) LoadRules(ruleDefinitions ...RuleBuilder) { // Process kicks off the analysis process for a given package func (gas *Analyzer) Process(packagePaths ...string) error { - packageConfig := loader.Config{Build: &build.Default, ParserMode: parser.ParseComments} + packageConfig := loader.Config{ + Build: &build.Default, + ParserMode: parser.ParseComments, + AllowErrors: true, + } for _, packagePath := range packagePaths { abspath, _ := filepath.Abs(packagePath) gas.logger.Println("Searching directory:", abspath) From f7c31f243904cfbcd02d545db42bbc91d65f89dc Mon Sep 17 00:00:00 2001 From: Grant Murphy Date: Tue, 30 Jan 2018 09:27:55 +1000 Subject: [PATCH 04/11] Using godep not glide for dependency management --- glide.lock | 25 ------------------------- 1 file changed, 25 deletions(-) delete mode 100644 glide.lock diff --git a/glide.lock b/glide.lock deleted file mode 100644 index d76519f..0000000 --- a/glide.lock +++ /dev/null @@ -1,25 +0,0 @@ -hash: fb4cbcb4f806804f30683cd298d9316522f1d7678498039eccdb29f020de1c7f -updated: 2017-05-09T21:54:08.9517391-07:00 -imports: -- name: github.com/kisielk/gotool - version: 0de1eaf82fa3f583ce21fde859f1e7e0c5e9b220 -- name: github.com/nbutton23/zxcvbn-go - version: a22cb81b2ecdde8b68e9ffb8824731cbf88e1de4 - subpackages: - - adjacency - - data - - entropy - - frequency - - match - - matching - - scoring - - utils/math -- name: github.com/ryanuber/go-glob - version: 572520ed46dbddaed19ea3d9541bdd0494163693 -- name: golang.org/x/tools - version: 1dbffd0798679c0c6b466e620725135944cfddea - subpackages: - - go/ast/astutil - - go/buildutil - - go/loader -testImports: [] From 485bc31df8fbc11f6e9b5bfd791c018a71c1900f Mon Sep 17 00:00:00 2001 From: Grant Murphy Date: Tue, 30 Jan 2018 09:32:04 +1000 Subject: [PATCH 05/11] Fix go vet errors in tests --- analyzer_test.go | 4 +--- import_tracker_test.go | 3 ++- rules/rules_test.go | 3 +-- 3 files changed, 4 insertions(+), 6 deletions(-) diff --git a/analyzer_test.go b/analyzer_test.go index cff3415..3422b5c 100644 --- a/analyzer_test.go +++ b/analyzer_test.go @@ -1,7 +1,6 @@ package gas_test import ( - "bytes" "io/ioutil" "log" "os" @@ -20,10 +19,9 @@ var _ = Describe("Analyzer", func() { var ( analyzer *gas.Analyzer logger *log.Logger - output *bytes.Buffer ) BeforeEach(func() { - logger, output = testutils.NewLogger() + logger, _ = testutils.NewLogger() analyzer = gas.NewAnalyzer(nil, logger) }) diff --git a/import_tracker_test.go b/import_tracker_test.go index abde8b7..492933c 100644 --- a/import_tracker_test.go +++ b/import_tracker_test.go @@ -2,7 +2,7 @@ package gas_test import ( . "github.com/onsi/ginkgo" - //. "github.com/onsi/gomega" + . "github.com/onsi/gomega" ) var _ = Describe("ImportTracker", func() { @@ -15,6 +15,7 @@ var _ = Describe("ImportTracker", func() { }) Context("when I have a valid go package", func() { It("should record all import specs", func() { + Expect(source).To(Equal(source)) Skip("Not implemented") }) diff --git a/rules/rules_test.go b/rules/rules_test.go index 6bc0d7d..6b0f884 100644 --- a/rules/rules_test.go +++ b/rules/rules_test.go @@ -17,14 +17,13 @@ var _ = Describe("gas rules", func() { var ( logger *log.Logger - output *bytes.Buffer config gas.Config analyzer *gas.Analyzer runner func(string, []testutils.CodeSample) ) BeforeEach(func() { - logger, output = testutils.NewLogger() + logger, _ = testutils.NewLogger() config = gas.NewConfig() analyzer = gas.NewAnalyzer(config, logger) runner = func(rule string, samples []testutils.CodeSample) { From 187a71124ed7fd75d2e1c185ad061e9477352193 Mon Sep 17 00:00:00 2001 From: Grant Murphy Date: Tue, 30 Jan 2018 09:35:35 +1000 Subject: [PATCH 06/11] Unused import --- rules/rules_test.go | 1 - 1 file changed, 1 deletion(-) diff --git a/rules/rules_test.go b/rules/rules_test.go index 6b0f884..c42902f 100644 --- a/rules/rules_test.go +++ b/rules/rules_test.go @@ -1,7 +1,6 @@ package rules_test import ( - "bytes" "fmt" "log" From 1429033acac0dd7e86c5aeb2ad75238cf6fe4ba7 Mon Sep 17 00:00:00 2001 From: Jon McClintock Date: Thu, 5 Oct 2017 21:32:03 +0000 Subject: [PATCH 07/11] Add support for #excluding specific rules --- analyzer.go | 91 +++++++++++++++++++++++++--------- analyzer_test.go | 65 +++++++++++++++++++++--- cmd/gas/main.go | 2 +- issue.go | 1 + issue_test.go | 2 +- rule.go | 3 +- rule_test.go | 4 ++ rules/big.go | 7 ++- rules/bind.go | 7 ++- rules/blacklist.go | 23 +++++---- rules/errors.go | 8 ++- rules/fileperms.go | 10 +++- rules/hardcoded_credentials.go | 7 ++- rules/rand.go | 7 ++- rules/rsa.go | 7 ++- rules/rulelist.go | 63 ++++++++++++----------- rules/rules_test.go | 2 +- rules/sql.go | 14 +++++- rules/ssh.go | 7 ++- rules/subproc.go | 9 +++- rules/tempfiles.go | 7 ++- rules/templates.go | 7 ++- rules/tls.go | 5 ++ rules/tls_config.go | 9 ++-- rules/unsafe.go | 7 ++- rules/weakcrypto.go | 8 ++- 26 files changed, 288 insertions(+), 94 deletions(-) diff --git a/analyzer.go b/analyzer.go index b7a5e8f..656c45e 100644 --- a/analyzer.go +++ b/analyzer.go @@ -25,6 +25,7 @@ import ( "os" "path" "reflect" + "regexp" "strings" "path/filepath" @@ -43,6 +44,7 @@ type Context struct { Root *ast.File Config map[string]interface{} Imports *ImportTracker + Ignores []map[string]bool } // Metrics used when reporting information about a scanning run. @@ -87,9 +89,9 @@ func NewAnalyzer(conf Config, logger *log.Logger) *Analyzer { // LoadRules instantiates all the rules to be used when analyzing source // packages -func (gas *Analyzer) LoadRules(ruleDefinitions ...RuleBuilder) { - for _, builder := range ruleDefinitions { - r, nodes := builder(gas.config) +func (gas *Analyzer) LoadRules(ruleDefinitions map[string]RuleBuilder) { + for id, def := range ruleDefinitions { + r, nodes := def(id, gas.config) gas.ruleset.Register(r, nodes...) } } @@ -147,41 +149,84 @@ func (gas *Analyzer) Process(packagePaths ...string) error { } // ignore a node (and sub-tree) if it is tagged with a "#nosec" comment -func (gas *Analyzer) ignore(n ast.Node) bool { +func (gas *Analyzer) ignore(n ast.Node) ([]string, bool) { if groups, ok := gas.context.Comments[n]; ok && !gas.ignoreNosec { for _, group := range groups { if strings.Contains(group.Text(), "#nosec") { gas.stats.NumNosec++ - return true + return nil, true + } + + if strings.Contains(group.Text(), "#exclude") { + gas.stats.NumNosec++ + + // Pull out the specific rules that are listed to be ignored. + re := regexp.MustCompile("!(G\\d{3})") + matches := re.FindAllStringSubmatch(group.Text(), -1) + + // Find the rule IDs to ignore. + ignores := make([]string, 0) + for _, v := range matches { + ignores = append(ignores, v[1]) + } + return ignores, false } } } - return false + return nil, false } // Visit runs the GAS visitor logic over an AST created by parsing go code. // Rule methods added with AddRule will be invoked as necessary. func (gas *Analyzer) Visit(n ast.Node) ast.Visitor { - if !gas.ignore(n) { - - // Track aliased and initialization imports - gas.context.Imports.TrackImport(n) - - for _, rule := range gas.ruleset.RegisteredFor(n) { - issue, err := rule.Match(n, gas.context) - if err != nil { - file, line := GetLocation(n, gas.context) - file = path.Base(file) - gas.logger.Printf("Rule error: %v => %s (%s:%d)\n", reflect.TypeOf(rule), err, file, line) - } - if issue != nil { - gas.issues = append(gas.issues, issue) - gas.stats.NumFound++ - } + // If we've reached the end of this branch, pop off the ignores stack. + if n == nil { + if len(gas.context.Ignores) > 0 { + gas.context.Ignores = gas.context.Ignores[1:] } return gas } - return nil + + // Get any new rule exclusions. + ignoredRules, ignoreAll := gas.ignore(n) + if ignoreAll { + return nil + } + + // Now create the union of exclusions. + ignores := make(map[string]bool, 0) + if len(gas.context.Ignores) > 0 { + for k, v := range gas.context.Ignores[0] { + ignores[k] = v + } + } + + for _, v := range ignoredRules { + ignores[v] = true + } + + // Push the new set onto the stack. + gas.context.Ignores = append([]map[string]bool{ignores}, gas.context.Ignores...) + + // Track aliased and initialization imports + gas.context.Imports.TrackImport(n) + + for _, rule := range gas.ruleset.RegisteredFor(n) { + if _, ok := ignores[rule.ID()]; ok { + continue + } + issue, err := rule.Match(n, gas.context) + if err != nil { + file, line := GetLocation(n, gas.context) + file = path.Base(file) + gas.logger.Printf("Rule error: %v => %s (%s:%d)\n", reflect.TypeOf(rule), err, file, line) + } + if issue != nil { + gas.issues = append(gas.issues, issue) + gas.stats.NumFound++ + } + } + return gas } // Report returns the current issues discovered and the metrics about the scan diff --git a/analyzer_test.go b/analyzer_test.go index 3422b5c..3bd947c 100644 --- a/analyzer_test.go +++ b/analyzer_test.go @@ -28,7 +28,7 @@ var _ = Describe("Analyzer", func() { Context("when processing a package", func() { It("should return an error if the package contains no Go files", func() { - analyzer.LoadRules(rules.Generate().Builders()...) + analyzer.LoadRules(rules.Generate().Builders()) dir, err := ioutil.TempDir("", "empty") defer os.RemoveAll(dir) Expect(err).ShouldNot(HaveOccurred()) @@ -38,7 +38,7 @@ var _ = Describe("Analyzer", func() { }) It("should return an error if the package fails to build", func() { - analyzer.LoadRules(rules.Generate().Builders()...) + analyzer.LoadRules(rules.Generate().Builders()) pkg := testutils.NewTestPackage() defer pkg.Close() pkg.AddFile("wonky.go", `func main(){ println("forgot the package")}`) @@ -51,7 +51,7 @@ var _ = Describe("Analyzer", func() { }) It("should be able to analyze mulitple Go files", func() { - analyzer.LoadRules(rules.Generate().Builders()...) + analyzer.LoadRules(rules.Generate().Builders()) pkg := testutils.NewTestPackage() defer pkg.Close() pkg.AddFile("foo.go", ` @@ -72,7 +72,7 @@ var _ = Describe("Analyzer", func() { }) It("should be able to analyze mulitple Go packages", func() { - analyzer.LoadRules(rules.Generate().Builders()...) + analyzer.LoadRules(rules.Generate().Builders()) pkg1 := testutils.NewTestPackage() pkg2 := testutils.NewTestPackage() defer pkg1.Close() @@ -98,7 +98,7 @@ var _ = Describe("Analyzer", func() { // Rule for MD5 weak crypto usage sample := testutils.SampleCodeG401[0] source := sample.Code - analyzer.LoadRules(rules.Generate(rules.NewRuleFilter(false, "G401")).Builders()...) + analyzer.LoadRules(rules.Generate(rules.NewRuleFilter(false, "G401")).Builders()) controlPackage := testutils.NewTestPackage() defer controlPackage.Close() @@ -114,7 +114,7 @@ var _ = Describe("Analyzer", func() { // Rule for MD5 weak crypto usage sample := testutils.SampleCodeG401[0] source := sample.Code - analyzer.LoadRules(rules.Generate(rules.NewRuleFilter(false, "G401")).Builders()...) + analyzer.LoadRules(rules.Generate(rules.NewRuleFilter(false, "G401")).Builders()) nosecPackage := testutils.NewTestPackage() defer nosecPackage.Close() @@ -126,6 +126,57 @@ var _ = Describe("Analyzer", func() { 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] + source := sample.Code + analyzer.LoadRules(rules.Generate(rules.NewRuleFilter(false, "G401")).Builders()) + + nosecPackage := testutils.NewTestPackage() + defer nosecPackage.Close() + nosecSource := strings.Replace(source, "h := md5.New()", "h := md5.New() // #exclude !G401", 1) + nosecPackage.AddFile("md5.go", nosecSource) + nosecPackage.Build() + + analyzer.Process(nosecPackage.Path) + nosecIssues, _ := analyzer.Report() + Expect(nosecIssues).Should(BeEmpty()) + }) + + 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 + analyzer.LoadRules(rules.Generate(rules.NewRuleFilter(false, "G401")).Builders()) + + nosecPackage := testutils.NewTestPackage() + defer nosecPackage.Close() + nosecSource := strings.Replace(source, "h := md5.New()", "h := md5.New() // #exclude !G301", 1) + nosecPackage.AddFile("md5.go", nosecSource) + nosecPackage.Build() + + analyzer.Process(nosecPackage.Path) + 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() { + // Rule for MD5 weak crypto usage + sample := testutils.SampleCodeG401[0] + source := sample.Code + analyzer.LoadRules(rules.Generate(rules.NewRuleFilter(false, "G401")).Builders()) + + nosecPackage := testutils.NewTestPackage() + defer nosecPackage.Close() + nosecSource := strings.Replace(source, "h := md5.New()", "h := md5.New() // #exclude !G301 !G401", 1) + nosecPackage.AddFile("md5.go", nosecSource) + nosecPackage.Build() + + analyzer.Process(nosecPackage.Path) + nosecIssues, _ := analyzer.Report() + Expect(nosecIssues).Should(BeEmpty()) + }) }) It("should be possible to overwrite nosec comments, and report issues", func() { @@ -138,7 +189,7 @@ var _ = Describe("Analyzer", func() { nosecIgnoreConfig := gas.NewConfig() nosecIgnoreConfig.SetGlobal("nosec", "true") customAnalyzer := gas.NewAnalyzer(nosecIgnoreConfig, logger) - customAnalyzer.LoadRules(rules.Generate(rules.NewRuleFilter(false, "G401")).Builders()...) + customAnalyzer.LoadRules(rules.Generate(rules.NewRuleFilter(false, "G401")).Builders()) nosecPackage := testutils.NewTestPackage() defer nosecPackage.Close() diff --git a/cmd/gas/main.go b/cmd/gas/main.go index 03dc30c..1f620aa 100644 --- a/cmd/gas/main.go +++ b/cmd/gas/main.go @@ -206,7 +206,7 @@ func main() { // Create the analyzer analyzer := gas.NewAnalyzer(config, logger) - analyzer.LoadRules(ruleDefinitions.Builders()...) + analyzer.LoadRules(ruleDefinitions.Builders()) vendor := regexp.MustCompile(`[\\/]vendor([\\/]|$)`) diff --git a/issue.go b/issue.go index 2113529..65fa862 100644 --- a/issue.go +++ b/issue.go @@ -47,6 +47,7 @@ type Issue struct { // MetaData is embedded in all GAS rules. The Severity, Confidence and What message // will be passed tbhrough to reported issues. type MetaData struct { + ID string Severity Score Confidence Score What string diff --git a/issue_test.go b/issue_test.go index c58ffb6..4a74071 100644 --- a/issue_test.go +++ b/issue_test.go @@ -78,7 +78,7 @@ var _ = Describe("Issue", func() { // Use SQL rule to check binary expr cfg := gas.NewConfig() - rule, _ := rules.NewSQLStrConcat(cfg) + rule, _ := rules.NewSQLStrConcat("TEST", cfg) issue, err := rule.Match(target, ctx) Expect(err).ShouldNot(HaveOccurred()) Expect(issue).ShouldNot(BeNil()) diff --git a/rule.go b/rule.go index 58e1ce9..95c6562 100644 --- a/rule.go +++ b/rule.go @@ -19,11 +19,12 @@ import ( // The Rule interface used by all rules supported by GAS. type Rule interface { + ID() string Match(ast.Node, *Context) (*Issue, error) } // RuleBuilder is used to register a rule definition with the analyzer -type RuleBuilder func(c Config) (Rule, []ast.Node) +type RuleBuilder func(id string, c Config) (Rule, []ast.Node) // A RuleSet maps lists of rules to the type of AST node they should be run on. // The anaylzer will only invoke rules contained in the list associated with the diff --git a/rule_test.go b/rule_test.go index 63b1b2a..196e575 100644 --- a/rule_test.go +++ b/rule_test.go @@ -15,6 +15,10 @@ type mockrule struct { callback func(n ast.Node, ctx *gas.Context) bool } +func (m *mockrule) ID() string { + return "MOCK" +} + func (m *mockrule) Match(n ast.Node, ctx *gas.Context) (*gas.Issue, error) { if m.callback(n, ctx) { return m.issue, nil diff --git a/rules/big.go b/rules/big.go index 00c3162..0aec080 100644 --- a/rules/big.go +++ b/rules/big.go @@ -26,6 +26,10 @@ type usingBigExp struct { calls []string } +func (r *usingBigExp) ID() string { + return r.MetaData.ID +} + func (r *usingBigExp) Match(n ast.Node, c *gas.Context) (gi *gas.Issue, err error) { if _, matched := gas.MatchCallByType(n, c, r.pkg, r.calls...); matched { return gas.NewIssue(c, n, r.What, r.Severity, r.Confidence), nil @@ -34,11 +38,12 @@ func (r *usingBigExp) Match(n ast.Node, c *gas.Context) (gi *gas.Issue, err erro } // NewUsingBigExp detects issues with modulus == 0 for Bignum -func NewUsingBigExp(conf gas.Config) (gas.Rule, []ast.Node) { +func NewUsingBigExp(id string, conf gas.Config) (gas.Rule, []ast.Node) { return &usingBigExp{ pkg: "*math/big.Int", calls: []string{"Exp"}, MetaData: gas.MetaData{ + ID: id, What: "Use of math/big.Int.Exp function should be audited for modulus == 0", Severity: gas.Low, Confidence: gas.High, diff --git a/rules/bind.go b/rules/bind.go index 62518eb..cd16dce 100644 --- a/rules/bind.go +++ b/rules/bind.go @@ -28,6 +28,10 @@ type bindsToAllNetworkInterfaces struct { pattern *regexp.Regexp } +func (r *bindsToAllNetworkInterfaces) ID() string { + return r.MetaData.ID +} + func (r *bindsToAllNetworkInterfaces) Match(n ast.Node, c *gas.Context) (*gas.Issue, error) { callExpr := r.calls.ContainsCallExpr(n, c) if callExpr == nil { @@ -43,7 +47,7 @@ func (r *bindsToAllNetworkInterfaces) Match(n ast.Node, c *gas.Context) (*gas.Is // NewBindsToAllNetworkInterfaces detects socket connections that are setup to // listen on all network interfaces. -func NewBindsToAllNetworkInterfaces(conf gas.Config) (gas.Rule, []ast.Node) { +func NewBindsToAllNetworkInterfaces(id string, conf gas.Config) (gas.Rule, []ast.Node) { calls := gas.NewCallList() calls.Add("net", "Listen") calls.Add("crypto/tls", "Listen") @@ -51,6 +55,7 @@ func NewBindsToAllNetworkInterfaces(conf gas.Config) (gas.Rule, []ast.Node) { calls: calls, pattern: regexp.MustCompile(`^(0.0.0.0|:).*$`), MetaData: gas.MetaData{ + ID: id, Severity: gas.Medium, Confidence: gas.High, What: "Binds to all network interfaces", diff --git a/rules/blacklist.go b/rules/blacklist.go index fbcfff0..9c4e0ba 100644 --- a/rules/blacklist.go +++ b/rules/blacklist.go @@ -32,6 +32,10 @@ func unquote(original string) string { return strings.TrimRight(copy, `"`) } +func (r *blacklistedImport) ID() string { + return r.MetaData.ID +} + func (r *blacklistedImport) Match(n ast.Node, c *gas.Context) (*gas.Issue, error) { if node, ok := n.(*ast.ImportSpec); ok { if description, ok := r.Blacklisted[unquote(node.Path.Value)]; ok { @@ -43,9 +47,10 @@ func (r *blacklistedImport) Match(n ast.Node, c *gas.Context) (*gas.Issue, error // NewBlacklistedImports reports when a blacklisted import is being used. // Typically when a deprecated technology is being used. -func NewBlacklistedImports(conf gas.Config, blacklist map[string]string) (gas.Rule, []ast.Node) { +func NewBlacklistedImports(id string, conf gas.Config, blacklist map[string]string) (gas.Rule, []ast.Node) { return &blacklistedImport{ MetaData: gas.MetaData{ + ID: id, Severity: gas.Medium, Confidence: gas.High, }, @@ -54,29 +59,29 @@ func NewBlacklistedImports(conf gas.Config, blacklist map[string]string) (gas.Ru } // NewBlacklistedImportMD5 fails if MD5 is imported -func NewBlacklistedImportMD5(conf gas.Config) (gas.Rule, []ast.Node) { - return NewBlacklistedImports(conf, map[string]string{ +func NewBlacklistedImportMD5(id string, conf gas.Config) (gas.Rule, []ast.Node) { + return NewBlacklistedImports(id, conf, map[string]string{ "crypto/md5": "Blacklisted import crypto/md5: weak cryptographic primitive", }) } // NewBlacklistedImportDES fails if DES is imported -func NewBlacklistedImportDES(conf gas.Config) (gas.Rule, []ast.Node) { - return NewBlacklistedImports(conf, map[string]string{ +func NewBlacklistedImportDES(id string, conf gas.Config) (gas.Rule, []ast.Node) { + return NewBlacklistedImports(id, conf, map[string]string{ "crypto/des": "Blacklisted import crypto/des: weak cryptographic primitive", }) } // NewBlacklistedImportRC4 fails if DES is imported -func NewBlacklistedImportRC4(conf gas.Config) (gas.Rule, []ast.Node) { - return NewBlacklistedImports(conf, map[string]string{ +func NewBlacklistedImportRC4(id string, conf gas.Config) (gas.Rule, []ast.Node) { + return NewBlacklistedImports(id, conf, map[string]string{ "crypto/rc4": "Blacklisted import crypto/rc4: weak cryptographic primitive", }) } // NewBlacklistedImportCGI fails if CGI is imported -func NewBlacklistedImportCGI(conf gas.Config) (gas.Rule, []ast.Node) { - return NewBlacklistedImports(conf, map[string]string{ +func NewBlacklistedImportCGI(id string, conf gas.Config) (gas.Rule, []ast.Node) { + return NewBlacklistedImports(id, conf, map[string]string{ "net/http/cgi": "Blacklisted import net/http/cgi: Go versions < 1.6.3 are vulnerable to Httpoxy attack: (CVE-2016-5386)", }) } diff --git a/rules/errors.go b/rules/errors.go index cda2087..b913f5c 100644 --- a/rules/errors.go +++ b/rules/errors.go @@ -26,6 +26,10 @@ type noErrorCheck struct { whitelist gas.CallList } +func (r *noErrorCheck) ID() string { + return r.MetaData.ID +} + func returnsError(callExpr *ast.CallExpr, ctx *gas.Context) int { if tv := ctx.Info.TypeOf(callExpr); tv != nil { switch t := tv.(type) { @@ -71,8 +75,7 @@ func (r *noErrorCheck) Match(n ast.Node, ctx *gas.Context) (*gas.Issue, error) { } // NewNoErrorCheck detects if the returned error is unchecked -func NewNoErrorCheck(conf gas.Config) (gas.Rule, []ast.Node) { - +func NewNoErrorCheck(id string, conf gas.Config) (gas.Rule, []ast.Node) { // TODO(gm) Come up with sensible defaults here. Or flip it to use a // black list instead. whitelist := gas.NewCallList() @@ -89,6 +92,7 @@ func NewNoErrorCheck(conf gas.Config) (gas.Rule, []ast.Node) { } return &noErrorCheck{ MetaData: gas.MetaData{ + ID: id, Severity: gas.Low, Confidence: gas.High, What: "Errors unhandled.", diff --git a/rules/fileperms.go b/rules/fileperms.go index b48720f..35dc317 100644 --- a/rules/fileperms.go +++ b/rules/fileperms.go @@ -29,6 +29,10 @@ type filePermissions struct { calls []string } +func (r *filePermissions) ID() string { + return r.MetaData.ID +} + func getConfiguredMode(conf map[string]interface{}, configKey string, defaultMode int64) int64 { var mode = defaultMode if value, ok := conf[configKey]; ok { @@ -58,13 +62,14 @@ func (r *filePermissions) Match(n ast.Node, c *gas.Context) (*gas.Issue, error) // NewFilePerms creates a rule to detect file creation with a more permissive than configured // permission mask. -func NewFilePerms(conf gas.Config) (gas.Rule, []ast.Node) { +func NewFilePerms(id string, conf gas.Config) (gas.Rule, []ast.Node) { mode := getConfiguredMode(conf, "G302", 0600) return &filePermissions{ mode: mode, pkg: "os", calls: []string{"OpenFile", "Chmod"}, MetaData: gas.MetaData{ + ID: id, Severity: gas.Medium, Confidence: gas.High, What: fmt.Sprintf("Expect file permissions to be %#o or less", mode), @@ -74,13 +79,14 @@ func NewFilePerms(conf gas.Config) (gas.Rule, []ast.Node) { // NewMkdirPerms creates a rule to detect directory creation with more permissive than // configured permission mask. -func NewMkdirPerms(conf gas.Config) (gas.Rule, []ast.Node) { +func NewMkdirPerms(id string, conf gas.Config) (gas.Rule, []ast.Node) { mode := getConfiguredMode(conf, "G301", 0750) return &filePermissions{ mode: mode, pkg: "os", calls: []string{"Mkdir", "MkdirAll"}, MetaData: gas.MetaData{ + ID: id, Severity: gas.Medium, Confidence: gas.High, What: fmt.Sprintf("Expect directory permissions to be %#o or less", mode), diff --git a/rules/hardcoded_credentials.go b/rules/hardcoded_credentials.go index bbba8ca..44f4011 100644 --- a/rules/hardcoded_credentials.go +++ b/rules/hardcoded_credentials.go @@ -33,6 +33,10 @@ type credentials struct { ignoreEntropy bool } +func (r *credentials) ID() string { + return r.MetaData.ID +} + func truncate(s string, n int) string { if n > len(s) { return s @@ -102,7 +106,7 @@ func (r *credentials) matchGenDecl(decl *ast.GenDecl, ctx *gas.Context) (*gas.Is // NewHardcodedCredentials attempts to find high entropy string constants being // assigned to variables that appear to be related to credentials. -func NewHardcodedCredentials(conf gas.Config) (gas.Rule, []ast.Node) { +func NewHardcodedCredentials(id string, conf gas.Config) (gas.Rule, []ast.Node) { pattern := `(?i)passwd|pass|password|pwd|secret|token` entropyThreshold := 80.0 perCharThreshold := 3.0 @@ -142,6 +146,7 @@ func NewHardcodedCredentials(conf gas.Config) (gas.Rule, []ast.Node) { ignoreEntropy: ignoreEntropy, truncate: truncateString, MetaData: gas.MetaData{ + ID: id, What: "Potential hardcoded credentials", Confidence: gas.Low, Severity: gas.High, diff --git a/rules/rand.go b/rules/rand.go index ace2e02..7d9673a 100644 --- a/rules/rand.go +++ b/rules/rand.go @@ -26,6 +26,10 @@ type weakRand struct { packagePath string } +func (w *weakRand) ID() string { + return w.MetaData.ID +} + func (w *weakRand) Match(n ast.Node, c *gas.Context) (*gas.Issue, error) { for _, funcName := range w.funcNames { if _, matched := gas.MatchCallByPackage(n, c, w.packagePath, funcName); matched { @@ -37,11 +41,12 @@ func (w *weakRand) Match(n ast.Node, c *gas.Context) (*gas.Issue, error) { } // NewWeakRandCheck detects the use of random number generator that isn't cryptographically secure -func NewWeakRandCheck(conf gas.Config) (gas.Rule, []ast.Node) { +func NewWeakRandCheck(id string, conf gas.Config) (gas.Rule, []ast.Node) { return &weakRand{ funcNames: []string{"Read", "Int"}, packagePath: "math/rand", MetaData: gas.MetaData{ + ID: id, Severity: gas.High, Confidence: gas.Medium, What: "Use of weak random number generator (math/rand instead of crypto/rand)", diff --git a/rules/rsa.go b/rules/rsa.go index 1394da4..4ca922a 100644 --- a/rules/rsa.go +++ b/rules/rsa.go @@ -27,6 +27,10 @@ type weakKeyStrength struct { bits int } +func (w *weakKeyStrength) ID() string { + return w.MetaData.ID +} + func (w *weakKeyStrength) Match(n ast.Node, c *gas.Context) (*gas.Issue, error) { if callExpr := w.calls.ContainsCallExpr(n, c); callExpr != nil { if bits, err := gas.GetInt(callExpr.Args[1]); err == nil && bits < (int64)(w.bits) { @@ -37,7 +41,7 @@ func (w *weakKeyStrength) Match(n ast.Node, c *gas.Context) (*gas.Issue, error) } // NewWeakKeyStrength builds a rule that detects RSA keys < 2048 bits -func NewWeakKeyStrength(conf gas.Config) (gas.Rule, []ast.Node) { +func NewWeakKeyStrength(id string, conf gas.Config) (gas.Rule, []ast.Node) { calls := gas.NewCallList() calls.Add("crypto/rsa", "GenerateKey") bits := 2048 @@ -45,6 +49,7 @@ func NewWeakKeyStrength(conf gas.Config) (gas.Rule, []ast.Node) { calls: calls, bits: bits, MetaData: gas.MetaData{ + ID: id, Severity: gas.Medium, Confidence: gas.High, What: fmt.Sprintf("RSA keys should be at least %d bits", bits), diff --git a/rules/rulelist.go b/rules/rulelist.go index 2846368..39b6dc5 100644 --- a/rules/rulelist.go +++ b/rules/rulelist.go @@ -21,6 +21,7 @@ import ( // RuleDefinition contains the description of a rule and a mechanism to // create it. type RuleDefinition struct { + ID string Description string Create gas.RuleBuilder } @@ -29,10 +30,10 @@ type RuleDefinition struct { type RuleList map[string]RuleDefinition // Builders returns all the create methods for a given rule list -func (rl RuleList) Builders() []gas.RuleBuilder { - builders := make([]gas.RuleBuilder, 0, len(rl)) +func (rl RuleList) Builders() map[string]gas.RuleBuilder { + builders := make(map[string]gas.RuleBuilder) for _, def := range rl { - builders = append(builders, def.Create) + builders[def.ID] = def.Create } return builders } @@ -58,45 +59,49 @@ func NewRuleFilter(action bool, ruleIDs ...string) RuleFilter { // Generate the list of rules to use func Generate(filters ...RuleFilter) RuleList { - rules := map[string]RuleDefinition{ + rules := []RuleDefinition{ // misc - "G101": {"Look for hardcoded credentials", NewHardcodedCredentials}, - "G102": {"Bind to all interfaces", NewBindsToAllNetworkInterfaces}, - "G103": {"Audit the use of unsafe block", NewUsingUnsafe}, - "G104": {"Audit errors not checked", NewNoErrorCheck}, - "G105": {"Audit the use of big.Exp function", NewUsingBigExp}, - "G106": {"Audit the use of ssh.InsecureIgnoreHostKey function", NewSSHHostKey}, + RuleDefinition{"G101", "Look for hardcoded credentials", NewHardcodedCredentials}, + RuleDefinition{"G102", "Bind to all interfaces", NewBindsToAllNetworkInterfaces}, + RuleDefinition{"G103", "Audit the use of unsafe block", NewUsingUnsafe}, + RuleDefinition{"G104", "Audit errors not checked", NewNoErrorCheck}, + RuleDefinition{"G105", "Audit the use of big.Exp function", NewUsingBigExp}, + RuleDefinition{"G106", "Audit the use of ssh.InsecureIgnoreHostKey function", NewSSHHostKey}, // injection - "G201": {"SQL query construction using format string", NewSQLStrFormat}, - "G202": {"SQL query construction using string concatenation", NewSQLStrConcat}, - "G203": {"Use of unescaped data in HTML templates", NewTemplateCheck}, - "G204": {"Audit use of command execution", NewSubproc}, + RuleDefinition{"G201", "SQL query construction using format string", NewSQLStrFormat}, + RuleDefinition{"G202", "SQL query construction using string concatenation", NewSQLStrConcat}, + RuleDefinition{"G203", "Use of unescaped data in HTML templates", NewTemplateCheck}, + RuleDefinition{"G204", "Audit use of command execution", NewSubproc}, // filesystem - "G301": {"Poor file permissions used when creating a directory", NewMkdirPerms}, - "G302": {"Poor file permisions used when creation file or using chmod", NewFilePerms}, - "G303": {"Creating tempfile using a predictable path", NewBadTempFile}, + RuleDefinition{"G301", "Poor file permissions used when creating a directory", NewMkdirPerms}, + RuleDefinition{"G302", "Poor file permisions used when creation file or using chmod", NewFilePerms}, + RuleDefinition{"G303", "Creating tempfile using a predictable path", NewBadTempFile}, // crypto - "G401": {"Detect the usage of DES, RC4, or MD5", 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}, + RuleDefinition{"G401", "Detect the usage of DES, RC4, or MD5", NewUsesWeakCryptography}, + RuleDefinition{"G402", "Look for bad TLS connection settings", NewIntermediateTLSCheck}, + RuleDefinition{"G403", "Ensure minimum RSA key length of 2048 bits", NewWeakKeyStrength}, + RuleDefinition{"G404", "Insecure random number source (rand)", NewWeakRandCheck}, // blacklist - "G501": {"Import blacklist: crypto/md5", NewBlacklistedImportMD5}, - "G502": {"Import blacklist: crypto/des", NewBlacklistedImportDES}, - "G503": {"Import blacklist: crypto/rc4", NewBlacklistedImportRC4}, - "G504": {"Import blacklist: net/http/cgi", NewBlacklistedImportCGI}, + RuleDefinition{"G501", "Import blacklist: crypto/md5", NewBlacklistedImportMD5}, + RuleDefinition{"G502", "Import blacklist: crypto/des", NewBlacklistedImportDES}, + RuleDefinition{"G503", "Import blacklist: crypto/rc4", NewBlacklistedImportRC4}, + RuleDefinition{"G504", "Import blacklist: net/http/cgi", NewBlacklistedImportCGI}, } - for rule := range rules { + ruleMap := make(map[string]RuleDefinition) + +RULES: + for _, rule := range rules { for _, filter := range filters { - if filter(rule) { - delete(rules, rule) + if filter(rule.ID) { + continue RULES } } + ruleMap[rule.ID] = rule } - return rules + return ruleMap } diff --git a/rules/rules_test.go b/rules/rules_test.go index a7dca95..9f0136c 100644 --- a/rules/rules_test.go +++ b/rules/rules_test.go @@ -26,7 +26,7 @@ var _ = Describe("gas rules", func() { config = gas.NewConfig() analyzer = gas.NewAnalyzer(config, logger) runner = func(rule string, samples []testutils.CodeSample) { - analyzer.LoadRules(rules.Generate(rules.NewRuleFilter(false, rule)).Builders()...) + analyzer.LoadRules(rules.Generate(rules.NewRuleFilter(false, rule)).Builders()) for n, sample := range samples { analyzer.Reset() pkg := testutils.NewTestPackage() diff --git a/rules/sql.go b/rules/sql.go index c6505e3..521b103 100644 --- a/rules/sql.go +++ b/rules/sql.go @@ -28,6 +28,10 @@ type sqlStatement struct { patterns []*regexp.Regexp } +func (r *sqlStatement) ID() string { + return r.MetaData.ID +} + // See if the string matches the patterns for the statement. func (s sqlStatement) MatchPatterns(str string) bool { for _, pattern := range s.patterns { @@ -42,6 +46,10 @@ type sqlStrConcat struct { sqlStatement } +func (r *sqlStrConcat) ID() string { + return r.MetaData.ID +} + // see if we can figure out what it is func (s *sqlStrConcat) checkObject(n *ast.Ident) bool { if n.Obj != nil { @@ -72,13 +80,14 @@ func (s *sqlStrConcat) Match(n ast.Node, c *gas.Context) (*gas.Issue, error) { } // NewSQLStrConcat looks for cases where we are building SQL strings via concatenation -func NewSQLStrConcat(conf gas.Config) (gas.Rule, []ast.Node) { +func NewSQLStrConcat(id string, conf gas.Config) (gas.Rule, []ast.Node) { return &sqlStrConcat{ sqlStatement: sqlStatement{ patterns: []*regexp.Regexp{ regexp.MustCompile(`(?)(SELECT|DELETE|INSERT|UPDATE|INTO|FROM|WHERE) `), }, MetaData: gas.MetaData{ + ID: id, Severity: gas.Medium, Confidence: gas.High, What: "SQL string concatenation", @@ -105,7 +114,7 @@ func (s *sqlStrFormat) Match(n ast.Node, c *gas.Context) (*gas.Issue, error) { } // NewSQLStrFormat looks for cases where we're building SQL query strings using format strings -func NewSQLStrFormat(conf gas.Config) (gas.Rule, []ast.Node) { +func NewSQLStrFormat(id string, conf gas.Config) (gas.Rule, []ast.Node) { rule := &sqlStrFormat{ calls: gas.NewCallList(), sqlStatement: sqlStatement{ @@ -114,6 +123,7 @@ func NewSQLStrFormat(conf gas.Config) (gas.Rule, []ast.Node) { regexp.MustCompile("%[^bdoxXfFp]"), }, MetaData: gas.MetaData{ + ID: id, Severity: gas.Medium, Confidence: gas.High, What: "SQL string formatting", diff --git a/rules/ssh.go b/rules/ssh.go index 99b7e27..049b408 100644 --- a/rules/ssh.go +++ b/rules/ssh.go @@ -12,6 +12,10 @@ type sshHostKey struct { calls []string } +func (r *sshHostKey) ID() string { + return r.MetaData.ID +} + func (r *sshHostKey) Match(n ast.Node, c *gas.Context) (gi *gas.Issue, err error) { if _, matches := gas.MatchCallByPackage(n, c, r.pkg, r.calls...); matches { return gas.NewIssue(c, n, r.What, r.Severity, r.Confidence), nil @@ -20,11 +24,12 @@ func (r *sshHostKey) Match(n ast.Node, c *gas.Context) (gi *gas.Issue, err error } // NewSSHHostKey rule detects the use of insecure ssh HostKeyCallback. -func NewSSHHostKey(conf gas.Config) (gas.Rule, []ast.Node) { +func NewSSHHostKey(id string, conf gas.Config) (gas.Rule, []ast.Node) { return &sshHostKey{ pkg: "golang.org/x/crypto/ssh", calls: []string{"InsecureIgnoreHostKey"}, MetaData: gas.MetaData{ + ID: id, What: "Use of ssh InsecureIgnoreHostKey should be audited", Severity: gas.Medium, Confidence: gas.High, diff --git a/rules/subproc.go b/rules/subproc.go index 4ddd8bd..0fd1376 100644 --- a/rules/subproc.go +++ b/rules/subproc.go @@ -22,9 +22,14 @@ import ( ) type subprocess struct { + gas.MetaData gas.CallList } +func (r *subprocess) ID() string { + return r.MetaData.ID +} + // TODO(gm) The only real potential for command injection with a Go project // is something like this: // @@ -50,8 +55,8 @@ func (r *subprocess) Match(n ast.Node, c *gas.Context) (*gas.Issue, error) { } // NewSubproc detects cases where we are forking out to an external process -func NewSubproc(conf gas.Config) (gas.Rule, []ast.Node) { - rule := &subprocess{gas.NewCallList()} +func NewSubproc(id string, conf gas.Config) (gas.Rule, []ast.Node) { + rule := &subprocess{gas.MetaData{ID: id}, gas.NewCallList()} rule.Add("os/exec", "Command") rule.Add("syscall", "Exec") return rule, []ast.Node{(*ast.CallExpr)(nil)} diff --git a/rules/tempfiles.go b/rules/tempfiles.go index 9af500d..3f9e8af 100644 --- a/rules/tempfiles.go +++ b/rules/tempfiles.go @@ -27,6 +27,10 @@ type badTempFile struct { args *regexp.Regexp } +func (t *badTempFile) ID() string { + return t.MetaData.ID +} + func (t *badTempFile) Match(n ast.Node, c *gas.Context) (gi *gas.Issue, err error) { if node := t.calls.ContainsCallExpr(n, c); node != nil { if arg, e := gas.GetString(node.Args[0]); t.args.MatchString(arg) && e == nil { @@ -37,7 +41,7 @@ func (t *badTempFile) Match(n ast.Node, c *gas.Context) (gi *gas.Issue, err erro } // NewBadTempFile detects direct writes to predictable path in temporary directory -func NewBadTempFile(conf gas.Config) (gas.Rule, []ast.Node) { +func NewBadTempFile(id string, conf gas.Config) (gas.Rule, []ast.Node) { calls := gas.NewCallList() calls.Add("io/ioutil", "WriteFile") calls.Add("os", "Create") @@ -45,6 +49,7 @@ func NewBadTempFile(conf gas.Config) (gas.Rule, []ast.Node) { calls: calls, args: regexp.MustCompile(`^/tmp/.*$|^/var/tmp/.*$`), MetaData: gas.MetaData{ + ID: id, Severity: gas.Medium, Confidence: gas.High, What: "File creation in shared tmp directory without using ioutil.Tempfile", diff --git a/rules/templates.go b/rules/templates.go index 4c09ad9..638d020 100644 --- a/rules/templates.go +++ b/rules/templates.go @@ -25,6 +25,10 @@ type templateCheck struct { calls gas.CallList } +func (t *templateCheck) ID() string { + return t.MetaData.ID +} + func (t *templateCheck) Match(n ast.Node, c *gas.Context) (*gas.Issue, error) { if node := t.calls.ContainsCallExpr(n, c); node != nil { for _, arg := range node.Args { @@ -38,7 +42,7 @@ func (t *templateCheck) Match(n ast.Node, c *gas.Context) (*gas.Issue, error) { // NewTemplateCheck constructs the template check rule. This rule is used to // find use of tempaltes where HTML/JS escaping is not being used -func NewTemplateCheck(conf gas.Config) (gas.Rule, []ast.Node) { +func NewTemplateCheck(id string, conf gas.Config) (gas.Rule, []ast.Node) { calls := gas.NewCallList() calls.Add("html/template", "HTML") @@ -48,6 +52,7 @@ func NewTemplateCheck(conf gas.Config) (gas.Rule, []ast.Node) { return &templateCheck{ calls: calls, MetaData: gas.MetaData{ + ID: id, Severity: gas.Medium, Confidence: gas.Low, What: "this method will not auto-escape HTML. Verify data is well formed.", diff --git a/rules/tls.go b/rules/tls.go index 2dce11d..e3af082 100644 --- a/rules/tls.go +++ b/rules/tls.go @@ -24,12 +24,17 @@ import ( ) type insecureConfigTLS struct { + gas.MetaData MinVersion int16 MaxVersion int16 requiredType string goodCiphers []string } +func (t *insecureConfigTLS) ID() string { + return t.MetaData.ID +} + func stringInSlice(a string, list []string) bool { for _, b := range list { if b == a { diff --git a/rules/tls_config.go b/rules/tls_config.go index 4f7afd3..a226d2e 100644 --- a/rules/tls_config.go +++ b/rules/tls_config.go @@ -8,8 +8,9 @@ import ( // NewModernTLSCheck creates a check for Modern TLS ciphers // DO NOT EDIT - generated by tlsconfig tool -func NewModernTLSCheck(conf gas.Config) (gas.Rule, []ast.Node) { +func NewModernTLSCheck(id string, conf gas.Config) (gas.Rule, []ast.Node) { return &insecureConfigTLS{ + MetaData: gas.MetaData{ID: id}, requiredType: "crypto/tls.Config", MinVersion: 0x0303, MaxVersion: 0x0303, @@ -28,8 +29,9 @@ func NewModernTLSCheck(conf gas.Config) (gas.Rule, []ast.Node) { // NewIntermediateTLSCheck creates a check for Intermediate TLS ciphers // DO NOT EDIT - generated by tlsconfig tool -func NewIntermediateTLSCheck(conf gas.Config) (gas.Rule, []ast.Node) { +func NewIntermediateTLSCheck(id string, conf gas.Config) (gas.Rule, []ast.Node) { return &insecureConfigTLS{ + MetaData: gas.MetaData{ID: id}, requiredType: "crypto/tls.Config", MinVersion: 0x0301, MaxVersion: 0x0303, @@ -68,8 +70,9 @@ func NewIntermediateTLSCheck(conf gas.Config) (gas.Rule, []ast.Node) { // NewOldTLSCheck creates a check for Old TLS ciphers // DO NOT EDIT - generated by tlsconfig tool -func NewOldTLSCheck(conf gas.Config) (gas.Rule, []ast.Node) { +func NewOldTLSCheck(id string, conf gas.Config) (gas.Rule, []ast.Node) { return &insecureConfigTLS{ + MetaData: gas.MetaData{ID: id}, requiredType: "crypto/tls.Config", MinVersion: 0x0300, MaxVersion: 0x0303, diff --git a/rules/unsafe.go b/rules/unsafe.go index 81b41c6..69fb601 100644 --- a/rules/unsafe.go +++ b/rules/unsafe.go @@ -26,6 +26,10 @@ type usingUnsafe struct { calls []string } +func (r *usingUnsafe) ID() string { + return r.MetaData.ID +} + func (r *usingUnsafe) Match(n ast.Node, c *gas.Context) (gi *gas.Issue, err error) { if _, matches := gas.MatchCallByPackage(n, c, r.pkg, r.calls...); matches { return gas.NewIssue(c, n, r.What, r.Severity, r.Confidence), nil @@ -35,11 +39,12 @@ func (r *usingUnsafe) Match(n ast.Node, c *gas.Context) (gi *gas.Issue, err erro // NewUsingUnsafe rule detects the use of the unsafe package. This is only // really useful for auditing purposes. -func NewUsingUnsafe(conf gas.Config) (gas.Rule, []ast.Node) { +func NewUsingUnsafe(id string, conf gas.Config) (gas.Rule, []ast.Node) { return &usingUnsafe{ pkg: "unsafe", calls: []string{"Alignof", "Offsetof", "Sizeof", "Pointer"}, MetaData: gas.MetaData{ + ID: id, What: "Use of unsafe calls should be audited", Severity: gas.Low, Confidence: gas.High, diff --git a/rules/weakcrypto.go b/rules/weakcrypto.go index d3adfdc..e64a617 100644 --- a/rules/weakcrypto.go +++ b/rules/weakcrypto.go @@ -25,8 +25,11 @@ type usesWeakCryptography struct { blacklist map[string][]string } -func (r *usesWeakCryptography) Match(n ast.Node, c *gas.Context) (*gas.Issue, error) { +func (r *usesWeakCryptography) ID() string { + return r.MetaData.ID +} +func (r *usesWeakCryptography) Match(n ast.Node, c *gas.Context) (*gas.Issue, error) { for pkg, funcs := range r.blacklist { if _, matched := gas.MatchCallByPackage(n, c, pkg, funcs...); matched { return gas.NewIssue(c, n, r.What, r.Severity, r.Confidence), nil @@ -36,7 +39,7 @@ func (r *usesWeakCryptography) Match(n ast.Node, c *gas.Context) (*gas.Issue, er } // NewUsesWeakCryptography detects uses of des.* md5.* or rc4.* -func NewUsesWeakCryptography(conf gas.Config) (gas.Rule, []ast.Node) { +func NewUsesWeakCryptography(id string, conf gas.Config) (gas.Rule, []ast.Node) { calls := make(map[string][]string) calls["crypto/des"] = []string{"NewCipher", "NewTripleDESCipher"} calls["crypto/md5"] = []string{"New", "Sum"} @@ -44,6 +47,7 @@ func NewUsesWeakCryptography(conf gas.Config) (gas.Rule, []ast.Node) { rule := &usesWeakCryptography{ blacklist: calls, MetaData: gas.MetaData{ + ID: id, Severity: gas.Medium, Confidence: gas.High, What: "Use of weak cryptographic primitive", From 105edba686c5ed5e106e4ecdcbe7768f0582732d Mon Sep 17 00:00:00 2001 From: Jon McClintock Date: Fri, 2 Mar 2018 23:52:39 +0000 Subject: [PATCH 08/11] Leftover from merge. --- rulelist.go | 97 ----------------------------------------------------- 1 file changed, 97 deletions(-) delete mode 100644 rulelist.go diff --git a/rulelist.go b/rulelist.go deleted file mode 100644 index e6ce155..0000000 --- a/rulelist.go +++ /dev/null @@ -1,97 +0,0 @@ -// (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 main - -import ( - "go/ast" - - gas "github.com/GoASTScanner/gas/core" - "github.com/GoASTScanner/gas/rules" -) - -type RuleInfo struct { - id string - description string - build func(string, map[string]interface{}) (gas.Rule, []ast.Node) -} - -// GetFullRuleList get the full list of all rules available to GAS -func GetFullRuleList() map[string]RuleInfo { - rules := []RuleInfo{ - // misc - RuleInfo{"G101", "Look for hardcoded credentials", rules.NewHardcodedCredentials}, - RuleInfo{"G102", "Bind to all interfaces", rules.NewBindsToAllNetworkInterfaces}, - RuleInfo{"G103", "Audit the use of unsafe block", rules.NewUsingUnsafe}, - RuleInfo{"G104", "Audit errors not checked", rules.NewNoErrorCheck}, - RuleInfo{"G105", "Audit the use of big.Exp function", rules.NewUsingBigExp}, - - // injection - RuleInfo{"G201", "SQL query construction using format string", rules.NewSqlStrFormat}, - RuleInfo{"G202", "SQL query construction using string concatenation", rules.NewSqlStrConcat}, - RuleInfo{"G203", "Use of unescaped data in HTML templates", rules.NewTemplateCheck}, - RuleInfo{"G204", "Audit use of command execution", rules.NewSubproc}, - - // filesystem - RuleInfo{"G301", "Poor file permissions used when creating a directory", rules.NewMkdirPerms}, - RuleInfo{"G302", "Poor file permisions used when creation file or using chmod", rules.NewFilePerms}, - RuleInfo{"G303", "Creating tempfile using a predictable path", rules.NewBadTempFile}, - - // crypto - RuleInfo{"G401", "Detect the usage of DES, RC4, or MD5", rules.NewUsesWeakCryptography}, - RuleInfo{"G402", "Look for bad TLS connection settings", rules.NewIntermediateTlsCheck}, - RuleInfo{"G403", "Ensure minimum RSA key length of 2048 bits", rules.NewWeakKeyStrength}, - RuleInfo{"G404", "Insecure random number source (rand)", rules.NewWeakRandCheck}, - - // blacklist - RuleInfo{"G501", "Import blacklist: crypto/md5", rules.NewBlacklist_crypto_md5}, - RuleInfo{"G502", "Import blacklist: crypto/des", rules.NewBlacklist_crypto_des}, - RuleInfo{"G503", "Import blacklist: crypto/rc4", rules.NewBlacklist_crypto_rc4}, - RuleInfo{"G504", "Import blacklist: net/http/cgi", rules.NewBlacklist_net_http_cgi}, - } - ruleMap := make(map[string]RuleInfo) - for _, v := range rules { - ruleMap[v.id] = v - } - return ruleMap -} - -func AddRules(analyzer *gas.Analyzer, conf map[string]interface{}) { - var all map[string]RuleInfo - - inc := conf["include"].([]string) - exc := conf["exclude"].([]string) - - // add included rules - if len(inc) == 0 { - all = GetFullRuleList() - } else { - all = map[string]RuleInfo{} - tmp := GetFullRuleList() - for _, v := range inc { - if val, ok := tmp[v]; ok { - all[v] = val - } - } - } - - // remove excluded rules - for _, v := range exc { - delete(all, v) - } - - for k, v := range all { - analyzer.AddRule(v.build(k, conf)) - } -} From 6b484e734eff4c9e29f3dcdcc05f3e286d1e69d5 Mon Sep 17 00:00:00 2001 From: Jon McClintock Date: Sat, 3 Mar 2018 00:03:39 +0000 Subject: [PATCH 09/11] Run gofmt --- rules/rulelist.go | 42 +++++++++++++++++++++--------------------- 1 file changed, 21 insertions(+), 21 deletions(-) diff --git a/rules/rulelist.go b/rules/rulelist.go index 39b6dc5..a1733d4 100644 --- a/rules/rulelist.go +++ b/rules/rulelist.go @@ -61,35 +61,35 @@ func NewRuleFilter(action bool, ruleIDs ...string) RuleFilter { func Generate(filters ...RuleFilter) RuleList { rules := []RuleDefinition{ // misc - RuleDefinition{"G101", "Look for hardcoded credentials", NewHardcodedCredentials}, - RuleDefinition{"G102", "Bind to all interfaces", NewBindsToAllNetworkInterfaces}, - RuleDefinition{"G103", "Audit the use of unsafe block", NewUsingUnsafe}, - RuleDefinition{"G104", "Audit errors not checked", NewNoErrorCheck}, - RuleDefinition{"G105", "Audit the use of big.Exp function", NewUsingBigExp}, - RuleDefinition{"G106", "Audit the use of ssh.InsecureIgnoreHostKey function", NewSSHHostKey}, + {"G101", "Look for hardcoded credentials", NewHardcodedCredentials}, + {"G102", "Bind to all interfaces", NewBindsToAllNetworkInterfaces}, + {"G103", "Audit the use of unsafe block", NewUsingUnsafe}, + {"G104", "Audit errors not checked", NewNoErrorCheck}, + {"G105", "Audit the use of big.Exp function", NewUsingBigExp}, + {"G106", "Audit the use of ssh.InsecureIgnoreHostKey function", NewSSHHostKey}, // injection - RuleDefinition{"G201", "SQL query construction using format string", NewSQLStrFormat}, - RuleDefinition{"G202", "SQL query construction using string concatenation", NewSQLStrConcat}, - RuleDefinition{"G203", "Use of unescaped data in HTML templates", NewTemplateCheck}, - RuleDefinition{"G204", "Audit use of command execution", NewSubproc}, + {"G201", "SQL query construction using format string", NewSQLStrFormat}, + {"G202", "SQL query construction using string concatenation", NewSQLStrConcat}, + {"G203", "Use of unescaped data in HTML templates", NewTemplateCheck}, + {"G204", "Audit use of command execution", NewSubproc}, // filesystem - RuleDefinition{"G301", "Poor file permissions used when creating a directory", NewMkdirPerms}, - RuleDefinition{"G302", "Poor file permisions used when creation file or using chmod", NewFilePerms}, - RuleDefinition{"G303", "Creating tempfile using a predictable path", NewBadTempFile}, + {"G301", "Poor file permissions used when creating a directory", NewMkdirPerms}, + {"G302", "Poor file permisions used when creation file or using chmod", NewFilePerms}, + {"G303", "Creating tempfile using a predictable path", NewBadTempFile}, // crypto - RuleDefinition{"G401", "Detect the usage of DES, RC4, or MD5", NewUsesWeakCryptography}, - RuleDefinition{"G402", "Look for bad TLS connection settings", NewIntermediateTLSCheck}, - RuleDefinition{"G403", "Ensure minimum RSA key length of 2048 bits", NewWeakKeyStrength}, - RuleDefinition{"G404", "Insecure random number source (rand)", NewWeakRandCheck}, + {"G401", "Detect the usage of DES, RC4, or MD5", 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}, // blacklist - RuleDefinition{"G501", "Import blacklist: crypto/md5", NewBlacklistedImportMD5}, - RuleDefinition{"G502", "Import blacklist: crypto/des", NewBlacklistedImportDES}, - RuleDefinition{"G503", "Import blacklist: crypto/rc4", NewBlacklistedImportRC4}, - RuleDefinition{"G504", "Import blacklist: net/http/cgi", NewBlacklistedImportCGI}, + {"G501", "Import blacklist: crypto/md5", NewBlacklistedImportMD5}, + {"G502", "Import blacklist: crypto/des", NewBlacklistedImportDES}, + {"G503", "Import blacklist: crypto/rc4", NewBlacklistedImportRC4}, + {"G504", "Import blacklist: net/http/cgi", NewBlacklistedImportCGI}, } ruleMap := make(map[string]RuleDefinition) From 18700c276f87dcd4583513dff2f2408842447719 Mon Sep 17 00:00:00 2001 From: Jon McClintock Date: Sat, 3 Mar 2018 00:04:48 +0000 Subject: [PATCH 10/11] Style tweak --- analyzer.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/analyzer.go b/analyzer.go index 656c45e..d7c2ab9 100644 --- a/analyzer.go +++ b/analyzer.go @@ -165,7 +165,7 @@ func (gas *Analyzer) ignore(n ast.Node) ([]string, bool) { matches := re.FindAllStringSubmatch(group.Text(), -1) // Find the rule IDs to ignore. - ignores := make([]string, 0) + var ignores []string for _, v := range matches { ignores = append(ignores, v[1]) } From 429ac07bbd245e7f9d8d61b5cc1a68063cb63d41 Mon Sep 17 00:00:00 2001 From: Jon McClintock Date: Thu, 8 Mar 2018 19:01:00 +0000 Subject: [PATCH 11/11] Change the exclude syntax to be a part of #nosec --- analyzer.go | 12 ++++++------ analyzer_test.go | 6 +++--- 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/analyzer.go b/analyzer.go index d7c2ab9..25b5318 100644 --- a/analyzer.go +++ b/analyzer.go @@ -154,16 +154,16 @@ func (gas *Analyzer) ignore(n ast.Node) ([]string, bool) { for _, group := range groups { if strings.Contains(group.Text(), "#nosec") { gas.stats.NumNosec++ - return nil, true - } - - if strings.Contains(group.Text(), "#exclude") { - gas.stats.NumNosec++ // Pull out the specific rules that are listed to be ignored. - re := regexp.MustCompile("!(G\\d{3})") + re := regexp.MustCompile("(G\\d{3})") matches := re.FindAllStringSubmatch(group.Text(), -1) + // If no specific rules were given, ignore everything. + if matches == nil || len(matches) == 0 { + return nil, true + } + // Find the rule IDs to ignore. var ignores []string for _, v := range matches { diff --git a/analyzer_test.go b/analyzer_test.go index 3bd947c..aca14a8 100644 --- a/analyzer_test.go +++ b/analyzer_test.go @@ -135,7 +135,7 @@ var _ = Describe("Analyzer", func() { nosecPackage := testutils.NewTestPackage() defer nosecPackage.Close() - nosecSource := strings.Replace(source, "h := md5.New()", "h := md5.New() // #exclude !G401", 1) + nosecSource := strings.Replace(source, "h := md5.New()", "h := md5.New() // #nosec G401", 1) nosecPackage.AddFile("md5.go", nosecSource) nosecPackage.Build() @@ -152,7 +152,7 @@ var _ = Describe("Analyzer", func() { nosecPackage := testutils.NewTestPackage() defer nosecPackage.Close() - nosecSource := strings.Replace(source, "h := md5.New()", "h := md5.New() // #exclude !G301", 1) + nosecSource := strings.Replace(source, "h := md5.New()", "h := md5.New() // #nosec G301", 1) nosecPackage.AddFile("md5.go", nosecSource) nosecPackage.Build() @@ -169,7 +169,7 @@ var _ = Describe("Analyzer", func() { nosecPackage := testutils.NewTestPackage() defer nosecPackage.Close() - nosecSource := strings.Replace(source, "h := md5.New()", "h := md5.New() // #exclude !G301 !G401", 1) + nosecSource := strings.Replace(source, "h := md5.New()", "h := md5.New() // #nosec G301 G401", 1) nosecPackage.AddFile("md5.go", nosecSource) nosecPackage.Build()