diff --git a/analyzer.go b/analyzer.go
index b7a5e8f..25b5318 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
+
+ // Pull out the specific rules that are listed to be ignored.
+ 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 {
+ 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..aca14a8 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() // #nosec 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() // #nosec 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() // #nosec 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 4e560ca..27bfe8b 100644
--- a/cmd/gas/main.go
+++ b/cmd/gas/main.go
@@ -207,7 +207,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/import_tracker_test.go b/import_tracker_test.go
deleted file mode 100644
index a1a46f1..0000000
--- a/import_tracker_test.go
+++ /dev/null
@@ -1,30 +0,0 @@
-package gas_test
-
-import (
- . "github.com/onsi/ginkgo"
- . "github.com/onsi/gomega"
-)
-
-var _ = Describe("ImportTracker", func() {
- var (
- source string
- )
-
- BeforeEach(func() {
- source = `// TODO(gm)`
- })
- Context("when I have a valid go package", func() {
- It("should record all import specs", func() {
- Expect(source).To(Equal(source))
- Skip("Not implemented")
- })
-
- It("should correctly track aliased package imports", func() {
- Skip("Not implemented")
- })
-
- It("should correctly track init only packages", func() {
- Skip("Not implemented")
- })
- })
-})
diff --git a/issue.go b/issue.go
index 75bb157..cf1751b 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 fa84b7d..7cd400a 100644
--- a/rules/hardcoded_credentials.go
+++ b/rules/hardcoded_credentials.go
@@ -32,6 +32,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
@@ -94,7 +98,7 @@ func (r *credentials) matchValueSpec(valueSpec *ast.ValueSpec, ctx *gas.Context)
// 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
@@ -134,6 +138,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 01d567f..d914cfb 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,46 +59,50 @@ 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},
+ {"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
- "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},
+ {"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
- "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},
- "G304": {"File path provided as taint input", NewReadFile},
+ {"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},
+ {"G304", "File path provided as taint input", NewReadFile},
// 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},
+ {"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
- "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},
+ {"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},
}
- 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 cde47e6..d982c8d 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",