Skip to content

Commit 62b5195

Browse files
MVrachevgcmurphy
authored andcommitted
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 <[email protected]>
1 parent 9cdfec4 commit 62b5195

File tree

6 files changed

+117
-18
lines changed

6 files changed

+117
-18
lines changed

analyzer.go

+34-2
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ import (
2626
"path"
2727
"reflect"
2828
"regexp"
29+
"strconv"
2930
"strings"
3031

3132
"golang.org/x/tools/go/loader"
@@ -64,6 +65,7 @@ type Analyzer struct {
6465
logger *log.Logger
6566
issues []*Issue
6667
stats *Metrics
68+
errors map[string][]Error // keys are file paths; values are the golang errors in those files
6769
}
6870

6971
// NewAnalyzer builds a new analyzer.
@@ -83,6 +85,7 @@ func NewAnalyzer(conf Config, logger *log.Logger) *Analyzer {
8385
logger: logger,
8486
issues: make([]*Issue, 0, 16),
8587
stats: &Metrics{},
88+
errors: make(map[string][]Error),
8689
}
8790
}
8891

@@ -129,6 +132,35 @@ func (gosec *Analyzer) Process(buildTags []string, packagePaths ...string) error
129132
if err != nil {
130133
return err
131134
}
135+
for _, packageInfo := range builtPackage.AllPackages {
136+
if len(packageInfo.Errors) != 0 {
137+
for _, packErr := range packageInfo.Errors {
138+
// infoErr contains information about the error
139+
// at index 0 is the file path
140+
// at index 1 is the line; index 2 is for column
141+
// at index 3 is the actual error
142+
infoErr := strings.Split(packErr.Error(), ":")
143+
filePath := infoErr[0]
144+
line, err := strconv.Atoi(infoErr[1])
145+
if err != nil {
146+
return err
147+
}
148+
column, err := strconv.Atoi(infoErr[2])
149+
if err != nil {
150+
return err
151+
}
152+
newErr := NewError(line, column, strings.TrimSpace(infoErr[3]))
153+
154+
if errSlice, ok := gosec.errors[filePath]; ok {
155+
gosec.errors[filePath] = append(errSlice, *newErr)
156+
} else {
157+
errSlice = make([]Error, 0)
158+
gosec.errors[filePath] = append(errSlice, *newErr)
159+
}
160+
}
161+
}
162+
}
163+
sortErrors(gosec.errors) // sorts errors by line and column in the file
132164

