diff --git a/issue.go b/issue.go index cf1751b..5ec39bc 100644 --- a/issue.go +++ b/issue.go @@ -38,6 +38,7 @@ const ( type Issue struct { Severity Score `json:"severity"` // issue severity (how problematic it is) Confidence Score `json:"confidence"` // issue confidence (how sure we are we found it) + RuleID string `json:"rule_id"` // Human readable explanation What string `json:"details"` // Human readable explanation File string `json:"file"` // File name we found it in Code string `json:"code"` // Impacted code line @@ -87,7 +88,7 @@ func codeSnippet(file *os.File, start int64, end int64, n ast.Node) (string, err } // NewIssue creates a new Issue -func NewIssue(ctx *Context, node ast.Node, desc string, severity Score, confidence Score) *Issue { +func NewIssue(ctx *Context, node ast.Node, ruleID, desc string, severity Score, confidence Score) *Issue { var code string fobj := ctx.FileSet.File(node.Pos()) name := fobj.Name() @@ -112,6 +113,7 @@ func NewIssue(ctx *Context, node ast.Node, desc string, severity Score, confiden return &Issue{ File: name, Line: line, + RuleID: ruleID, What: desc, Confidence: confidence, Severity: severity, diff --git a/issue_test.go b/issue_test.go index 4a74071..1203697 100644 --- a/issue_test.go +++ b/issue_test.go @@ -37,7 +37,7 @@ var _ = Describe("Issue", func() { ast.Walk(v, ctx.Root) Expect(target).ShouldNot(BeNil()) - issue := gas.NewIssue(ctx, target, "", gas.High, gas.High) + issue := gas.NewIssue(ctx, target, "TEST", "", gas.High, gas.High) Expect(issue).ShouldNot(BeNil()) Expect(issue.Code).Should(MatchRegexp(`"bar"`)) Expect(issue.Line).Should(Equal("2")) diff --git a/rules/big.go b/rules/big.go index 0aec080..f4aeb3e 100644 --- a/rules/big.go +++ b/rules/big.go @@ -32,7 +32,7 @@ func (r *usingBigExp) ID() string { 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 gas.NewIssue(c, n, r.ID(), r.What, r.Severity, r.Confidence), nil } return nil, nil } diff --git a/rules/bind.go b/rules/bind.go index cd16dce..1cd8bf2 100644 --- a/rules/bind.go +++ b/rules/bind.go @@ -39,7 +39,7 @@ func (r *bindsToAllNetworkInterfaces) Match(n ast.Node, c *gas.Context) (*gas.Is } if arg, err := gas.GetString(callExpr.Args[1]); err == nil { if r.pattern.MatchString(arg) { - return gas.NewIssue(c, n, r.What, r.Severity, r.Confidence), nil + return gas.NewIssue(c, n, r.ID(), r.What, r.Severity, r.Confidence), nil } } return nil, nil diff --git a/rules/blacklist.go b/rules/blacklist.go index 9c4e0ba..92d8ed4 100644 --- a/rules/blacklist.go +++ b/rules/blacklist.go @@ -39,7 +39,7 @@ func (r *blacklistedImport) ID() string { 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 { - return gas.NewIssue(c, node, description, r.Severity, r.Confidence), nil + return gas.NewIssue(c, node, r.ID(), description, r.Severity, r.Confidence), nil } } return nil, nil diff --git a/rules/errors.go b/rules/errors.go index b913f5c..03ededf 100644 --- a/rules/errors.go +++ b/rules/errors.go @@ -59,7 +59,7 @@ func (r *noErrorCheck) Match(n ast.Node, ctx *gas.Context) (*gas.Issue, error) { return nil, nil } if id, ok := stmt.Lhs[pos].(*ast.Ident); ok && id.Name == "_" { - return gas.NewIssue(ctx, n, r.What, r.Severity, r.Confidence), nil + return gas.NewIssue(ctx, n, r.ID(), r.What, r.Severity, r.Confidence), nil } } } @@ -67,7 +67,7 @@ func (r *noErrorCheck) Match(n ast.Node, ctx *gas.Context) (*gas.Issue, error) { if callExpr, ok := stmt.X.(*ast.CallExpr); ok && r.whitelist.ContainsCallExpr(stmt.X, ctx) == nil { pos := returnsError(callExpr, ctx) if pos >= 0 { - return gas.NewIssue(ctx, n, r.What, r.Severity, r.Confidence), nil + return gas.NewIssue(ctx, n, r.ID(), r.What, r.Severity, r.Confidence), nil } } } diff --git a/rules/fileperms.go b/rules/fileperms.go index 35dc317..6276c85 100644 --- a/rules/fileperms.go +++ b/rules/fileperms.go @@ -54,7 +54,7 @@ func (r *filePermissions) Match(n ast.Node, c *gas.Context) (*gas.Issue, error) if callexpr, matched := gas.MatchCallByPackage(n, c, r.pkg, r.calls...); matched { modeArg := callexpr.Args[len(callexpr.Args)-1] if mode, err := gas.GetInt(modeArg); err == nil && mode > r.mode { - return gas.NewIssue(c, n, r.What, r.Severity, r.Confidence), nil + return gas.NewIssue(c, n, r.ID(), r.What, r.Severity, r.Confidence), nil } } return nil, nil diff --git a/rules/hardcoded_credentials.go b/rules/hardcoded_credentials.go index 7cd400a..0040710 100644 --- a/rules/hardcoded_credentials.go +++ b/rules/hardcoded_credentials.go @@ -69,7 +69,7 @@ func (r *credentials) matchAssign(assign *ast.AssignStmt, ctx *gas.Context) (*ga for _, e := range assign.Rhs { if val, err := gas.GetString(e); err == nil { if r.ignoreEntropy || (!r.ignoreEntropy && r.isHighEntropyString(val)) { - return gas.NewIssue(ctx, assign, r.What, r.Severity, r.Confidence), nil + return gas.NewIssue(ctx, assign, r.ID(), r.What, r.Severity, r.Confidence), nil } } } @@ -88,7 +88,7 @@ func (r *credentials) matchValueSpec(valueSpec *ast.ValueSpec, ctx *gas.Context) } if val, err := gas.GetString(valueSpec.Values[index]); err == nil { if r.ignoreEntropy || (!r.ignoreEntropy && r.isHighEntropyString(val)) { - return gas.NewIssue(ctx, valueSpec, r.What, r.Severity, r.Confidence), nil + return gas.NewIssue(ctx, valueSpec, r.ID(), r.What, r.Severity, r.Confidence), nil } } } diff --git a/rules/rand.go b/rules/rand.go index 7d9673a..c85f10f 100644 --- a/rules/rand.go +++ b/rules/rand.go @@ -33,7 +33,7 @@ func (w *weakRand) ID() string { 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 { - return gas.NewIssue(c, n, w.What, w.Severity, w.Confidence), nil + return gas.NewIssue(c, n, w.ID(), w.What, w.Severity, w.Confidence), nil } } diff --git a/rules/readfile.go b/rules/readfile.go index ce0c3fb..d6c2186 100644 --- a/rules/readfile.go +++ b/rules/readfile.go @@ -38,7 +38,7 @@ func (r *readfile) Match(n ast.Node, c *gas.Context) (*gas.Issue, error) { if ident, ok := arg.(*ast.Ident); ok { obj := c.Info.ObjectOf(ident) if _, ok := obj.(*types.Var); ok && !gas.TryResolve(ident, c) { - return gas.NewIssue(c, n, r.What, r.Severity, r.Confidence), nil + return gas.NewIssue(c, n, r.ID(), r.What, r.Severity, r.Confidence), nil } } } diff --git a/rules/rsa.go b/rules/rsa.go index 4ca922a..99c6e82 100644 --- a/rules/rsa.go +++ b/rules/rsa.go @@ -34,7 +34,7 @@ func (w *weakKeyStrength) ID() string { 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) { - return gas.NewIssue(c, n, w.What, w.Severity, w.Confidence), nil + return gas.NewIssue(c, n, w.ID(), w.What, w.Severity, w.Confidence), nil } } return nil, nil diff --git a/rules/sql.go b/rules/sql.go index 521b103..2367095 100644 --- a/rules/sql.go +++ b/rules/sql.go @@ -72,7 +72,7 @@ func (s *sqlStrConcat) Match(n ast.Node, c *gas.Context) (*gas.Issue, error) { if second, ok := node.Y.(*ast.Ident); ok && s.checkObject(second) { return nil, nil } - return gas.NewIssue(c, n, s.What, s.Severity, s.Confidence), nil + return gas.NewIssue(c, n, s.ID(), s.What, s.Severity, s.Confidence), nil } } } @@ -107,7 +107,7 @@ func (s *sqlStrFormat) Match(n ast.Node, c *gas.Context) (*gas.Issue, error) { // TODO(gm) improve confidence if database/sql is being used if node := s.calls.ContainsCallExpr(n, c); node != nil { if arg, e := gas.GetString(node.Args[0]); s.MatchPatterns(arg) && e == nil { - return gas.NewIssue(c, n, s.What, s.Severity, s.Confidence), nil + return gas.NewIssue(c, n, s.ID(), s.What, s.Severity, s.Confidence), nil } } return nil, nil diff --git a/rules/ssh.go b/rules/ssh.go index 049b408..f4f18cc 100644 --- a/rules/ssh.go +++ b/rules/ssh.go @@ -18,7 +18,7 @@ func (r *sshHostKey) ID() string { 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 + return gas.NewIssue(c, n, r.ID(), r.What, r.Severity, r.Confidence), nil } return nil, nil } diff --git a/rules/subproc.go b/rules/subproc.go index 0fd1376..8c0f675 100644 --- a/rules/subproc.go +++ b/rules/subproc.go @@ -45,11 +45,11 @@ func (r *subprocess) Match(n ast.Node, c *gas.Context) (*gas.Issue, error) { if ident, ok := arg.(*ast.Ident); ok { obj := c.Info.ObjectOf(ident) if _, ok := obj.(*types.Var); ok && !gas.TryResolve(ident, c) { - return gas.NewIssue(c, n, "Subprocess launched with variable", gas.Medium, gas.High), nil + return gas.NewIssue(c, n, r.ID(), "Subprocess launched with variable", gas.Medium, gas.High), nil } } } - return gas.NewIssue(c, n, "Subprocess launching should be audited", gas.Low, gas.High), nil + return gas.NewIssue(c, n, r.ID(), "Subprocess launching should be audited", gas.Low, gas.High), nil } return nil, nil } diff --git a/rules/tempfiles.go b/rules/tempfiles.go index 3f9e8af..664f774 100644 --- a/rules/tempfiles.go +++ b/rules/tempfiles.go @@ -34,7 +34,7 @@ func (t *badTempFile) ID() string { 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 { - return gas.NewIssue(c, n, t.What, t.Severity, t.Confidence), nil + return gas.NewIssue(c, n, t.ID(), t.What, t.Severity, t.Confidence), nil } } return nil, nil diff --git a/rules/templates.go b/rules/templates.go index 638d020..66c37d6 100644 --- a/rules/templates.go +++ b/rules/templates.go @@ -33,7 +33,7 @@ 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 { if _, ok := arg.(*ast.BasicLit); !ok { // basic lits are safe - return gas.NewIssue(c, n, t.What, t.Severity, t.Confidence), nil + return gas.NewIssue(c, n, t.ID(), t.What, t.Severity, t.Confidence), nil } } } diff --git a/rules/tls.go b/rules/tls.go index e3af082..c437930 100644 --- a/rules/tls.go +++ b/rules/tls.go @@ -51,7 +51,7 @@ func (t *insecureConfigTLS) processTLSCipherSuites(n ast.Node, c *gas.Context) * if ident, ok := cipher.(*ast.SelectorExpr); ok { if !stringInSlice(ident.Sel.Name, t.goodCiphers) { err := fmt.Sprintf("TLS Bad Cipher Suite: %s", ident.Sel.Name) - return gas.NewIssue(c, ident, err, gas.High, gas.High) + return gas.NewIssue(c, ident, t.ID(), err, gas.High, gas.High) } } } @@ -66,39 +66,39 @@ func (t *insecureConfigTLS) processTLSConfVal(n *ast.KeyValueExpr, c *gas.Contex case "InsecureSkipVerify": if node, ok := n.Value.(*ast.Ident); ok { if node.Name != "false" { - return gas.NewIssue(c, n, "TLS InsecureSkipVerify set true.", gas.High, gas.High) + return gas.NewIssue(c, n, t.ID(), "TLS InsecureSkipVerify set true.", gas.High, gas.High) } } else { // TODO(tk): symbol tab look up to get the actual value - return gas.NewIssue(c, n, "TLS InsecureSkipVerify may be true.", gas.High, gas.Low) + return gas.NewIssue(c, n, t.ID(), "TLS InsecureSkipVerify may be true.", gas.High, gas.Low) } case "PreferServerCipherSuites": if node, ok := n.Value.(*ast.Ident); ok { if node.Name == "false" { - return gas.NewIssue(c, n, "TLS PreferServerCipherSuites set false.", gas.Medium, gas.High) + return gas.NewIssue(c, n, t.ID(), "TLS PreferServerCipherSuites set false.", gas.Medium, gas.High) } } else { // TODO(tk): symbol tab look up to get the actual value - return gas.NewIssue(c, n, "TLS PreferServerCipherSuites may be false.", gas.Medium, gas.Low) + return gas.NewIssue(c, n, t.ID(), "TLS PreferServerCipherSuites may be false.", gas.Medium, gas.Low) } case "MinVersion": if ival, ierr := gas.GetInt(n.Value); ierr == nil { if (int16)(ival) < t.MinVersion { - return gas.NewIssue(c, n, "TLS MinVersion too low.", gas.High, gas.High) + return gas.NewIssue(c, n, t.ID(), "TLS MinVersion too low.", gas.High, gas.High) } // TODO(tk): symbol tab look up to get the actual value - return gas.NewIssue(c, n, "TLS MinVersion may be too low.", gas.High, gas.Low) + return gas.NewIssue(c, n, t.ID(), "TLS MinVersion may be too low.", gas.High, gas.Low) } case "MaxVersion": if ival, ierr := gas.GetInt(n.Value); ierr == nil { if (int16)(ival) < t.MaxVersion { - return gas.NewIssue(c, n, "TLS MaxVersion too low.", gas.High, gas.High) + return gas.NewIssue(c, n, t.ID(), "TLS MaxVersion too low.", gas.High, gas.High) } // TODO(tk): symbol tab look up to get the actual value - return gas.NewIssue(c, n, "TLS MaxVersion may be too low.", gas.High, gas.Low) + return gas.NewIssue(c, n, t.ID(), "TLS MaxVersion may be too low.", gas.High, gas.Low) } case "CipherSuites": diff --git a/rules/unsafe.go b/rules/unsafe.go index 69fb601..8742dbc 100644 --- a/rules/unsafe.go +++ b/rules/unsafe.go @@ -32,7 +32,7 @@ func (r *usingUnsafe) ID() string { 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 + return gas.NewIssue(c, n, r.ID(), r.What, r.Severity, r.Confidence), nil } return nil, nil } diff --git a/rules/weakcrypto.go b/rules/weakcrypto.go index e64a617..2fb9686 100644 --- a/rules/weakcrypto.go +++ b/rules/weakcrypto.go @@ -32,7 +32,7 @@ func (r *usesWeakCryptography) ID() string { 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 + return gas.NewIssue(c, n, r.ID(), r.What, r.Severity, r.Confidence), nil } } return nil, nil