Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: improve sarif descriptive text and fingerprint #1720

Merged
merged 3 commits into from
Feb 22, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
70 changes: 54 additions & 16 deletions grype/presenter/sarif/presenter.go
Original file line number Diff line number Diff line change
@@ -1,9 +1,12 @@
package sarif

import (
"crypto/sha256"
"fmt"
"hash"
"io"
"path/filepath"
"regexp"
"strings"

"github.com/owenrumney/go-sarif/sarif"
Expand Down Expand Up @@ -215,18 +218,19 @@ func (pres *Presenter) locations(m match.Match) []*sarif.Location {
img := metadata.UserInput
locations := m.Package.Locations.ToSlice()
for _, l := range locations {
trimmedPath := strings.TrimPrefix(pres.locationPath(l), "/")
trimmedPath := strings.TrimLeft(pres.locationPath(l), "/")
logicalLocations = append(logicalLocations, &sarif.LogicalLocation{
FullyQualifiedName: sp(fmt.Sprintf("%s@%s:/%s", img, l.FileSystemID, trimmedPath)),
Name: sp(l.RealPath),
})
}

// this is a hack to get results to show up in GitHub, as it requires relative paths for the location
// but we really won't have any information about what Dockerfile on the filesystem was used to build the image
// TODO we could add configuration to specify the prefix, a user might want to specify an image name and architecture
// in the case of multiple vuln scans, for example
physicalLocation = fmt.Sprintf("image/%s", physicalLocation)
// GitHub requires paths for the location, but we really don't have any information about what
// file(s) these originated from in the repository. e.g. which Dockerfile was used to build an image,
// so we just use a short path-compatible image name here, not the entire user input as it may include
// sha and/or tags which are likely to change between runs and aren't really necessary for a general
// path to find file where the package originated
physicalLocation = fmt.Sprintf("%s %s", imageShortPathName(pres.src), physicalLocation)
case source.FileSourceMetadata:
locations := m.Package.Locations.ToSlice()
for _, l := range locations {
Expand Down Expand Up @@ -387,11 +391,12 @@ func (pres *Presenter) sarifResults() []*sarif.Result {
out = append(out, &sarif.Result{
RuleID: sp(pres.ruleID(m)),
Message: pres.resultMessage(m),
// According to the SARIF spec, I believe we should be using AnalysisTarget.URI to indicate a logical
// According to the SARIF spec, it may be correct to use AnalysisTarget.URI to indicate a logical
// file such as a "Dockerfile" but GitHub does not work well with this
// FIXME github "requires" partialFingerprints
// PartialFingerprints: ???
Locations: pres.locations(m),
// GitHub requires partialFingerprints to upload to the API; these are automatically filled in
// when using the CodeQL upload action. See: https://docs.github.com/en/code-security/code-scanning/integrating-with-code-scanning/sarif-support-for-code-scanning#providing-data-to-track-code-scanning-alerts-across-runs
PartialFingerprints: pres.partialFingerprints(m),
Locations: pres.locations(m),
})
}
return out
Expand All @@ -409,19 +414,40 @@ func sp(sarif string) *string {

func (pres *Presenter) resultMessage(m match.Match) sarif.Message {
path := pres.packagePath(m.Package)
message := fmt.Sprintf("The path %s reports %s at version %s ", path, m.Package.Name, m.Package.Version)

if _, ok := pres.src.Metadata.(source.DirectorySourceMetadata); ok {
message = fmt.Sprintf("%s which would result in a vulnerable (%s) package installed", message, m.Package.Type)
} else {
message = fmt.Sprintf("%s which is a vulnerable (%s) package installed in the container", message, m.Package.Type)
src := pres.inputPath()
switch meta := pres.src.Metadata.(type) {
case source.StereoscopeImageSourceMetadata:
src = fmt.Sprintf("in image %s at: %s", meta.UserInput, path)
case source.FileSourceMetadata, source.DirectorySourceMetadata:
src = fmt.Sprintf("at: %s", path)
}
message := fmt.Sprintf("A %s vulnerability in %s package: %s, version %s was found %s",
pres.severityText(m), m.Package.Type, m.Package.Name, m.Package.Version, src)

return sarif.Message{
Text: &message,
}
}

func (pres *Presenter) partialFingerprints(m match.Match) map[string]any {
p := m.Package
hasher := sha256.New()
if meta, ok := pres.src.Metadata.(source.StereoscopeImageSourceMetadata); ok {
hashWrite(hasher, pres.src.Name, meta.Architecture, meta.OS)
}
hashWrite(hasher, string(p.Type), p.Name, p.Version, pres.packagePath(p))
return map[string]any{
// this is meant to include <hash>:<line>, but there isn't line information here, so just include :1
"primaryLocationLineHash": fmt.Sprintf("%x:1", hasher.Sum([]byte{})),
}
}

func hashWrite(hasher hash.Hash, values ...string) {
for _, value := range values {
_, _ = hasher.Write([]byte(value))
}
}

func ruleName(m match.Match) string {
if len(m.Details) > 0 {
d := m.Details[0]
Expand All @@ -436,3 +462,15 @@ func ruleName(m match.Match) string {
}
return m.Vulnerability.ID
}

var nonPathChars = regexp.MustCompile("[^a-zA-Z0-9-_.]")

// imageShortPathName returns path-compatible text describing the image. if the image name is the form
// some/path/to/image, it will return the image portion of the name.
func imageShortPathName(s *source.Description) string {
imageName := s.Name
parts := strings.Split(imageName, "/")
imageName = parts[len(parts)-1]
imageName = nonPathChars.ReplaceAllString(imageName, "")
return imageName
}
39 changes: 37 additions & 2 deletions grype/presenter/sarif/presenter_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -227,8 +227,8 @@ func TestToSarifReport(t *testing.T) {
name: "image",
scheme: internal.ImageSource,
locations: map[string]string{
"CVE-1999-0001-package-1": "image/somefile-1.txt",
"CVE-1999-0002-package-2": "image/somefile-2.txt",
"CVE-1999-0001-package-1": "user-input somefile-1.txt",
"CVE-1999-0002-package-2": "user-input somefile-2.txt",
},
},
}
Expand Down Expand Up @@ -418,3 +418,38 @@ func Test_cvssScore(t *testing.T) {
})
}
}

func Test_imageShortPathName(t *testing.T) {
tests := []struct {
name string
in string
expected string
}{
{
name: "valid single name",
in: "simple.-_name",
expected: "simple.-_name",
},
{
name: "valid name in org",
in: "some-org/some-image",
expected: "some-image",
},
{
name: "name and org with many invalid chars",
in: "some/*^&$#%$#@*(}{<><./,valid-()(#)@!(~@#$#%^&**[]{-chars",
expected: "valid--chars",
},
}

for _, test := range tests {
t.Run(test.name, func(t *testing.T) {
got := imageShortPathName(&source.Description{
Name: test.in,
Metadata: nil,
})

assert.Equal(t, test.expected, got)
})
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@
{
"ruleId": "CVE-1999-0001-package-1",
"message": {
"text": "The path /some/path/somefile-1.txt reports package-1 at version 1.1.1 which would result in a vulnerable (rpm) package installed"
"text": "A low vulnerability in rpm package: package-1, version 1.1.1 was found at: /some/path/somefile-1.txt"
},
"locations": [
{
Expand All @@ -68,12 +68,15 @@
}
}
}
]
],
"partialFingerprints": {
"primaryLocationLineHash": "0eefd3962fe456b80e5ddad4ec777c7f75b3c0586db887eff1c98f376fff60ba:1"
}
},
{
"ruleId": "CVE-1999-0002-package-2",
"message": {
"text": "The path /some/path/somefile-2.txt reports package-2 at version 2.2.2 which would result in a vulnerable (deb) package installed"
"text": "A critical vulnerability in deb package: package-2, version 2.2.2 was found at: /some/path/somefile-2.txt"
},
"locations": [
{
Expand All @@ -89,7 +92,10 @@
}
}
}
]
],
"partialFingerprints": {
"primaryLocationLineHash": "0d4ef10dce50e71641e9314195020cea18febe4c6a4a8145a485154383d4fe0b:1"
}
}
]
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -52,13 +52,13 @@
{
"ruleId": "CVE-1999-0001-package-1",
"message": {
"text": "The path somefile-1.txt reports package-1 at version 1.1.1 which is a vulnerable (rpm) package installed in the container"
"text": "A low vulnerability in rpm package: package-1, version 1.1.1 was found in image user-input at: somefile-1.txt"
},
"locations": [
{
"physicalLocation": {
"artifactLocation": {
"uri": "image/somefile-1.txt"
"uri": "user-input somefile-1.txt"
},
"region": {
"startLine": 1,
Expand All @@ -74,18 +74,21 @@
}
]
}
]
],
"partialFingerprints": {
"primaryLocationLineHash": "efe125c0a2b4bdafe476b69ba51a49734780c62b93803950319056acebe4323f:1"
}
},
{
"ruleId": "CVE-1999-0002-package-2",
"message": {
"text": "The path somefile-2.txt reports package-2 at version 2.2.2 which is a vulnerable (deb) package installed in the container"
"text": "A critical vulnerability in deb package: package-2, version 2.2.2 was found in image user-input at: somefile-2.txt"
},
"locations": [
{
"physicalLocation": {
"artifactLocation": {
"uri": "image/somefile-2.txt"
"uri": "user-input somefile-2.txt"
},
"region": {
"startLine": 1,
Expand All @@ -101,7 +104,10 @@
}
]
}
]
],
"partialFingerprints": {
"primaryLocationLineHash": "bafe9890c7cda00bf4d1b1a57d1d20b08e27162e718235a3d38a9a8d2f449ed1:1"
}
}
]
}
Expand Down