133165
for _, pkg := range builtPackage.Created {
134166
gosec.logger.Println("Checking package:", pkg.String())
@@ -233,8 +265,8 @@ func (gosec *Analyzer) Visit(n ast.Node) ast.Visitor {
233265
}
234266

235267
// Report returns the current issues discovered and the metrics about the scan
236-
func (gosec *Analyzer) Report() ([]*Issue, *Metrics) {
237-
return gosec.issues, gosec.stats
268+
func (gosec *Analyzer) Report() ([]*Issue, *Metrics, map[string][]Error) {
269+
return gosec.issues, gosec.stats, gosec.errors
238270
}
239271

240272
// Reset clears state such as context, issues and metrics from the configured analyzer

analyzer_test.go

+33-8
Original file line numberDiff line numberDiff line change
@@ -68,7 +68,7 @@ var _ = Describe("Analyzer", func() {
6868
pkg.Build()
6969
err := analyzer.Process(buildTags, pkg.Path)
7070
Expect(err).ShouldNot(HaveOccurred())
71-
_, metrics := analyzer.Report()
71+
_, metrics, _ := analyzer.Report()
7272
Expect(metrics.NumFiles).To(Equal(2))
7373
})
7474

@@ -90,7 +90,7 @@ var _ = Describe("Analyzer", func() {
9090
pkg2.Build()
9191
err := analyzer.Process(buildTags, pkg1.Path, pkg2.Path)
9292
Expect(err).ShouldNot(HaveOccurred())
93-
_, metrics := analyzer.Report()
93+
_, metrics, _ := analyzer.Report()
9494
Expect(metrics.NumFiles).To(Equal(2))
9595
})
9696

@@ -106,11 +106,36 @@ var _ = Describe("Analyzer", func() {
106106
controlPackage.AddFile("md5.go", source)
107107
controlPackage.Build()
108108
analyzer.Process(buildTags, controlPackage.Path)
109-
controlIssues, _ := analyzer.Report()
109+
controlIssues, _, _ := analyzer.Report()
110110
Expect(controlIssues).Should(HaveLen(sample.Errors))
111111

112112
})
113113

114+
It("should report for Golang errors and invalid files", func() {
115+
analyzer.LoadRules(rules.Generate().Builders())
116+
pkg := testutils.NewTestPackage()
117+
defer pkg.Close()
118+
pkg.AddFile("foo.go", `
119+
package main
120+
func main()
121+
}`)
122+
pkg.Build()
123+
err := analyzer.Process(buildTags, pkg.Path)
124+
Expect(err).ShouldNot(HaveOccurred())
125+
_, _, golangErrors := analyzer.Report()
126+
keys := make([]string, len(golangErrors))
127+
i := 0
128+
for key := range golangErrors {
129+
keys[i] = key
130+
i++
131+
}
132+
fileErr := golangErrors[keys[0]]
133+
Expect(len(fileErr)).To(Equal(1))
134+
Expect(fileErr[0].Line).To(Equal(4))
135+
Expect(fileErr[0].Column).To(Equal(5))
136+
Expect(fileErr[0].Err).Should(MatchRegexp(`expected declaration, found '}'`))
137+
})
138+
114139
It("should not report errors when a nosec comment is present", func() {
115140
// Rule for MD5 weak crypto usage
116141
sample := testutils.SampleCodeG401[0]
@@ -124,7 +149,7 @@ var _ = Describe("Analyzer", func() {
124149
nosecPackage.Build()
125150

126151
analyzer.Process(buildTags, nosecPackage.Path)
127-
nosecIssues, _ := analyzer.Report()
152+
nosecIssues, _, _ := analyzer.Report()
128153
Expect(nosecIssues).Should(BeEmpty())
129154
})
130155

@@ -141,7 +166,7 @@ var _ = Describe("Analyzer", func() {
141166
nosecPackage.Build()
142167

143168
analyzer.Process(buildTags, nosecPackage.Path)
144-
nosecIssues, _ := analyzer.Report()
169+
nosecIssues, _, _ := analyzer.Report()
145170
Expect(nosecIssues).Should(BeEmpty())
146171
})
147172

@@ -158,7 +183,7 @@ var _ = Describe("Analyzer", func() {
158183
nosecPackage.Build()
159184

160185
analyzer.Process(buildTags, nosecPackage.Path)
161-
nosecIssues, _ := analyzer.Report()
186+
nosecIssues, _, _ := analyzer.Report()
162187
Expect(nosecIssues).Should(HaveLen(sample.Errors))
163188
})
164189

@@ -175,7 +200,7 @@ var _ = Describe("Analyzer", func() {
175200
nosecPackage.Build()
176201

177202
analyzer.Process(buildTags, nosecPackage.Path)
178-
nosecIssues, _ := analyzer.Report()
203+
nosecIssues, _, _ := analyzer.Report()
179204
Expect(nosecIssues).Should(BeEmpty())
180205
})
181206

@@ -212,7 +237,7 @@ var _ = Describe("Analyzer", func() {
212237
nosecPackage.Build()
213238

214239
customAnalyzer.Process(buildTags, nosecPackage.Path)
215-
nosecIssues, _ := customAnalyzer.Report()
240+
nosecIssues, _, _ := customAnalyzer.Report()
216241
Expect(nosecIssues).Should(HaveLen(sample.Errors))
217242

218243
})

cmd/gosec/main.go

+6-6
Original file line numberDiff line numberDiff line change
@@ -165,19 +165,19 @@ func loadRules(include, exclude string) rules.RuleList {
165165
return rules.Generate(filters...)
166166
}
167167

168-
func saveOutput(filename, format string, issues []*gosec.Issue, metrics *gosec.Metrics) error {
168+
func saveOutput(filename, format string, issues []*gosec.Issue, metrics *gosec.Metrics, errors map[string][]gosec.Error) error {
169169
if filename != "" {
170170
outfile, err := os.Create(filename)
171171
if err != nil {
172172
return err
173173
}
174174
defer outfile.Close()
175-
err = output.CreateReport(outfile, format, issues, metrics)
175+
err = output.CreateReport(outfile, format, issues, metrics, errors)
176176
if err != nil {
177177
return err
178178
}
179179
} else {
180-
err := output.CreateReport(os.Stdout, format, issues, metrics)
180+
err := output.CreateReport(os.Stdout, format, issues, metrics, errors)
181181
if err != nil {
182182
return err
183183
}
@@ -323,7 +323,7 @@ func main() {
323323
}
324324

325325
// Collect the results
326-
issues, metrics := analyzer.Report()
326+
issues, metrics, errors := analyzer.Report()
327327

328328
// Sort the issue by severity
329329
if *flagSortIssues {
@@ -344,15 +344,15 @@ func main() {
344344
}
345345

346346
// Create output report
347-
if err := saveOutput(*flagOutput, *flagFormat, issues, metrics); err != nil {
347+
if err := saveOutput(*flagOutput, *flagFormat, issues, metrics, errors); err != nil {
348348
logger.Fatal(err)
349349
}
350350

351351
// Finalize logging
352352
logWriter.Close() // #nosec
353353

354354
// Do we have an issue? If so exit 1 unless NoFail is set
355-
if issuesFound && !*flagNoFail {
355+
if issuesFound && !*flagNoFail || len(errors) != 0 {
356356
os.Exit(1)
357357
}
358358
}

errors.go

+34
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
1+
package gosec
2+
3+
import (
4+
"sort"
5+
)
6+
7+
// Error is used when there are golang errors while parsing the AST
8+
type Error struct {
9+
Line int `json:"line"`
10+
Column int `json:"column"`
11+
Err string `json:"error"`
12+
}
13+
14+
// NewError creates Error object
15+
func NewError(line, column int, err string) *Error {
16+
return &Error{
17+
Line: line,
18+
Column: column,
19+
Err: err,
20+
}
21+
}
22+
23+
// sortErros sorts the golang erros by line
24+
func sortErrors(allErrors map[string][]Error) {
25+
26+
for _, errors := range allErrors {
27+
sort.Slice(errors, func(i, j int) bool {
28+
if errors[i].Line == errors[j].Line {
29+
return errors[i].Column <= errors[j].Column
30+
}
31+
return errors[i].Line < errors[j].Line
32+
})
33+
}
34+
}

output/formatter.go

+9-1
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,12 @@ const (
4444
)
4545

4646
var text = `Results:
47+
{{range $filePath,$fileErrors := .Errors}}
48+
Golang errors in file: [{{ $filePath }}]:
49+
{{range $index, $error := $fileErrors}}
50+
> [line {{$error.Line}} : column {{$error.Column}}] - {{$error.Err}}
51+
{{end}}
52+
{{end}}
4753
{{ range $index, $issue := .Issues }}
4854
[{{ $issue.File }}:{{ $issue.Line }}] - {{ $issue.RuleID }}: {{ $issue.What }} (Confidence: {{ $issue.Confidence}}, Severity: {{ $issue.Severity }})
4955
> {{ $issue.Code }}
@@ -58,14 +64,16 @@ Summary:
5864
`
5965

6066
type reportInfo struct {
67+
Errors map[string][]gosec.Error `json:"Golang errors"`
6168
Issues []*gosec.Issue
6269
Stats *gosec.Metrics
6370
}
6471

6572
// CreateReport generates a report based for the supplied issues and metrics given
6673
// the specified format. The formats currently accepted are: json, csv, html and text.
67-
func CreateReport(w io.Writer, format string, issues []*gosec.Issue, metrics *gosec.Metrics) error {
74+
func CreateReport(w io.Writer, format string, issues []*gosec.Issue, metrics *gosec.Metrics, errors map[string][]gosec.Error) error {
6875
data := &reportInfo{
76+
Errors: errors,
6977
Issues: issues,
7078
Stats: metrics,
7179
}

rules/rules_test.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,7 @@ var _ = Describe("gosec rules", func() {
4747
Expect(err).ShouldNot(HaveOccurred())
4848
err = analyzer.Process(buildTags, pkg.Path)
4949
Expect(err).ShouldNot(HaveOccurred())
50-
issues, _ := analyzer.Report()
50+
issues, _, _ := analyzer.Report()
5151
if len(issues) != sample.Errors {
5252
fmt.Println(sample.Code)
5353
}

0 commit comments

Comments
 (0)