Add the rule ID to issues (#188)

This commit is contained in:
jonmcclintock 2018-03-12 01:18:44 -07:00 committed by Grant Murphy
parent a0367559a7
commit 2115402409
19 changed files with 33 additions and 31 deletions

View file

@ -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,

View file

@ -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"))

View file

@ -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
}

View file

@ -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

View file

@ -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

View file

@ -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
}
}
}

View file

@ -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

View file

@ -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
}
}
}

View file

@ -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
}
}

View file

@ -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
}
}
}

View file

@ -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

View file

@ -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

View file

@ -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
}

View file

@ -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
}

View file

@ -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

View file

@ -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
}
}
}

View file

@ -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":

View file

@ -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
}

View file

@ -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