Simplify analyzer and command line interface

The analyzer now only handles packages rather than one off files. This simplifies the CLI functionality significantly.
securego · gcmurphy · Jan 5, 2018 · Apr 25, 2017 · Apr 26, 2017 · Apr 26, 2017
commit 026fe4c5346719c83a27e50cb4995d8657605d58
diff --git a/analyzer.go b/analyzer.go
@@ -12,39 +12,22 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
-// Package core holds the central scanning logic used by GAS
+// Package gas holds the central scanning logic used by GAS
 package gas
 
 import (
 	"go/ast"
-	"go/importer"
-	"go/parser"
+	"go/build"
 	"go/token"
 	"go/types"
 	"log"
-	"os"
 	"path"
 	"reflect"
 	"strings"
 
 	"golang.org/x/tools/go/loader"
 )
 
-// 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
-}
-
-func NewImportInfo() *ImportInfo {
-	return &ImportInfo{
-		make(map[string]string),
-		make(map[string]string),
-		make(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.
@@ -55,19 +38,9 @@ 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.
-type Rule interface {
-	Match(ast.Node, *Context) (*Issue, error)
+	Imports  *ImportTracker
 }
 
-// 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
-// type of AST node it is currently visiting.
-type RuleSet map[reflect.Type][]Rule
-
 // Metrics used when reporting information about a scanning run.
 type Metrics struct {
 	NumFiles int `json:"files"`
@@ -84,134 +57,81 @@ type Analyzer struct {
 	context     *Context
 	config      Config
 	logger      *log.Logger
-	Issues      []*Issue `json:"issues"`
-	Stats       *Metrics `json:"metrics"`
+	issues      []*Issue
+	stats       *Metrics
 }
 
 // NewAnalyzer builds a new anaylzer.
-func NewAnalyzer(conf Config, logger *log.Logger) Analyzer {
-	if logger == nil {
-		logger = log.New(os.Stdout, "[gas]", 0)
-	}
+func NewAnalyzer(conf Config, logger *log.Logger) *Analyzer {
 	ignoreNoSec := false
 	if val, err := conf.Get("ignoreNoSec"); err == nil {
 		if override, ok := val.(bool); ok {
 			ignoreNoSec = override
 		}
 	}
-	a := Analyzer{
+	return &Analyzer{
 		ignoreNosec: ignoreNoSec,
 		ruleset:     make(RuleSet),
 		context:     &Context{},
 		config:      conf,
 		logger:      logger,
-		Issues:      make([]*Issue, 0, 16),
-		Stats:       &Metrics{0, 0, 0, 0},
+		issues:      make([]*Issue, 0, 16),
+		stats:       &Metrics{},
 	}
-
-	return a
 }
 
-func (gas *Analyzer) process(filename string, source interface{}) error {
-	mode := parser.ParseComments
-	gas.context.FileSet = token.NewFileSet()
-	root, err := parser.ParseFile(gas.context.FileSet, filename, source, mode)
-	if err == nil {
-		gas.context.Config = gas.config
-		gas.context.Comments = ast.NewCommentMap(gas.context.FileSet, root, root.Comments)
-		gas.context.Root = root
-
-		// here we get type info
-		gas.context.Info = &types.Info{
-			Types:      make(map[ast.Expr]types.TypeAndValue),
-			Defs:       make(map[*ast.Ident]types.Object),
-			Uses:       make(map[*ast.Ident]types.Object),
-			Selections: make(map[*ast.SelectorExpr]*types.Selection),
-			Scopes:     make(map[ast.Node]*types.Scope),
-			Implicits:  make(map[ast.Node]types.Object),
-		}
-
-		conf := types.Config{Importer: importer.Default()}
-		gas.context.Pkg, err = conf.Check("pkg", gas.context.FileSet, []*ast.File{root}, gas.context.Info)
-		if err != nil {
-			// TODO(gm) Type checker not currently considering all files within a package
-			// see: issue #113
-			gas.logger.Printf(`Error during type checking: "%s"`, err)
-			err = nil
-		}
-
-		gas.context.Imports = NewImportInfo()
-		for _, pkg := range gas.context.Pkg.Imports() {
-			gas.context.Imports.Imported[pkg.Path()] = pkg.Name()
-		}
-		ast.Walk(gas, root)
-		gas.Stats.NumFiles++
+func (gas *Analyzer) LoadRules(ruleDefinitions ...RuleBuilder) {
+	for _, builder := range ruleDefinitions {
+		r, nodes := builder(gas.config)
+		gas.ruleset.Register(r, nodes...)
 	}
-	return err
 }
 
-// AddRule adds a rule into a rule set list mapped to the given AST node's type.
-// The node is only needed for its type and is not otherwise used.
-func (gas *Analyzer) AddRule(r Rule, nodes []ast.Node) {
-	for _, n := range nodes {
-		t := reflect.TypeOf(n)
-		if val, ok := gas.ruleset[t]; ok {
-			gas.ruleset[t] = append(val, r)
-		} else {
-			gas.ruleset[t] = []Rule{r}
-		}
-	}
-}
+func (gas *Analyzer) Process(packagePath string) error {
 
-// Process reads in a source file, convert it to an AST and traverse it.
-// Rule methods added with AddRule will be invoked as necessary.
-func (gas *Analyzer) Process(filename string) error {
-	err := gas.process(filename, nil)
-	fun := func(f *token.File) bool {
-		gas.Stats.NumLines += f.LineCount()
-		return true
+	basePackage, err := build.Default.ImportDir(packagePath, build.ImportComment)
+	if err != nil {
+		return err
 	}
-	gas.context.FileSet.Iterate(fun)
-	return err
-}
 
-func (gas *Analyzer) ProcessPackage(prog *loader.Program, pkg *loader.PackageInfo, file *ast.File) error {
+	packageConfig := loader.Config{Build: &build.Default}
+	packageFiles := make([]string, 0)
+	for _, filename := range basePackage.GoFiles {
+		packageFiles = append(packageFiles, path.Join(packagePath, filename))
+	}
 
-	gas.context.FileSet = prog.Fset
-	gas.context.Config = gas.config
-	gas.context.Comments = ast.NewCommentMap(gas.context.FileSet, file, file.Comments)
-	gas.context.Root = file
-	gas.context.Info = &pkg.Info
-	gas.context.Pkg = pkg.Pkg
-	gas.context.Imports = NewImportInfo()
-	for _, imported := range gas.context.Pkg.Imports() {
-		gas.context.Imports.Imported[imported.Path()] = imported.Name()
+	packageConfig.CreateFromFilenames(basePackage.Name, packageFiles...)
+	builtPackage, err := packageConfig.Load()
+	if err != nil {
+		return err
 	}
-	ast.Walk(gas, file)
-	gas.Stats.NumFiles++
-	gas.Stats.NumLines += prog.Fset.File(file.Pos()).LineCount()
-	return nil
-}
 
-// ProcessSource will convert a source code string into an AST and traverse it.
-// Rule methods added with AddRule will be invoked as necessary. The string is
-// identified by the filename given but no file IO will be done.
-func (gas *Analyzer) ProcessSource(filename string, source string) error {
-	err := gas.process(filename, source)
-	fun := func(f *token.File) bool {
-		gas.Stats.NumLines += f.LineCount()
-		return true
+	for _, pkg := range builtPackage.Created {
+		gas.logger.Println("Checking package:", pkg.String())
+		for _, file := range pkg.Files {
+			gas.logger.Println("Checking file:", builtPackage.Fset.File(file.Pos()).Name())
+			gas.context.FileSet = builtPackage.Fset
+			gas.context.Config = gas.config
+			gas.context.Comments = ast.NewCommentMap(gas.context.FileSet, file, file.Comments)
+			gas.context.Root = file
+			gas.context.Info = &pkg.Info
+			gas.context.Pkg = pkg.Pkg
+			gas.context.Imports = NewImportTracker()
+			gas.context.Imports.TrackPackages(gas.context.Pkg.Imports()...)
+			ast.Walk(gas, file)
+			gas.stats.NumFiles++
+			gas.stats.NumLines += builtPackage.Fset.File(file.Pos()).LineCount()
+		}
 	}
-	gas.context.FileSet.Iterate(fun)
-	return err
+	return nil
 }
 
 // ignore a node (and sub-tree) if it is tagged with a "#nosec" comment
 func (gas *Analyzer) ignore(n ast.Node) bool {
 	if groups, ok := gas.context.Comments[n]; ok && !gas.ignoreNosec {
 		for _, group := range groups {
 			if strings.Contains(group.Text(), "#nosec") {
-				gas.Stats.NumNosec++
+				gas.stats.NumNosec++
 				return true
 			}
 		}
@@ -225,38 +145,26 @@ 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
-				}
+		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)
 			}
-			// 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 issue != nil {
+				gas.issues = append(gas.issues, issue)
+				gas.stats.NumFound++
 			}
 		}
 		return gas
 	}
 	return nil
 }
+
+// Report returns the current issues discovered and the metrics about the scan
+func (gas *Analyzer) Report() ([]*Issue, *Metrics) {
+	return gas.issues, gas.stats
+}