Report for Golang errors (#284)

* Report for Golang errors

Right now if you use Gosec to scan invalid go file and if you report the result in a text, JSON, CSV or another file format you will always receive 0 issues.
The reason for that is that Gosec can't parse the AST of invalid go files and thus will not report anything.

The real problem here is that the user will never know about the issue if he generates the output in a file.

Signed-off-by: Martin Vrachev <mvrachev@vmware.com>
This commit is contained in:
Martin Vrachev 2019-02-27 00:24:06 +02:00 committed by Grant Murphy
parent 9cdfec40ca
commit 62b5195dd9
6 changed files with 117 additions and 18 deletions

View file

@ -26,6 +26,7 @@ import (
"path" "path"
"reflect" "reflect"
"regexp" "regexp"
"strconv"
"strings" "strings"
"golang.org/x/tools/go/loader" "golang.org/x/tools/go/loader"
@ -64,6 +65,7 @@ type Analyzer struct {
logger *log.Logger logger *log.Logger
issues []*Issue issues []*Issue
stats *Metrics stats *Metrics
errors map[string][]Error // keys are file paths; values are the golang errors in those files
} }
// NewAnalyzer builds a new analyzer. // NewAnalyzer builds a new analyzer.
@ -83,6 +85,7 @@ func NewAnalyzer(conf Config, logger *log.Logger) *Analyzer {
logger: logger, logger: logger,
issues: make([]*Issue, 0, 16), issues: make([]*Issue, 0, 16),
stats: &Metrics{}, stats: &Metrics{},
errors: make(map[string][]Error),
} }
} }
@ -129,6 +132,35 @@ func (gosec *Analyzer) Process(buildTags []string, packagePaths ...string) error
if err != nil { if err != nil {
return err return err
} }
for _, packageInfo := range builtPackage.AllPackages {
if len(packageInfo.Errors) != 0 {
for _, packErr := range packageInfo.Errors {
// infoErr contains information about the error
// at index 0 is the file path
// at index 1 is the line; index 2 is for column
// at index 3 is the actual error
infoErr := strings.Split(packErr.Error(), ":")
filePath := infoErr[0]
line, err := strconv.Atoi(infoErr[1])
if err != nil {
return err
}
column, err := strconv.Atoi(infoErr[2])
if err != nil {
return err
}
newErr := NewError(line, column, strings.TrimSpace(infoErr[3]))
if errSlice, ok := gosec.errors[filePath]; ok {
gosec.errors[filePath] = append(errSlice, *newErr)
} else {
errSlice = make([]Error, 0)
gosec.errors[filePath] = append(errSlice, *newErr)
}
}
}
}
sortErrors(gosec.errors) // sorts errors by line and column in the file
for _, pkg := range builtPackage.Created { for _, pkg := range builtPackage.Created {
gosec.logger.Println("Checking package:", pkg.String()) gosec.logger.Println("Checking package:", pkg.String())
@ -233,8 +265,8 @@ func (gosec *Analyzer) Visit(n ast.Node) ast.Visitor {
} }
// Report returns the current issues discovered and the metrics about the scan // Report returns the current issues discovered and the metrics about the scan
func (gosec *Analyzer) Report() ([]*Issue, *Metrics) { func (gosec *Analyzer) Report() ([]*Issue, *Metrics, map[string][]Error) {
return gosec.issues, gosec.stats return gosec.issues, gosec.stats, gosec.errors
} }
// Reset clears state such as context, issues and metrics from the configured analyzer // Reset clears state such as context, issues and metrics from the configured analyzer

View file

@ -68,7 +68,7 @@ var _ = Describe("Analyzer", func() {
pkg.Build() pkg.Build()
err := analyzer.Process(buildTags, pkg.Path) err := analyzer.Process(buildTags, pkg.Path)
Expect(err).ShouldNot(HaveOccurred()) Expect(err).ShouldNot(HaveOccurred())
_, metrics := analyzer.Report() _, metrics, _ := analyzer.Report()
Expect(metrics.NumFiles).To(Equal(2)) Expect(metrics.NumFiles).To(Equal(2))
}) })
@ -90,7 +90,7 @@ var _ = Describe("Analyzer", func() {
pkg2.Build() pkg2.Build()
err := analyzer.Process(buildTags, pkg1.Path, pkg2.Path) err := analyzer.Process(buildTags, pkg1.Path, pkg2.Path)
Expect(err).ShouldNot(HaveOccurred()) Expect(err).ShouldNot(HaveOccurred())
_, metrics := analyzer.Report() _, metrics, _ := analyzer.Report()
Expect(metrics.NumFiles).To(Equal(2)) Expect(metrics.NumFiles).To(Equal(2))
}) })
@ -106,11 +106,36 @@ var _ = Describe("Analyzer", func() {
controlPackage.AddFile("md5.go", source) controlPackage.AddFile("md5.go", source)
controlPackage.Build() controlPackage.Build()
analyzer.Process(buildTags, controlPackage.Path) analyzer.Process(buildTags, controlPackage.Path)
controlIssues, _ := analyzer.Report() controlIssues, _, _ := analyzer.Report()
Expect(controlIssues).Should(HaveLen(sample.Errors)) Expect(controlIssues).Should(HaveLen(sample.Errors))
}) })
It("should report for Golang errors and invalid files", func() {
analyzer.LoadRules(rules.Generate().Builders())
pkg := testutils.NewTestPackage()
defer pkg.Close()
pkg.AddFile("foo.go", `
package main
func main()
}`)
pkg.Build()
err := analyzer.Process(buildTags, pkg.Path)
Expect(err).ShouldNot(HaveOccurred())
_, _, golangErrors := analyzer.Report()
keys := make([]string, len(golangErrors))
i := 0
for key := range golangErrors {
keys[i] = key
i++
}
fileErr := golangErrors[keys[0]]
Expect(len(fileErr)).To(Equal(1))
Expect(fileErr[0].Line).To(Equal(4))
Expect(fileErr[0].Column).To(Equal(5))
Expect(fileErr[0].Err).Should(MatchRegexp(`expected declaration, found '}'`))
})
It("should not report errors when a nosec comment is present", func() { It("should not report errors when a nosec comment is present", func() {
// Rule for MD5 weak crypto usage // Rule for MD5 weak crypto usage
sample := testutils.SampleCodeG401[0] sample := testutils.SampleCodeG401[0]
@ -124,7 +149,7 @@ var _ = Describe("Analyzer", func() {
nosecPackage.Build() nosecPackage.Build()
analyzer.Process(buildTags, nosecPackage.Path) analyzer.Process(buildTags, nosecPackage.Path)
nosecIssues, _ := analyzer.Report() nosecIssues, _, _ := analyzer.Report()
Expect(nosecIssues).Should(BeEmpty()) Expect(nosecIssues).Should(BeEmpty())
}) })
@ -141,7 +166,7 @@ var _ = Describe("Analyzer", func() {
nosecPackage.Build() nosecPackage.Build()
analyzer.Process(buildTags, nosecPackage.Path) analyzer.Process(buildTags, nosecPackage.Path)
nosecIssues, _ := analyzer.Report() nosecIssues, _, _ := analyzer.Report()
Expect(nosecIssues).Should(BeEmpty()) Expect(nosecIssues).Should(BeEmpty())
}) })
@ -158,7 +183,7 @@ var _ = Describe("Analyzer", func() {
nosecPackage.Build() nosecPackage.Build()
analyzer.Process(buildTags, nosecPackage.Path) analyzer.Process(buildTags, nosecPackage.Path)
nosecIssues, _ := analyzer.Report() nosecIssues, _, _ := analyzer.Report()
Expect(nosecIssues).Should(HaveLen(sample.Errors)) Expect(nosecIssues).Should(HaveLen(sample.Errors))
}) })
@ -175,7 +200,7 @@ var _ = Describe("Analyzer", func() {
nosecPackage.Build() nosecPackage.Build()
analyzer.Process(buildTags, nosecPackage.Path) analyzer.Process(buildTags, nosecPackage.Path)
nosecIssues, _ := analyzer.Report() nosecIssues, _, _ := analyzer.Report()
Expect(nosecIssues).Should(BeEmpty()) Expect(nosecIssues).Should(BeEmpty())
}) })
@ -212,7 +237,7 @@ var _ = Describe("Analyzer", func() {
nosecPackage.Build() nosecPackage.Build()
customAnalyzer.Process(buildTags, nosecPackage.Path) customAnalyzer.Process(buildTags, nosecPackage.Path)
nosecIssues, _ := customAnalyzer.Report() nosecIssues, _, _ := customAnalyzer.Report()
Expect(nosecIssues).Should(HaveLen(sample.Errors)) Expect(nosecIssues).Should(HaveLen(sample.Errors))
}) })

