internal/sarif: compute relative paths for findings
And also make sure the paths are not added in binary mode.
Updates golang/go#61347
Change-Id: If48fe57215cdecb01b8b687fbe042aae584f1d6d
Reviewed-on: https://go-review.googlesource.com/c/vuln/+/558016
Reviewed-by: Maceo Thompson <maceothompson@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Run-TryBot: Zvonimir Pavlinovic <zpavlinovic@google.com>
diff --git a/cmd/govulncheck/testdata/common/testfiles/source-call/source_call_sarif.ct b/cmd/govulncheck/testdata/common/testfiles/source-call/source_call_sarif.ct
index 30598fc..548d608 100644
--- a/cmd/govulncheck/testdata/common/testfiles/source-call/source_call_sarif.ct
+++ b/cmd/govulncheck/testdata/common/testfiles/source-call/source_call_sarif.ct
@@ -113,7 +113,10 @@
"locations": [
{
"physicalLocation": {
- "artifactLocation": {},
+ "artifactLocation": {
+ "uri": "go.mod",
+ "uriBaseId": "%SRCROOT%"
+ },
"region": {
"startLine": 1
}
@@ -131,7 +134,10 @@
"locations": [
{
"physicalLocation": {
- "artifactLocation": {},
+ "artifactLocation": {
+ "uri": "go.mod",
+ "uriBaseId": "%SRCROOT%"
+ },
"region": {
"startLine": 1
}
@@ -148,7 +154,10 @@
"module": "golang.org/vuln",
"location": {
"physicalLocation": {
- "artifactLocation": {},
+ "artifactLocation": {
+ "uri": "vuln.go",
+ "uriBaseId": "%SRCROOT%"
+ },
"region": {
"startLine": 14,
"startColumn": 20
@@ -163,7 +172,10 @@
"module": "github.com/tidwall/gjson",
"location": {
"physicalLocation": {
- "artifactLocation": {},
+ "artifactLocation": {
+ "uri": "gjson.go",
+ "uriBaseId": "%GOMODCACHE%"
+ },
"region": {
"startLine": 297,
"startColumn": 12
@@ -178,7 +190,10 @@
"module": "github.com/tidwall/gjson",
"location": {
"physicalLocation": {
- "artifactLocation": {},
+ "artifactLocation": {
+ "uri": "gjson.go",
+ "uriBaseId": "%GOMODCACHE%"
+ },
"region": {
"startLine": 1881,
"startColumn": 36
@@ -193,7 +208,10 @@
"module": "github.com/tidwall/gjson",
"location": {
"physicalLocation": {
- "artifactLocation": {},
+ "artifactLocation": {
+ "uri": "gjson.go",
+ "uriBaseId": "%GOMODCACHE%"
+ },
"region": {
"startLine": 220,
"startColumn": 17
@@ -222,7 +240,10 @@
"module": "golang.org/vuln",
"location": {
"physicalLocation": {
- "artifactLocation": {},
+ "artifactLocation": {
+ "uri": "vuln.go",
+ "uriBaseId": "%SRCROOT%"
+ },
"region": {
"startLine": 14,
"startColumn": 20
@@ -237,7 +258,10 @@
"module": "github.com/tidwall/gjson",
"location": {
"physicalLocation": {
- "artifactLocation": {},
+ "artifactLocation": {
+ "uri": "gjson.go",
+ "uriBaseId": "%GOMODCACHE%"
+ },
"region": {
"startLine": 297,
"startColumn": 12
@@ -252,7 +276,10 @@
"module": "github.com/tidwall/gjson",
"location": {
"physicalLocation": {
- "artifactLocation": {},
+ "artifactLocation": {
+ "uri": "gjson.go",
+ "uriBaseId": "%GOMODCACHE%"
+ },
"region": {
"startLine": 1881,
"startColumn": 36
@@ -267,7 +294,10 @@
"module": "github.com/tidwall/gjson",
"location": {
"physicalLocation": {
- "artifactLocation": {},
+ "artifactLocation": {
+ "uri": "gjson.go",
+ "uriBaseId": "%GOMODCACHE%"
+ },
"region": {
"startLine": 2587,
"startColumn": 21
@@ -282,7 +312,10 @@
"module": "github.com/tidwall/gjson",
"location": {
"physicalLocation": {
- "artifactLocation": {},
+ "artifactLocation": {
+ "uri": "gjson.go",
+ "uriBaseId": "%GOMODCACHE%"
+ },
"region": {
"startLine": 2631,
"startColumn": 21
@@ -297,7 +330,10 @@
"module": "github.com/tidwall/gjson",
"location": {
"physicalLocation": {
- "artifactLocation": {},
+ "artifactLocation": {
+ "uri": "gjson.go",
+ "uriBaseId": "%GOMODCACHE%"
+ },
"region": {
"startLine": 220,
"startColumn": 17
@@ -321,7 +357,10 @@
"locations": [
{
"physicalLocation": {
- "artifactLocation": {},
+ "artifactLocation": {
+ "uri": "go.mod",
+ "uriBaseId": "%SRCROOT%"
+ },
"region": {
"startLine": 1
}
@@ -338,7 +377,10 @@
"module": "golang.org/vuln",
"location": {
"physicalLocation": {
- "artifactLocation": {},
+ "artifactLocation": {
+ "uri": "vuln.go",
+ "uriBaseId": "%SRCROOT%"
+ },
"region": {
"startLine": 13,
"startColumn": 16
@@ -353,7 +395,10 @@
"module": "golang.org/x/text",
"location": {
"physicalLocation": {
- "artifactLocation": {},
+ "artifactLocation": {
+ "uri": "language/parse.go",
+ "uriBaseId": "%GOMODCACHE%"
+ },
"region": {
"startLine": 228,
"startColumn": 6
@@ -382,7 +427,10 @@
"module": "golang.org/vuln",
"location": {
"physicalLocation": {
- "artifactLocation": {},
+ "artifactLocation": {
+ "uri": "vuln.go",
+ "uriBaseId": "%SRCROOT%"
+ },
"region": {
"startLine": 13,
"startColumn": 16
@@ -397,7 +445,10 @@
"module": "golang.org/x/text",
"location": {
"physicalLocation": {
- "artifactLocation": {},
+ "artifactLocation": {
+ "uri": "language/parse.go",
+ "uriBaseId": "%GOMODCACHE%"
+ },
"region": {
"startLine": 228,
"startColumn": 6
@@ -421,7 +472,10 @@
"locations": [
{
"physicalLocation": {
- "artifactLocation": {},
+ "artifactLocation": {
+ "uri": "go.mod",
+ "uriBaseId": "%SRCROOT%"
+ },
"region": {
"startLine": 1
}
@@ -438,7 +492,10 @@
"module": "golang.org/vuln",
"location": {
"physicalLocation": {
- "artifactLocation": {},
+ "artifactLocation": {
+ "uri": "vuln.go",
+ "uriBaseId": "%SRCROOT%"
+ },
"region": {
"startLine": 14,
"startColumn": 20
@@ -453,7 +510,10 @@
"module": "github.com/tidwall/gjson",
"location": {
"physicalLocation": {
- "artifactLocation": {},
+ "artifactLocation": {
+ "uri": "gjson.go",
+ "uriBaseId": "%GOMODCACHE%"
+ },
"region": {
"startLine": 296,
"startColumn": 17
@@ -482,7 +542,10 @@
"module": "golang.org/vuln",
"location": {
"physicalLocation": {
- "artifactLocation": {},
+ "artifactLocation": {
+ "uri": "vuln.go",
+ "uriBaseId": "%SRCROOT%"
+ },
"region": {
"startLine": 14,
"startColumn": 20
@@ -497,7 +560,10 @@
"module": "github.com/tidwall/gjson",
"location": {
"physicalLocation": {
- "artifactLocation": {},
+ "artifactLocation": {
+ "uri": "gjson.go",
+ "uriBaseId": "%GOMODCACHE%"
+ },
"region": {
"startLine": 296,
"startColumn": 17
diff --git a/cmd/govulncheck/testdata/common/testfiles/source-module/source_module_sarif.ct b/cmd/govulncheck/testdata/common/testfiles/source-module/source_module_sarif.ct
index e1fe76b..b3b5512 100644
--- a/cmd/govulncheck/testdata/common/testfiles/source-module/source_module_sarif.ct
+++ b/cmd/govulncheck/testdata/common/testfiles/source-module/source_module_sarif.ct
@@ -113,7 +113,10 @@
"locations": [
{
"physicalLocation": {
- "artifactLocation": {},
+ "artifactLocation": {
+ "uri": "go.mod",
+ "uriBaseId": "%SRCROOT%"
+ },
"region": {
"startLine": 1
}
@@ -131,7 +134,10 @@
"locations": [
{
"physicalLocation": {
- "artifactLocation": {},
+ "artifactLocation": {
+ "uri": "go.mod",
+ "uriBaseId": "%SRCROOT%"
+ },
"region": {
"startLine": 1
}
@@ -149,7 +155,10 @@
"locations": [
{
"physicalLocation": {
- "artifactLocation": {},
+ "artifactLocation": {
+ "uri": "go.mod",
+ "uriBaseId": "%SRCROOT%"
+ },
"region": {
"startLine": 1
}
@@ -167,7 +176,10 @@
"locations": [
{
"physicalLocation": {
- "artifactLocation": {},
+ "artifactLocation": {
+ "uri": "go.mod",
+ "uriBaseId": "%SRCROOT%"
+ },
"region": {
"startLine": 1
}
diff --git a/cmd/govulncheck/testdata/common/testfiles/source-package/source_package_sarif.ct b/cmd/govulncheck/testdata/common/testfiles/source-package/source_package_sarif.ct
index d82a995..dd17a42 100644
--- a/cmd/govulncheck/testdata/common/testfiles/source-package/source_package_sarif.ct
+++ b/cmd/govulncheck/testdata/common/testfiles/source-package/source_package_sarif.ct
@@ -113,7 +113,10 @@
"locations": [
{
"physicalLocation": {
- "artifactLocation": {},
+ "artifactLocation": {
+ "uri": "go.mod",
+ "uriBaseId": "%SRCROOT%"
+ },
"region": {
"startLine": 1
}
@@ -131,7 +134,10 @@
"locations": [
{
"physicalLocation": {
- "artifactLocation": {},
+ "artifactLocation": {
+ "uri": "go.mod",
+ "uriBaseId": "%SRCROOT%"
+ },
"region": {
"startLine": 1
}
@@ -149,7 +155,10 @@
"locations": [
{
"physicalLocation": {
- "artifactLocation": {},
+ "artifactLocation": {
+ "uri": "go.mod",
+ "uriBaseId": "%SRCROOT%"
+ },
"region": {
"startLine": 1
}
@@ -167,7 +176,10 @@
"locations": [
{
"physicalLocation": {
- "artifactLocation": {},
+ "artifactLocation": {
+ "uri": "go.mod",
+ "uriBaseId": "%SRCROOT%"
+ },
"region": {
"startLine": 1
}
diff --git a/internal/sarif/handler.go b/internal/sarif/handler.go
index 7bf9326..f9aa2ee 100644
--- a/internal/sarif/handler.go
+++ b/internal/sarif/handler.go
@@ -10,6 +10,7 @@
"io"
"sort"
+ "golang.org/x/vuln/internal"
"golang.org/x/vuln/internal/govulncheck"
"golang.org/x/vuln/internal/osv"
"golang.org/x/vuln/internal/traces"
@@ -165,7 +166,10 @@
// Attach result to the go.mod file for source analysis.
// But there is no such place for binaries.
locs = []Location{{PhysicalLocation: PhysicalLocation{
- // TODO: add artifact location for go.mod
+ ArtifactLocation: ArtifactLocation{
+ URI: "go.mod",
+ URIBaseID: SrcRootID,
+ },
Region: Region{StartLine: 1}, // for now, point to the first line
}}}
}
@@ -174,8 +178,8 @@
RuleID: fs[0].OSV,
Level: level(fs[0], h.cfg),
Message: Description{Text: resultMessage(fs, h.cfg)},
- Stacks: stacks(fs),
- CodeFlows: codeFlows(fs),
+ Stacks: stacks(h, fs),
+ CodeFlows: codeFlows(h, fs),
Locations: locs,
}
results = append(results, res)
@@ -250,14 +254,14 @@
}
}
-func stacks(fs []*govulncheck.Finding) []Stack {
+func stacks(h *handler, fs []*govulncheck.Finding) []Stack {
if fs[0].Trace[0].Function == "" { // not call level findings
return nil
}
var stacks []Stack
for _, f := range fs {
- stacks = append(stacks, stack(f))
+ stacks = append(stacks, stack(h, f))
}
// Sort stacks for deterministic output. We sort by message
// which is effectively sorting by full symbol name. The
@@ -267,8 +271,9 @@
}
// stack transforms call stack in f to a sarif stack.
-func stack(f *govulncheck.Finding) Stack {
+func stack(h *handler, f *govulncheck.Finding) Stack {
trace := f.Trace
+ top := trace[len(trace)-1] // belongs to top level module
var frames []Frame
for i := len(trace) - 1; i >= 0; i-- { // vulnerable symbol is at the top frame
@@ -277,19 +282,24 @@
if frame.Position != nil {
pos = *frame.Position
}
- frames = append(frames, Frame{
- Module: frame.Module,
- Location: Location{
- Message: Description{Text: symbol(frame)}, // show the (full) symbol name
- PhysicalLocation: PhysicalLocation{
- // TODO: add artifact location
- Region: Region{
- StartLine: pos.Line,
- StartColumn: pos.Column,
- },
+
+ sf := Frame{
+ Module: frame.Module,
+ Location: Location{Message: Description{Text: symbol(frame)}}, // show the (full) symbol name
+ }
+ if h.cfg.ScanMode != govulncheck.ScanModeBinary {
+ sf.Location.PhysicalLocation = PhysicalLocation{
+ ArtifactLocation: ArtifactLocation{
+ URI: pos.Filename,
+ URIBaseID: uriID(top.Module, frame.Module),
},
- },
- })
+ Region: Region{
+ StartLine: pos.Line,
+ StartColumn: pos.Column,
+ },
+ }
+ }
+ frames = append(frames, sf)
}
return Stack{
@@ -298,7 +308,7 @@
}
}
-func codeFlows(fs []*govulncheck.Finding) []CodeFlow {
+func codeFlows(h *handler, fs []*govulncheck.Finding) []CodeFlow {
if fs[0].Trace[0].Function == "" { // not call level findings
return nil
}
@@ -316,7 +326,7 @@
var codeFlows []CodeFlow
for fr, fs := range m {
- tfs := threadFlows(fs)
+ tfs := threadFlows(h, fs)
codeFlows = append(codeFlows, CodeFlow{
ThreadFlows: tfs,
// TODO: should we instead show the message from govulncheck text output?
@@ -330,10 +340,12 @@
return codeFlows
}
-func threadFlows(fs []*govulncheck.Finding) []ThreadFlow {
+func threadFlows(h *handler, fs []*govulncheck.Finding) []ThreadFlow {
var tfs []ThreadFlow
for _, f := range fs {
trace := traces.Compact(f)
+ top := trace[len(trace)-1] // belongs to top level module
+
var tf []ThreadFlowLocation
for i := len(trace) - 1; i >= 0; i-- { // vulnerable symbol is at the top frame
// TODO: should we, similar to govulncheck text output, only
@@ -343,20 +355,36 @@
if frame.Position != nil {
pos = *frame.Position
}
- tf = append(tf, ThreadFlowLocation{
- Module: frame.Module,
- Location: Location{
- Message: Description{Text: symbol(frame)}, // show the (full) symbol name
- PhysicalLocation: PhysicalLocation{
- // TODO: add artifact location
- Region: Region{
- StartLine: pos.Line,
- StartColumn: pos.Column,
- },
+
+ tfl := ThreadFlowLocation{
+ Module: frame.Module,
+ Location: Location{Message: Description{Text: symbol(frame)}}, // show the (full) symbol name
+ }
+ if h.cfg.ScanMode != govulncheck.ScanModeBinary {
+ tfl.Location.PhysicalLocation = PhysicalLocation{
+ ArtifactLocation: ArtifactLocation{
+ URI: pos.Filename,
+ URIBaseID: uriID(top.Module, frame.Module),
},
- }})
+ Region: Region{
+ StartLine: pos.Line,
+ StartColumn: pos.Column,
+ },
+ }
+ }
+ tf = append(tf, tfl)
}
tfs = append(tfs, ThreadFlow{Locations: tf})
}
return tfs
}
+
+func uriID(top, module string) string {
+ if top == module {
+ return SrcRootID
+ }
+ if module == internal.GoStdModulePath {
+ return GoRootID
+ }
+ return GoModCacheID
+}
diff --git a/internal/sarif/sarif.go b/internal/sarif/sarif.go
index fd00e2f..53d2ba3 100644
--- a/internal/sarif/sarif.go
+++ b/internal/sarif/sarif.go
@@ -5,8 +5,13 @@
// Package sarif defines Static Analysis Results Interchange Format
// (SARIF) types supported by govulncheck.
//
-// See https://www.oasis-open.org/committees/tc_home.php?wg_abbrev=sarif
-// for more information on the SARIF format.
+// The implementation covers the subset of the specification available
+// at https://www.oasis-open.org/committees/tc_home.php?wg_abbrev=sarif.
+//
+// If govulncheck is used in source mode, the locations will include a
+// physical location implemented as a path relative to either the source
+// module (%SRCROOT%), Go root (%GOROOT%), or Go module cache (%GOMODCACHE%)
+// URI base id.
package sarif
import "golang.org/x/vuln/internal/govulncheck"
@@ -151,6 +156,12 @@
Region Region `json:"region,omitempty"`
}
+const (
+ SrcRootID = "%SRCROOT%"
+ GoRootID = "%GOROOT%"
+ GoModCacheID = "%GOMODCACHE%"
+)
+
// ArtifactLocation is a path to an offending file.
type ArtifactLocation struct {
// URI is a path to the artifact. If URIBaseID is empty, then
