From 7a275fd0ad1b9637c04deadcee81799a70c1db0d Mon Sep 17 00:00:00 2001 From: Grant Murphy Date: Mon, 7 Nov 2016 09:13:20 -0800 Subject: [PATCH] MatchCallByPackage updated to avoid GetCallObject There seems to be an inconsistency in the way that the type.Info.Uses map is populated by the type checker in Go 1.5 and the latest release. It is possible to ascertain the package that relates to an object 1.7.x release but this does not work for earlier Go versions. To work around this limitation we now track imports, and monitor if they are aliased or initalization only imports. --- core/analyzer.go | 42 ++++++++++++++++++++++++++++++++++++++++-- core/helpers.go | 44 +++++++++++++++++++++++++++++++++++--------- rules/weakcrypto.go | 23 ++++++++++++++--------- 3 files changed, 89 insertions(+), 20 deletions(-) diff --git a/core/analyzer.go b/core/analyzer.go index ceb3621..82beb76 100644 --- a/core/analyzer.go +++ b/core/analyzer.go @@ -27,6 +27,13 @@ import ( "strings" ) +// ImportInfo is used to track aliased and initialization only imports. +type ImportInfo struct { + Imported map[string]string + Aliased map[string]string + InitOnly map[string]bool +} + // The Context is populated with data parsed from the source code as it is scanned. // It is passed through to all rule functions as they are called. Rules may use // this data in conjunction withe the encoutered AST node. @@ -37,6 +44,7 @@ type Context struct { Pkg *types.Package Root *ast.File Config map[string]interface{} + Imports *ImportInfo } // The Rule interface used by all rules supported by GAS. @@ -77,8 +85,20 @@ func NewAnalyzer(conf map[string]interface{}, logger *log.Logger) Analyzer { ignoreNosec: conf["ignoreNosec"].(bool), ruleset: make(RuleSet), Issues: make([]Issue, 0), - context: Context{token.NewFileSet(), nil, nil, nil, nil, nil}, - logger: logger, + context: Context{ + token.NewFileSet(), + nil, + nil, + nil, + nil, + nil, + &ImportInfo{ + make(map[string]string), + make(map[string]string), + make(map[string]bool), + }, + }, + logger: logger, } // TODO(tkelsey): use the inc/exc lists @@ -110,6 +130,10 @@ func (gas *Analyzer) process(filename string, source interface{}) error { return err } + for _, pkg := range gas.context.Pkg.Imports() { + gas.context.Imports.Imported[pkg.Path()] = pkg.Name() + } + ast.Walk(gas, root) gas.Stats.NumFiles++ } @@ -169,6 +193,20 @@ func (gas *Analyzer) ignore(n ast.Node) bool { // 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 { + if imported.Name != nil { + path := strings.Trim(imported.Path.Value, `"`) + if imported.Name.Name == "_" { + // Initialization import + gas.context.Imports.InitOnly[path] = true + } else { + // Aliased import + gas.context.Imports.Aliased[imported.Name.Name] = path + } + } + } if val, ok := gas.ruleset[reflect.TypeOf(n)]; ok { for _, rule := range val { ret, err := rule.Match(n, &gas.context) diff --git a/core/helpers.go b/core/helpers.go index 81bd9e8..dd25821 100644 --- a/core/helpers.go +++ b/core/helpers.go @@ -47,19 +47,45 @@ func MatchCall(n ast.Node, r *regexp.Regexp) *ast.CallExpr { return nil } -// MatchCallByObject ses the type checker to resolve the associated object with a -// particular *ast.CallExpr. This object is used to determine if the -// package and identifier name matches the passed in parameters. +// MatchCallByPackage ensures that the specified package is imported, +// adjusts the name for any aliases and ignores cases that are +// initialization only imports. // // Usage: -// node, obj := MatchCall(n, ctx, "math/rand", "Read") +// node, matched := MatchCallByPackage(n, ctx, "math/rand", "Read") // -func MatchCallByObject(n ast.Node, c *Context, pkg, name string) (*ast.CallExpr, types.Object) { - call, obj := GetCallObject(n, c) - if obj != nil && obj.Pkg().Path() == pkg && obj.Name() == name { - return call, obj +func MatchCallByPackage(n ast.Node, c *Context, pkg string, names ...string) (*ast.CallExpr, bool) { + + importName, imported := c.Imports.Imported[pkg] + if !imported { + return nil, false } - return nil, nil + + if _, initonly := c.Imports.InitOnly[pkg]; initonly { + return nil, false + } + + if alias, ok := c.Imports.Aliased[pkg]; ok { + importName = alias + } + + switch node := n.(type) { + case *ast.CallExpr: + switch fn := node.Fun.(type) { + case *ast.SelectorExpr: + switch expr := fn.X.(type) { + case *ast.Ident: + if expr.Name == importName { + for _, name := range names { + if fn.Sel.Name == name { + return node, true + } + } + } + } + } + } + return nil, false } // MatchCompLit will match an ast.CompositeLit if its string value obays the given regex. diff --git a/rules/weakcrypto.go b/rules/weakcrypto.go index 8bb919e..a10de70 100644 --- a/rules/weakcrypto.go +++ b/rules/weakcrypto.go @@ -16,33 +16,38 @@ package rules import ( "go/ast" - "regexp" gas "github.com/GoASTScanner/gas/core" ) type UsesWeakCryptography struct { gas.MetaData - pattern *regexp.Regexp + blacklist map[string][]string } func (r *UsesWeakCryptography) Match(n ast.Node, c *gas.Context) (*gas.Issue, error) { - if node := gas.MatchCall(n, r.pattern); node != nil { - return gas.NewIssue(c, n, r.What, r.Severity, r.Confidence), nil + + 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 nil, nil } // Uses des.* md5.* or rc4.* -func NewUsesWeakCryptography(conf map[string]interface{}) (r gas.Rule, n ast.Node) { - r = &UsesWeakCryptography{ - pattern: regexp.MustCompile(`des\.NewCipher|des\.NewTripleDESCipher|md5\.New|md5\.Sum|rc4\.NewCipher`), +func NewUsesWeakCryptography(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"} + calls["crypto/rc4"] = []string{"NewCipher"} + rule := &UsesWeakCryptography{ + blacklist: calls, MetaData: gas.MetaData{ Severity: gas.Medium, Confidence: gas.High, What: "Use of weak cryptographic primitive", }, } - n = (*ast.CallExpr)(nil) - return + return rule, (*ast.CallExpr)(nil) }