From c4f5932ab7450a6d9be30e15ce8b0d3ee5101f59 Mon Sep 17 00:00:00 2001 From: Matthieu MOREL Date: Fri, 7 May 2021 16:54:34 +0200 Subject: [PATCH] Refactor : Replace Cwe with cwe.Weakness --- README.md | 16 ++++-- cmd/gosec/sort_issues_test.go | 2 +- cwe/data.go | 10 +++- cwe/types.go | 30 +++++++++- issue.go | 101 +++++++++++++++++----------------- issue_test.go | 2 +- report/csv/writer.go | 3 +- report/formatter_test.go | 31 ++++++----- report/golint/writer.go | 2 +- report/sarif/formatter.go | 25 ++++----- report/text/template.go | 2 +- 11 files changed, 128 insertions(+), 96 deletions(-) diff --git a/README.md b/README.md index 7b17467..dbfc96f 100644 --- a/README.md +++ b/README.md @@ -47,6 +47,7 @@ echo " gosec_vX.Y.Z_OS.tar.gz" | sha256sum - gosec --help ``` + ### GitHub Action You can run `gosec` as a GitHub action as follows: @@ -123,7 +124,6 @@ paths, and produce reports in different formats. By default all rules will be run against the supplied input files. To recursively scan from the current directory you can supply `./...` as the input argument. - ### Available rules - G101: Look for hard coded credentials @@ -173,9 +173,10 @@ $ gosec -include=G101,G203,G401 ./... # Run everything except for rule G303 $ gosec -exclude=G303 ./... ``` + ### CWE Mapping -Every issue detected by `gosec` is mapped to a [CWE (Common Weakness Enumeration)](http://cwe.mitre.org/data/index.html) which describes in more generic terms the vulnerability. The exact mapping can be found [here](https://github.com/securego/gosec/blob/master/issue.go#L49). +Every issue detected by `gosec` is mapped to a [CWE (Common Weakness Enumeration)](http://cwe.mitre.org/data/index.html) which describes in more generic terms the vulnerability. The exact mapping can be found [here](https://github.com/securego/gosec/blob/master/issue.go#L50). ### Configuration @@ -197,6 +198,7 @@ A number of global settings can be provided in a configuration file as follows: # Run with a global configuration file $ gosec -conf config.json . ``` + Also some rules accept configuration. For instance on rule `G104`, it is possible to define packages along with a list of functions which will be skipped when auditing the not checked errors: @@ -224,7 +226,7 @@ You can also configure the hard-coded credentials rule `G101` with additional pa ### Dependencies -gosec will fetch automatically the dependencies of the code which is being analyzed when go module is turned on (e.g.` GO111MODULE=on`). If this is not the case, +gosec will fetch automatically the dependencies of the code which is being analyzed when go module is turned on (e.g.`GO111MODULE=on`). If this is not the case, the dependencies need to be explicitly downloaded by running the `go get -d` command before the scan. ### Excluding test files and folders @@ -307,6 +309,7 @@ $ gosec -fmt=json -out=results.json *.go ### Build You can build the binary with: + ```bash make ``` @@ -314,11 +317,13 @@ make ### Note on Sarif Types Generation Install the tool with : + ```bash go get -u github.com/a-h/generate/cmd/schema-generate ``` Then generate the types with : + ```bash schema-generate -i sarif-schema-2.1.0.json -o mypath/types.go ``` @@ -326,10 +331,10 @@ schema-generate -i sarif-schema-2.1.0.json -o mypath/types.go Most of the MarshallJSON/UnmarshalJSON are removed except the one for PropertyBag which is handy to inline the additionnal properties. The rest can be removed. The URI,ID, UUID, GUID were renamed so it fits the Golang convention defined [here](https://github.com/golang/lint/blob/master/lint.go#L700) - ### Tests You can run all unit tests using: + ```bash make test ``` @@ -360,7 +365,8 @@ into a volume as follows: ```bash docker run --rm -it -w // -v /:/ securego/gosec //... ``` -**Note:** the current working directory needs to be set with `-w` option in order to get successfully resolved the dependencies from go module file + +**Note:** the current working directory needs to be set with `-w` option in order to get successfully resolved the dependencies from go module file ### Generate TLS rule diff --git a/cmd/gosec/sort_issues_test.go b/cmd/gosec/sort_issues_test.go index 2b06689..6b4bfc7 100644 --- a/cmd/gosec/sort_issues_test.go +++ b/cmd/gosec/sort_issues_test.go @@ -17,7 +17,7 @@ var defaultIssue = gosec.Issue{ Confidence: gosec.High, Severity: gosec.High, Code: "1: testcode", - Cwe: gosec.GetCwe("G101"), + Cwe: gosec.GetCweByRule("G101"), } func createIssue() gosec.Issue { diff --git a/cwe/data.go b/cwe/data.go index 4b9d375..2553c3c 100644 --- a/cwe/data.go +++ b/cwe/data.go @@ -1,6 +1,6 @@ package cwe -var data = map[string]Weakness{ +var data = map[string]*Weakness{ "118": { ID: "118", Description: "The software does not restrict or incorrectly restricts operations within the boundaries of a resource that is accessed using an index or pointer, such as memory or files.", @@ -104,6 +104,10 @@ var data = map[string]Weakness{ } //Get Retrieves a CWE weakness by it's id -func Get(id string) Weakness { - return data[id] +func Get(id string) *Weakness { + weakness, ok := data[id] + if ok && weakness != nil { + return weakness + } + return nil } diff --git a/cwe/types.go b/cwe/types.go index 428ef8f..45d9cd2 100644 --- a/cwe/types.go +++ b/cwe/types.go @@ -1,9 +1,17 @@ package cwe import ( + "encoding/json" "fmt" ) +const ( + //URL is the base URL for CWE definitions + URL = "https://cwe.mitre.org/data/definitions/" + //Acronym is the acronym of CWE + Acronym = "CWE" +) + // Weakness defines a CWE weakness based on http://cwe.mitre.org/data/xsd/cwe_schema_v6.4.xsd type Weakness struct { ID string @@ -11,7 +19,23 @@ type Weakness struct { Description string } -//URL Expose the CWE URL -func (w *Weakness) URL() string { - return fmt.Sprintf("https://cwe.mitre.org/data/definitions/%s.html", w.ID) +//SprintURL format the CWE URL +func (w *Weakness) SprintURL() string { + return fmt.Sprintf("%s%s.html", URL, w.ID) +} + +//SprintID format the CWE ID +func (w *Weakness) SprintID() string { + return fmt.Sprintf("%s-%s", Acronym, w.ID) +} + +//MarshalJSON print only id and URL +func (w *Weakness) MarshalJSON() ([]byte, error) { + return json.Marshal(&struct { + ID string `json:"id"` + URL string `json:"url"` + }{ + ID: w.ID, + URL: w.SprintURL(), + }) } diff --git a/issue.go b/issue.go index aa58c34..166ee35 100644 --- a/issue.go +++ b/issue.go @@ -19,6 +19,7 @@ import ( "bytes" "encoding/json" "fmt" + "github.com/securego/gosec/v2/cwe" "go/ast" "go/token" "os" @@ -41,62 +42,60 @@ const ( // the beginning and after the end of a code snippet const SnippetOffset = 1 -// Cwe id and url -type Cwe struct { - ID string - URL string +// GetCweByRule retrieves a cwe weakness for a given RuleID +func GetCweByRule(id string) *cwe.Weakness { + cweID, ok := ruleToCWE[id] + if ok && cweID != "" { + return cwe.Get(cweID) + } + return nil } -// GetCwe creates a cwe object for a given RuleID -func GetCwe(id string) Cwe { - return Cwe{ID: id, URL: fmt.Sprintf("https://cwe.mitre.org/data/definitions/%s.html", id)} -} - -// IssueToCWE maps gosec rules to CWEs -var IssueToCWE = map[string]Cwe{ - "G101": GetCwe("798"), - "G102": GetCwe("200"), - "G103": GetCwe("242"), - "G104": GetCwe("703"), - "G106": GetCwe("322"), - "G107": GetCwe("88"), - "G108": GetCwe("200"), - "G109": GetCwe("190"), - "G110": GetCwe("409"), - "G201": GetCwe("89"), - "G202": GetCwe("89"), - "G203": GetCwe("79"), - "G204": GetCwe("78"), - "G301": GetCwe("276"), - "G302": GetCwe("276"), - "G303": GetCwe("377"), - "G304": GetCwe("22"), - "G305": GetCwe("22"), - "G306": GetCwe("276"), - "G307": GetCwe("703"), - "G401": GetCwe("326"), - "G402": GetCwe("295"), - "G403": GetCwe("310"), - "G404": GetCwe("338"), - "G501": GetCwe("327"), - "G502": GetCwe("327"), - "G503": GetCwe("327"), - "G504": GetCwe("327"), - "G505": GetCwe("327"), - "G601": GetCwe("118"), +// ruleToCWE maps gosec rules to CWEs +var ruleToCWE = map[string]string{ + "G101": "798", + "G102": "200", + "G103": "242", + "G104": "703", + "G106": "322", + "G107": "88", + "G108": "200", + "G109": "190", + "G110": "409", + "G201": "89", + "G202": "89", + "G203": "79", + "G204": "78", + "G301": "276", + "G302": "276", + "G303": "377", + "G304": "22", + "G305": "22", + "G306": "276", + "G307": "703", + "G401": "326", + "G402": "295", + "G403": "310", + "G404": "338", + "G501": "327", + "G502": "327", + "G503": "327", + "G504": "327", + "G505": "327", + "G601": "118", } // Issue is returned by a gosec rule if it discovers an issue with the scanned code. 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) - Cwe Cwe `json:"cwe"` // Cwe associated with RuleID - 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 - Line string `json:"line"` // Line number in file - Col string `json:"column"` // Column number in line + Severity Score `json:"severity"` // issue severity (how problematic it is) + Confidence Score `json:"confidence"` // issue confidence (how sure we are we found it) + Cwe *cwe.Weakness `json:"cwe"` // Cwe associated with RuleID + 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 + Line string `json:"line"` // Line number in file + Col string `json:"column"` // Column number in line } // FileLocation point out the file path and line number in file @@ -196,6 +195,6 @@ func NewIssue(ctx *Context, node ast.Node, ruleID, desc string, severity Score, Confidence: confidence, Severity: severity, Code: code, - Cwe: IssueToCWE[ruleID], + Cwe: GetCweByRule(ruleID), } } diff --git a/issue_test.go b/issue_test.go index b7bfe19..12a2405 100644 --- a/issue_test.go +++ b/issue_test.go @@ -42,7 +42,7 @@ var _ = Describe("Issue", func() { Expect(issue.Code).Should(MatchRegexp(`"bar"`)) Expect(issue.Line).Should(Equal("2")) Expect(issue.Col).Should(Equal("16")) - Expect(issue.Cwe.ID).Should(Equal("")) + Expect(issue.Cwe).Should(BeNil()) }) It("should return an error if specific context is not able to be obtained", func() { diff --git a/report/csv/writer.go b/report/csv/writer.go index 799be87..f1658c9 100644 --- a/report/csv/writer.go +++ b/report/csv/writer.go @@ -2,7 +2,6 @@ package csv import ( "encoding/csv" - "fmt" "github.com/securego/gosec/v2/report/core" "io" ) @@ -19,7 +18,7 @@ func WriteReport(w io.Writer, data *core.ReportInfo) error { issue.Severity.String(), issue.Confidence.String(), issue.Code, - fmt.Sprintf("CWE-%s", issue.Cwe.ID), + issue.Cwe.SprintID(), }) if err != nil { return err diff --git a/report/formatter_test.go b/report/formatter_test.go index f501693..59000cb 100644 --- a/report/formatter_test.go +++ b/report/formatter_test.go @@ -9,6 +9,7 @@ import ( . "github.com/onsi/ginkgo" . "github.com/onsi/gomega" "github.com/securego/gosec/v2" + "github.com/securego/gosec/v2/cwe" "github.com/securego/gosec/v2/report/core" "github.com/securego/gosec/v2/report/junit" "github.com/securego/gosec/v2/report/sonar" @@ -16,13 +17,13 @@ import ( ) func createIssueWithFileWhat(file, what string) *gosec.Issue { - issue := createIssue("i1", gosec.GetCwe("G101")) + issue := createIssue("i1", gosec.GetCweByRule("G101")) issue.File = file issue.What = what return &issue } -func createIssue(ruleID string, cwe gosec.Cwe) gosec.Issue { +func createIssue(ruleID string, weakness *cwe.Weakness) gosec.Issue { return gosec.Issue{ File: "/home/src/project/test.go", Line: "1", @@ -32,12 +33,12 @@ func createIssue(ruleID string, cwe gosec.Cwe) gosec.Issue { Confidence: gosec.High, Severity: gosec.High, Code: "1: testcode", - Cwe: cwe, + Cwe: weakness, } } -func createReportInfo(rule string, cwe gosec.Cwe) core.ReportInfo { - issue := createIssue(rule, cwe) +func createReportInfo(rule string, weakness *cwe.Weakness) core.ReportInfo { + issue := createIssue(rule, weakness) metrics := gosec.Metrics{} return core.ReportInfo{ Errors: map[string][]gosec.Error{}, @@ -284,7 +285,7 @@ var _ = Describe("Formatter", func() { It("csv formatted report should contain the CWE mapping", func() { for _, rule := range grules { - cwe := gosec.IssueToCWE[rule] + cwe := gosec.GetCweByRule(rule) issue := createIssue(rule, cwe) error := map[string][]gosec.Error{} @@ -298,7 +299,7 @@ var _ = Describe("Formatter", func() { }) It("xml formatted report should contain the CWE mapping", func() { for _, rule := range grules { - cwe := gosec.IssueToCWE[rule] + cwe := gosec.GetCweByRule(rule) issue := createIssue(rule, cwe) error := map[string][]gosec.Error{} @@ -312,7 +313,7 @@ var _ = Describe("Formatter", func() { }) It("json formatted report should contain the CWE mapping", func() { for _, rule := range grules { - cwe := gosec.IssueToCWE[rule] + cwe := gosec.GetCweByRule(rule) issue := createIssue(rule, cwe) error := map[string][]gosec.Error{} @@ -332,7 +333,7 @@ var _ = Describe("Formatter", func() { }) It("html formatted report should contain the CWE mapping", func() { for _, rule := range grules { - cwe := gosec.IssueToCWE[rule] + cwe := gosec.GetCweByRule(rule) issue := createIssue(rule, cwe) error := map[string][]gosec.Error{} @@ -352,7 +353,7 @@ var _ = Describe("Formatter", func() { }) It("yaml formatted report should contain the CWE mapping", func() { for _, rule := range grules { - cwe := gosec.IssueToCWE[rule] + cwe := gosec.GetCweByRule(rule) issue := createIssue(rule, cwe) error := map[string][]gosec.Error{} @@ -372,7 +373,7 @@ var _ = Describe("Formatter", func() { }) It("junit-xml formatted report should contain the CWE mapping", func() { for _, rule := range grules { - cwe := gosec.IssueToCWE[rule] + cwe := gosec.GetCweByRule(rule) issue := createIssue(rule, cwe) error := map[string][]gosec.Error{} @@ -392,7 +393,7 @@ var _ = Describe("Formatter", func() { }) It("text formatted report should contain the CWE mapping", func() { for _, rule := range grules { - cwe := gosec.IssueToCWE[rule] + cwe := gosec.GetCweByRule(rule) issue := createIssue(rule, cwe) error := map[string][]gosec.Error{} @@ -412,7 +413,7 @@ var _ = Describe("Formatter", func() { }) It("sonarqube formatted report shouldn't contain the CWE mapping", func() { for _, rule := range grules { - cwe := gosec.IssueToCWE[rule] + cwe := gosec.GetCweByRule(rule) issue := createIssue(rule, cwe) error := map[string][]gosec.Error{} buf := new(bytes.Buffer) @@ -432,7 +433,7 @@ var _ = Describe("Formatter", func() { }) It("golint formatted report should contain the CWE mapping", func() { for _, rule := range grules { - cwe := gosec.IssueToCWE[rule] + cwe := gosec.GetCweByRule(rule) issue := createIssue(rule, cwe) error := map[string][]gosec.Error{} @@ -446,7 +447,7 @@ var _ = Describe("Formatter", func() { }) It("sarif formatted report should contain the CWE mapping", func() { for _, rule := range grules { - cwe := gosec.IssueToCWE[rule] + cwe := gosec.GetCweByRule(rule) issue := createIssue(rule, cwe) error := map[string][]gosec.Error{} diff --git a/report/golint/writer.go b/report/golint/writer.go index 39aa400..ed22f37 100644 --- a/report/golint/writer.go +++ b/report/golint/writer.go @@ -15,7 +15,7 @@ func WriteReport(w io.Writer, data *core.ReportInfo) error { for _, issue := range data.Issues { what := issue.What if issue.Cwe.ID != "" { - what = fmt.Sprintf("[CWE-%s] %s", issue.Cwe.ID, issue.What) + what = fmt.Sprintf("[%s] %s", issue.Cwe.SprintID(), issue.What) } // issue.Line uses "start-end" format for multiple line detection. diff --git a/report/sarif/formatter.go b/report/sarif/formatter.go index 6b94474..bee5719 100644 --- a/report/sarif/formatter.go +++ b/report/sarif/formatter.go @@ -19,7 +19,6 @@ const ( sarifNote = sarifLevel("note") sarifWarning = sarifLevel("warning") sarifError = sarifLevel("error") - cweAcronym = "CWE" ) //GenerateReport Convert a gosec report to a Sarif Report @@ -36,7 +35,7 @@ func GenerateReport(rootPaths []string, data *core.ReportInfo) (*Report, error) results := []*Result{} taxa := make([]*ReportingDescriptor, 0) - weaknesses := make(map[string]cwe.Weakness) + weaknesses := make(map[string]*cwe.Weakness) for _, issue := range data.Issues { _, ok := weaknesses[issue.Cwe.ID] @@ -117,18 +116,18 @@ func parseSarifRule(issue *gosec.Issue) *ReportingDescriptor { Level: getSarifLevel(issue.Severity.String()), }, Relationships: []*ReportingDescriptorRelationship{ - buildSarifReportingDescriptorRelationship(issue.Cwe.ID), + buildSarifReportingDescriptorRelationship(issue.Cwe), }, } } -func buildSarifReportingDescriptorRelationship(weaknessID string) *ReportingDescriptorRelationship { +func buildSarifReportingDescriptorRelationship(weakness *cwe.Weakness) *ReportingDescriptorRelationship { return &ReportingDescriptorRelationship{ Target: &ReportingDescriptorReference{ - ID: weaknessID, - GUID: uuid3(cweAcronym + weaknessID), + ID: weakness.ID, + GUID: uuid3(weakness.SprintID()), ToolComponent: &ToolComponentReference{ - Name: cweAcronym, + Name: cwe.Acronym, }, }, Kinds: []string{"superset"}, @@ -149,7 +148,7 @@ func buildSarifTaxonomies(taxa []*ReportingDescriptor) []*ToolComponent { func buildCWETaxonomy(version string, taxa []*ReportingDescriptor) *ToolComponent { return &ToolComponent{ - Name: cweAcronym, + Name: cwe.Acronym, Version: version, ReleaseDateUtc: "2021-03-15", InformationURI: fmt.Sprintf("https://cwe.mitre.org/data/published/cwe_v%s.pdf/", version), @@ -158,19 +157,19 @@ func buildCWETaxonomy(version string, taxa []*ReportingDescriptor) *ToolComponen ShortDescription: &MultiformatMessageString{ Text: "The MITRE Common Weakness Enumeration", }, - GUID: uuid3(cweAcronym), + GUID: uuid3(cwe.Acronym), IsComprehensive: true, MinimumRequiredLocalizedDataSemanticVersion: version, Taxa: taxa, } } -func parseSarifTaxon(weakness cwe.Weakness) *ReportingDescriptor { +func parseSarifTaxon(weakness *cwe.Weakness) *ReportingDescriptor { return &ReportingDescriptor{ ID: weakness.ID, Name: weakness.Name, - GUID: uuid3(cweAcronym + weakness.ID), - HelpURI: weakness.URL(), + GUID: uuid3(weakness.SprintID()), + HelpURI: weakness.SprintURL(), ShortDescription: &MultiformatMessageString{ Text: weakness.Description, }, @@ -189,7 +188,7 @@ func buildSarifDriver(rules []*ReportingDescriptor) *ToolComponent { Name: "gosec", Version: gosecVersion, SupportedTaxonomies: []*ToolComponentReference{ - {Name: cweAcronym, GUID: uuid3(cweAcronym)}, + {Name: cwe.Acronym, GUID: uuid3(cwe.Acronym)}, }, InformationURI: "https://github.com/securego/gosec/", Rules: rules, diff --git a/report/text/template.go b/report/text/template.go index 33f44d4..3004085 100644 --- a/report/text/template.go +++ b/report/text/template.go @@ -8,7 +8,7 @@ Golang errors in file: [{{ $filePath }}]: {{end}} {{end}} {{ range $index, $issue := .Issues }} -[{{ highlight $issue.FileLocation $issue.Severity }}] - {{ $issue.RuleID }} (CWE-{{ $issue.Cwe.ID }}): {{ $issue.What }} (Confidence: {{ $issue.Confidence}}, Severity: {{ $issue.Severity }}) +[{{ highlight $issue.FileLocation $issue.Severity }}] - {{ $issue.RuleID }} ({{ $issue.Cwe.SprintID }}): {{ $issue.What }} (Confidence: {{ $issue.Confidence}}, Severity: {{ $issue.Severity }}) {{ printCode $issue }} {{ end }}