View file

@ -165,19 +165,19 @@ func loadRules(include, exclude string) rules.RuleList {
return rules.Generate(filters...) return rules.Generate(filters...)
} }
func saveOutput(filename, format string, issues []*gosec.Issue, metrics *gosec.Metrics) error { func saveOutput(filename, format string, issues []*gosec.Issue, metrics *gosec.Metrics, errors map[string][]gosec.Error) error {
if filename != "" { if filename != "" {
outfile, err := os.Create(filename) outfile, err := os.Create(filename)
if err != nil { if err != nil {
return err return err
} }
defer outfile.Close() defer outfile.Close()
err = output.CreateReport(outfile, format, issues, metrics) err = output.CreateReport(outfile, format, issues, metrics, errors)
if err != nil { if err != nil {
return err return err
} }
} else { } else {
err := output.CreateReport(os.Stdout, format, issues, metrics) err := output.CreateReport(os.Stdout, format, issues, metrics, errors)
if err != nil { if err != nil {
return err return err
} }
@ -323,7 +323,7 @@ func main() {
} }
// Collect the results // Collect the results
issues, metrics := analyzer.Report() issues, metrics, errors := analyzer.Report()
// Sort the issue by severity // Sort the issue by severity
if *flagSortIssues { if *flagSortIssues {
@ -344,7 +344,7 @@ func main() {
} }
// Create output report // Create output report
if err := saveOutput(*flagOutput, *flagFormat, issues, metrics); err != nil { if err := saveOutput(*flagOutput, *flagFormat, issues, metrics, errors); err != nil {
logger.Fatal(err) logger.Fatal(err)
} }
@ -352,7 +352,7 @@ func main() {
logWriter.Close() // #nosec logWriter.Close() // #nosec
// Do we have an issue? If so exit 1 unless NoFail is set // Do we have an issue? If so exit 1 unless NoFail is set
if issuesFound && !*flagNoFail { if issuesFound && !*flagNoFail || len(errors) != 0 {
os.Exit(1) os.Exit(1)
} }
} }

34
errors.go Normal file
View file

@ -0,0 +1,34 @@
package gosec
import (
"sort"
)
// Error is used when there are golang errors while parsing the AST
type Error struct {
Line int `json:"line"`
Column int `json:"column"`
Err string `json:"error"`
}
// NewError creates Error object
func NewError(line, column int, err string) *Error {
return &Error{
Line: line,
Column: column,
Err: err,
}
}
// sortErros sorts the golang erros by line
func sortErrors(allErrors map[string][]Error) {
for _, errors := range allErrors {
sort.Slice(errors, func(i, j int) bool {
if errors[i].Line == errors[j].Line {
return errors[i].Column <= errors[j].Column
}
return errors[i].Line < errors[j].Line
})
}
}

View file

@ -44,6 +44,12 @@ const (
) )
var text = `Results: var text = `Results:
{{range $filePath,$fileErrors := .Errors}}
Golang errors in file: [{{ $filePath }}]:
{{range $index, $error := $fileErrors}}
> [line {{$error.Line}} : column {{$error.Column}}] - {{$error.Err}}
{{end}}
{{end}}
{{ range $index, $issue := .Issues }} {{ range $index, $issue := .Issues }}
[{{ $issue.File }}:{{ $issue.Line }}] - {{ $issue.RuleID }}: {{ $issue.What }} (Confidence: {{ $issue.Confidence}}, Severity: {{ $issue.Severity }}) [{{ $issue.File }}:{{ $issue.Line }}] - {{ $issue.RuleID }}: {{ $issue.What }} (Confidence: {{ $issue.Confidence}}, Severity: {{ $issue.Severity }})
> {{ $issue.Code }} > {{ $issue.Code }}
@ -58,14 +64,16 @@ Summary:
` `
type reportInfo struct { type reportInfo struct {
Errors map[string][]gosec.Error `json:"Golang errors"`
Issues []*gosec.Issue Issues []*gosec.Issue
Stats *gosec.Metrics Stats *gosec.Metrics
} }
// CreateReport generates a report based for the supplied issues and metrics given // CreateReport generates a report based for the supplied issues and metrics given
// the specified format. The formats currently accepted are: json, csv, html and text. // the specified format. The formats currently accepted are: json, csv, html and text.
func CreateReport(w io.Writer, format string, issues []*gosec.Issue, metrics *gosec.Metrics) error { func CreateReport(w io.Writer, format string, issues []*gosec.Issue, metrics *gosec.Metrics, errors map[string][]gosec.Error) error {
data := &reportInfo{ data := &reportInfo{
Errors: errors,
Issues: issues, Issues: issues,
Stats: metrics, Stats: metrics,
} }

View file

@ -47,7 +47,7 @@ var _ = Describe("gosec rules", func() {
Expect(err).ShouldNot(HaveOccurred()) Expect(err).ShouldNot(HaveOccurred())
err = analyzer.Process(buildTags, pkg.Path) err = analyzer.Process(buildTags, pkg.Path)
Expect(err).ShouldNot(HaveOccurred()) Expect(err).ShouldNot(HaveOccurred())
issues, _ := analyzer.Report() issues, _, _ := analyzer.Report()
if len(issues) != sample.Errors { if len(issues) != sample.Errors {
fmt.Println(sample.Code) fmt.Println(sample.Code)
} }