Skip to content

Commit

Permalink
fix(report): Fix invalid URI in SARIF report (#7645)
Browse files Browse the repository at this point in the history 
Co-authored-by: DmitriyLewen <[email protected]>
  • Loading branch information
@pbaumard @DmitriyLewen
pbaumard and DmitriyLewen authored Oct 7, 2024
1 parent ab3a3b2 commit 015bb88
Show file tree
Hide file tree
Showing 2 changed files with 41 additions and 30 deletions.
37 changes: 24 additions & 13 deletions pkg/report/sarif.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import (
"fmt"
"html"
"io"
"net/url"
"path/filepath"
"regexp"
"strings"
Expand All @@ -15,6 +16,7 @@ import (

"github.com/aquasecurity/trivy/pkg/fanal/artifact"
ftypes "github.com/aquasecurity/trivy/pkg/fanal/types"
"github.com/aquasecurity/trivy/pkg/log"
"github.com/aquasecurity/trivy/pkg/types"
)

Expand Down Expand Up @@ -61,9 +63,9 @@ type sarifData struct {
helpMarkdown string
resourceClass types.ResultClass
severity string
url string
url *url.URL
resultIndex int
artifactLocation string
artifactLocation *url.URL
locationMessage string
message string
cvssScore string
Expand Down Expand Up @@ -97,8 +99,8 @@ func (sw *SarifWriter) addSarifRule(data *sarifData) {
"precision": "very-high",
"security-severity": data.cvssScore,
})
if data.url != "" {
r.WithHelpURI(data.url)
if data.url != nil && data.url.String() != "" {
r.WithHelpURI(data.url.String())
}
}

Expand All @@ -109,7 +111,7 @@ func (sw *SarifWriter) addSarifResult(data *sarifData) {
WithRuleIndex(data.resultIndex).
WithMessage(sarif.NewTextMessage(data.message)).
WithLevel(toSarifErrorLevel(data.severity)).
WithLocations(toSarifLocations(data.locations, data.artifactLocation, data.locationMessage))
WithLocations(toSarifLocations(data.locations, data.artifactLocation.String(), data.locationMessage))
sw.run.AddResult(result)
}

Expand Down Expand Up @@ -163,9 +165,9 @@ func (sw *SarifWriter) Write(ctx context.Context, report types.Report) error {
vulnerabilityId: vuln.VulnerabilityID,
severity: vuln.Severity,
cvssScore: getCVSSScore(vuln),
url: vuln.PrimaryURL,
url: toUri(vuln.PrimaryURL),
resourceClass: res.Class,
artifactLocation: path,
artifactLocation: toUri(path),
locationMessage: fmt.Sprintf("%v: %v@%v", path, vuln.PkgName, vuln.InstalledVersion),
locations: sw.getLocations(vuln.PkgName, vuln.InstalledVersion, path, res.Packages),
resultIndex: getRuleIndex(vuln.VulnerabilityID, ruleIndexes),
Expand All @@ -186,9 +188,9 @@ func (sw *SarifWriter) Write(ctx context.Context, report types.Report) error {
vulnerabilityId: misconf.ID,
severity: misconf.Severity,
cvssScore: severityToScore(misconf.Severity),
url: misconf.PrimaryURL,
url: toUri(misconf.PrimaryURL),
resourceClass: res.Class,
artifactLocation: locationURI,
artifactLocation: toUri(locationURI),
locationMessage: locationURI,
locations: []location{
{
Expand All @@ -213,9 +215,9 @@ func (sw *SarifWriter) Write(ctx context.Context, report types.Report) error {
vulnerabilityId: secret.RuleID,
severity: secret.Severity,
cvssScore: severityToScore(secret.Severity),
url: builtinRulesUrl,
url: toUri(builtinRulesUrl),
resourceClass: res.Class,
artifactLocation: target,
artifactLocation: toUri(target),
locationMessage: target,
locations: []location{
{
Expand All @@ -242,9 +244,9 @@ func (sw *SarifWriter) Write(ctx context.Context, report types.Report) error {
vulnerabilityId: id,
severity: license.Severity,
cvssScore: severityToScore(license.Severity),
url: license.Link,
url: toUri(license.Link),
resourceClass: res.Class,
artifactLocation: target,
artifactLocation: toUri(target),
resultIndex: getRuleIndex(id, ruleIndexes),
shortDescription: desc,
fullDescription: desc,
Expand Down Expand Up @@ -348,6 +350,15 @@ func clearURI(s string) string {
return strings.ReplaceAll(strings.ReplaceAll(s, "\\", "/"), "git::https:/", "")
}

func toUri(str string) *url.URL {
uri, err := url.Parse(str)
if err != nil {
logger := log.WithPrefix("sarif")
logger.Error("Unable to parse URI", log.String("URI", str), log.Err(err))
}
return uri
}

func (sw *SarifWriter) getLocations(name, version, path string, pkgs []ftypes.Package) []location {
id := fmt.Sprintf("%s@%s@%s", path, name, version)
locs, ok := sw.locationCache[id]
Expand Down
34 changes: 17 additions & 17 deletions pkg/report/sarif_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ func TestReportWriter_Sarif(t *testing.T) {
},
Results: types.Results{
{
Target: "library/test",
Target: "library/test 1",
Class: types.ClassOSPkg,
Packages: []ftypes.Package{
{
Expand Down Expand Up @@ -137,10 +137,10 @@ func TestReportWriter_Sarif(t *testing.T) {
Message: sarif.Message{Text: lo.ToPtr("Package: foo\nInstalled Version: 1.2.3\nVulnerability CVE-2020-0001\nSeverity: HIGH\nFixed Version: 3.4.5\nLink: [CVE-2020-0001](https://avd.aquasec.com/nvd/cve-2020-0001)")},
Locations: []*sarif.Location{
{
Message: &sarif.Message{Text: lo.ToPtr("library/test: [email protected]")},
Message: &sarif.Message{Text: lo.ToPtr("library/test 1: [email protected]")},
PhysicalLocation: &sarif.PhysicalLocation{
ArtifactLocation: &sarif.ArtifactLocation{
URI: lo.ToPtr("library/test"),
URI: lo.ToPtr("library/test%201"),
URIBaseId: lo.ToPtr("ROOTPATH"),
},
Region: &sarif.Region{
Expand All @@ -152,10 +152,10 @@ func TestReportWriter_Sarif(t *testing.T) {
},
},
{
Message: &sarif.Message{Text: lo.ToPtr("library/test: [email protected]")},
Message: &sarif.Message{Text: lo.ToPtr("library/test 1: [email protected]")},
PhysicalLocation: &sarif.PhysicalLocation{
ArtifactLocation: &sarif.ArtifactLocation{
URI: lo.ToPtr("library/test"),
URI: lo.ToPtr("library/test%201"),
URIBaseId: lo.ToPtr("ROOTPATH"),
},
Region: &sarif.Region{
Expand Down Expand Up @@ -192,7 +192,7 @@ func TestReportWriter_Sarif(t *testing.T) {
input: types.Report{
Results: types.Results{
{
Target: "library/test",
Target: "library/test 1",
Class: types.ClassConfig,
Misconfigurations: []types.DetectedMisconfiguration{
{
Expand Down Expand Up @@ -283,13 +283,13 @@ func TestReportWriter_Sarif(t *testing.T) {
RuleID: lo.ToPtr("KSV001"),
RuleIndex: lo.ToPtr[uint](0),
Level: lo.ToPtr("error"),
Message: sarif.Message{Text: lo.ToPtr("Artifact: library/test\nType: \nVulnerability KSV001\nSeverity: HIGH\nMessage: Message\nLink: [KSV001](https://avd.aquasec.com/appshield/ksv001)")},
Message: sarif.Message{Text: lo.ToPtr("Artifact: library/test 1\nType: \nVulnerability KSV001\nSeverity: HIGH\nMessage: Message\nLink: [KSV001](https://avd.aquasec.com/appshield/ksv001)")},
Locations: []*sarif.Location{
{
Message: &sarif.Message{Text: lo.ToPtr("library/test")},
Message: &sarif.Message{Text: lo.ToPtr("library/test 1")},
PhysicalLocation: &sarif.PhysicalLocation{
ArtifactLocation: &sarif.ArtifactLocation{
URI: lo.ToPtr("library/test"),
URI: lo.ToPtr("library/test%201"),
URIBaseId: lo.ToPtr("ROOTPATH"),
},
Region: &sarif.Region{
Expand All @@ -306,13 +306,13 @@ func TestReportWriter_Sarif(t *testing.T) {
RuleID: lo.ToPtr("KSV002"),
RuleIndex: lo.ToPtr[uint](1),
Level: lo.ToPtr("error"),
Message: sarif.Message{Text: lo.ToPtr("Artifact: library/test\nType: \nVulnerability KSV002\nSeverity: CRITICAL\nMessage: Message\nLink: [KSV002](https://avd.aquasec.com/appshield/ksv002)")},
Message: sarif.Message{Text: lo.ToPtr("Artifact: library/test 1\nType: \nVulnerability KSV002\nSeverity: CRITICAL\nMessage: Message\nLink: [KSV002](https://avd.aquasec.com/appshield/ksv002)")},
Locations: []*sarif.Location{
{
Message: &sarif.Message{Text: lo.ToPtr("library/test")},
Message: &sarif.Message{Text: lo.ToPtr("library/test 1")},
PhysicalLocation: &sarif.PhysicalLocation{
ArtifactLocation: &sarif.ArtifactLocation{
URI: lo.ToPtr("library/test"),
URI: lo.ToPtr("library/test%201"),
URIBaseId: lo.ToPtr("ROOTPATH"),
},
Region: &sarif.Region{
Expand Down Expand Up @@ -341,7 +341,7 @@ func TestReportWriter_Sarif(t *testing.T) {
input: types.Report{
Results: types.Results{
{
Target: "library/test",
Target: "library/test 1",
Class: types.ClassSecret,
Secrets: []types.DetectedSecret{
{
Expand Down Expand Up @@ -400,13 +400,13 @@ func TestReportWriter_Sarif(t *testing.T) {
RuleID: lo.ToPtr("aws-secret-access-key"),
RuleIndex: lo.ToPtr[uint](0),
Level: lo.ToPtr("error"),
Message: sarif.Message{Text: lo.ToPtr("Artifact: library/test\nType: \nSecret AWS Secret Access Key\nSeverity: CRITICAL\nMatch: 'AWS_secret_KEY'=\"****************************************\"")},
Message: sarif.Message{Text: lo.ToPtr("Artifact: library/test 1\nType: \nSecret AWS Secret Access Key\nSeverity: CRITICAL\nMatch: 'AWS_secret_KEY'=\"****************************************\"")},
Locations: []*sarif.Location{
{
Message: &sarif.Message{Text: lo.ToPtr("library/test")},
Message: &sarif.Message{Text: lo.ToPtr("library/test 1")},
PhysicalLocation: &sarif.PhysicalLocation{
ArtifactLocation: &sarif.ArtifactLocation{
URI: lo.ToPtr("library/test"),
URI: lo.ToPtr("library/test%201"),
URIBaseId: lo.ToPtr("ROOTPATH"),
},
Region: &sarif.Region{
Expand Down Expand Up @@ -495,7 +495,7 @@ func TestReportWriter_Sarif(t *testing.T) {
Message: sarif.NewTextMessage(""),
PhysicalLocation: &sarif.PhysicalLocation{
ArtifactLocation: &sarif.ArtifactLocation{
URI: lo.ToPtr("OS Packages"),
URI: lo.ToPtr("OS%20Packages"),
URIBaseId: lo.ToPtr("ROOTPATH"),
},
Region: &sarif.Region{
Expand Down

0 comments on commit 015bb88

Please sign in to comment.