From bd80eaab95b9dfb8ed83e26d6816f2e0b2452aca Mon Sep 17 00:00:00 2001 From: Maceo Thompson Date: Tue, 9 Jul 2024 13:04:34 -0500 Subject: [PATCH] internal/openvex: populate product subcomponents Populates the "subcomponent" field of a outputted vex statement with the PURL to the vulnerable dependency. updates golang/go#68152 Change-Id: I9e7b9a6686744496b3409ee9d4d0f3d70917db45 Reviewed-on: https://go-review.googlesource.com/c/vuln/+/598956 LUCI-TryBot-Result: Go LUCI Reviewed-by: Zvonimir Pavlinovic --- .../testfiles/binary-call/binary_vex.ct | 30 +++- .../testfiles/source-call/source_call_vex.ct | 30 +++- internal/openvex/handler.go | 97 +++++++++-- internal/openvex/handler_test.go | 155 +++++++++++++++--- internal/openvex/purl.go | 28 ++++ internal/openvex/purl_test.go | 65 ++++++++ internal/openvex/vex.go | 9 +- 7 files changed, 367 insertions(+), 47 deletions(-) create mode 100644 internal/openvex/purl.go create mode 100644 internal/openvex/purl_test.go diff --git a/cmd/govulncheck/testdata/common/testfiles/binary-call/binary_vex.ct b/cmd/govulncheck/testdata/common/testfiles/binary-call/binary_vex.ct index b269509..1dffe5c 100644 --- a/cmd/govulncheck/testdata/common/testfiles/binary-call/binary_vex.ct +++ b/cmd/govulncheck/testdata/common/testfiles/binary-call/binary_vex.ct @@ -3,7 +3,7 @@ $ govulncheck -format openvex -mode binary ${common_vuln_binary} { "@context": "https://openvex.dev/ns/v0.2.0", - "@id": "govulncheck/vex:b1a12e6f591b29f244e67c80a88d10539c220a04f6ca48d3fe7af2faf0189437", + "@id": "govulncheck/vex:261b597336f7aa5eb53a4a196c354c5afed43fe55658ae3816194192b5268881", "author": "Unknown Author", "timestamp": "2024-01-01T00:00:00", "version": 1, @@ -21,7 +21,12 @@ $ govulncheck -format openvex -mode binary ${common_vuln_binary} }, "products": [ { - "@id": "Unknown Product" + "@id": "Unknown Product", + "subcomponents": [ + { + "@id": "pkg:golang/golang.org%2Fx%2Ftext@v0.3.0" + } + ] } ], "status": "not_affected", @@ -40,7 +45,12 @@ $ govulncheck -format openvex -mode binary ${common_vuln_binary} }, "products": [ { - "@id": "Unknown Product" + "@id": "Unknown Product", + "subcomponents": [ + { + "@id": "pkg:golang/github.com%2Ftidwall%2Fgjson@v1.6.5" + } + ] } ], "status": "affected" @@ -57,7 +67,12 @@ $ govulncheck -format openvex -mode binary ${common_vuln_binary} }, "products": [ { - "@id": "Unknown Product" + "@id": "Unknown Product", + "subcomponents": [ + { + "@id": "pkg:golang/golang.org%2Fx%2Ftext@v0.3.0" + } + ] } ], "status": "not_affected", @@ -78,7 +93,12 @@ $ govulncheck -format openvex -mode binary ${common_vuln_binary} }, "products": [ { - "@id": "Unknown Product" + "@id": "Unknown Product", + "subcomponents": [ + { + "@id": "pkg:golang/github.com%2Ftidwall%2Fgjson@v1.6.5" + } + ] } ], "status": "affected" diff --git a/cmd/govulncheck/testdata/common/testfiles/source-call/source_call_vex.ct b/cmd/govulncheck/testdata/common/testfiles/source-call/source_call_vex.ct index cca8ee9..c17485f 100644 --- a/cmd/govulncheck/testdata/common/testfiles/source-call/source_call_vex.ct +++ b/cmd/govulncheck/testdata/common/testfiles/source-call/source_call_vex.ct @@ -3,7 +3,7 @@ $ govulncheck -C ${moddir}/vuln -format openvex ./... { "@context": "https://openvex.dev/ns/v0.2.0", - "@id": "govulncheck/vex:b1a12e6f591b29f244e67c80a88d10539c220a04f6ca48d3fe7af2faf0189437", + "@id": "govulncheck/vex:261b597336f7aa5eb53a4a196c354c5afed43fe55658ae3816194192b5268881", "author": "Unknown Author", "timestamp": "2024-01-01T00:00:00", "version": 1, @@ -21,7 +21,12 @@ $ govulncheck -C ${moddir}/vuln -format openvex ./... }, "products": [ { - "@id": "Unknown Product" + "@id": "Unknown Product", + "subcomponents": [ + { + "@id": "pkg:golang/golang.org%2Fx%2Ftext@v0.3.0" + } + ] } ], "status": "not_affected", @@ -40,7 +45,12 @@ $ govulncheck -C ${moddir}/vuln -format openvex ./... }, "products": [ { - "@id": "Unknown Product" + "@id": "Unknown Product", + "subcomponents": [ + { + "@id": "pkg:golang/github.com%2Ftidwall%2Fgjson@v1.6.5" + } + ] } ], "status": "affected" @@ -57,7 +67,12 @@ $ govulncheck -C ${moddir}/vuln -format openvex ./... }, "products": [ { - "@id": "Unknown Product" + "@id": "Unknown Product", + "subcomponents": [ + { + "@id": "pkg:golang/golang.org%2Fx%2Ftext@v0.3.0" + } + ] } ], "status": "not_affected", @@ -78,7 +93,12 @@ $ govulncheck -C ${moddir}/vuln -format openvex ./... }, "products": [ { - "@id": "Unknown Product" + "@id": "Unknown Product", + "subcomponents": [ + { + "@id": "pkg:golang/github.com%2Ftidwall%2Fgjson@v1.6.5" + } + ] } ], "status": "affected" diff --git a/internal/openvex/handler.go b/internal/openvex/handler.go index b5e43aa..374c4d8 100644 --- a/internal/openvex/handler.go +++ b/internal/openvex/handler.go @@ -26,17 +26,22 @@ const ( ) type handler struct { - w io.Writer - cfg *govulncheck.Config - osvs map[string]*osv.Entry - levels map[string]findingLevel + w io.Writer + cfg *govulncheck.Config + osvs map[string]*osv.Entry + // findings contains same-level findings for an + // OSV at the most precise level of granularity + // available. This means, for instance, that if + // an osv is indeed called, then all findings for + // the osv will have call stack info. + findings map[string][]*govulncheck.Finding } func NewHandler(w io.Writer) *handler { return &handler{ - w: w, - osvs: make(map[string]*osv.Entry), - levels: make(map[string]findingLevel), + w: w, + osvs: make(map[string]*osv.Entry), + findings: make(map[string][]*govulncheck.Finding), } } @@ -67,11 +72,52 @@ func foundAtLevel(f *govulncheck.Finding) findingLevel { return required } +// moreSpecific favors a call finding over a non-call +// finding and a package finding over a module finding. +func moreSpecific(f1, f2 *govulncheck.Finding) int { + if len(f1.Trace) > 1 && len(f2.Trace) > 1 { + // Both are call stack findings. + return 0 + } + if len(f1.Trace) > 1 { + return -1 + } + if len(f2.Trace) > 1 { + return 1 + } + + fr1, fr2 := f1.Trace[0], f2.Trace[0] + if fr1.Function != "" && fr2.Function == "" { + return -1 + } + if fr1.Function == "" && fr2.Function != "" { + return 1 + } + if fr1.Package != "" && fr2.Package == "" { + return -1 + } + if fr1.Package == "" && fr2.Package != "" { + return -1 + } + return 0 // findings always have module info +} + func (h *handler) Finding(f *govulncheck.Finding) error { - fLevel := foundAtLevel(f) - if fLevel > h.levels[f.OSV] { - h.levels[f.OSV] = fLevel + fs := h.findings[f.OSV] + if len(fs) == 0 { + fs = []*govulncheck.Finding{f} + } else { + if ms := moreSpecific(f, fs[0]); ms == -1 { + // The new finding is more specific, so we need + // to erase existing findings and add the new one. + fs = []*govulncheck.Finding{f} + } else if ms == 0 { + // The new finding is at the same level of precision. + fs = append(fs, f) + } + // Otherwise, the new finding is at a less precise level. } + h.findings[f.OSV] = fs return nil } @@ -102,6 +148,23 @@ func toVex(h *handler) Document { return doc } +// Given a slice of findings, returns those findings as a set of subcomponents +// that are unique per the vulnerable artifact's PURL. +func subcomponentSet(findings []*govulncheck.Finding) []Component { + var scs []Component + seen := make(map[string]bool) + for _, f := range findings { + purl := purlFromFinding(f) + if !seen[purl] { + scs = append(scs, Component{ + ID: purlFromFinding(f), + }) + seen[purl] = true + } + } + return scs +} + // statements combines all OSVs found by govulncheck and generates the list of // vex statements with the proper affected level and justification to match the // openVex specification. @@ -118,13 +181,16 @@ func statements(h *handler) []Statement { var statements []Statement for id, osv := range h.osvs { - if _, found := h.levels[id]; !found { + // if there are no findings emitted for a given OSV that means that + // the vulnerable module is not required at a vulnerable version. + if len(h.findings[id]) == 0 { continue } description := osv.Summary if description == "" { description = osv.Details } + s := Statement{ Vulnerability: Vulnerability{ ID: fmt.Sprintf("https://pkg.go.dev/vuln/%s", id), @@ -134,19 +200,22 @@ func statements(h *handler) []Statement { }, Products: []Product{ { - ID: DefaultPID, + Component: Component{ID: DefaultPID}, + Subcomponents: subcomponentSet(h.findings[id]), }, }, } - if h.levels[id] >= scanLevel { + // Findings are guaranteed to be at the same level, so we can just check the first element + fLevel := foundAtLevel(h.findings[id][0]) + if fLevel >= scanLevel { s.Status = StatusAffected } else { s.Status = StatusNotAffected s.ImpactStatement = Impact s.Justification = JustificationNotPresent // We only reach this case if running in symbol mode - if h.levels[id] == imported { + if fLevel == imported { s.Justification = JustificationNotExecuted } } diff --git a/internal/openvex/handler_test.go b/internal/openvex/handler_test.go index b2c760b..fa0d700 100644 --- a/internal/openvex/handler_test.go +++ b/internal/openvex/handler_test.go @@ -10,24 +10,23 @@ import ( "golang.org/x/vuln/internal/govulncheck" ) -func TestFinding(t *testing.T) { - const id1 = "ID1" +func TestSubcomponentSet(t *testing.T) { + id1 := "GO-2021-0265" + id2 := "GO-2022-1234" tests := []struct { name string - id string findings []*govulncheck.Finding - want findingLevel + want int }{ { - name: "multiple", - id: id1, + name: "multiple findings at same level different mod", findings: []*govulncheck.Finding{ { OSV: id1, Trace: []*govulncheck.Frame{ { - Module: "mod", - Package: "mod/pkg", + Module: "mod1", + Package: "mod1/pkg", }, }, }, @@ -35,9 +34,64 @@ func TestFinding(t *testing.T) { OSV: id1, Trace: []*govulncheck.Frame{ { - Module: "mod", - Package: "mod/pkg", - Function: "func", + Module: "mod2", + Package: "mod2/pkg2", + }, + }, + }, + }, + want: 2, + }, + { + name: "multiple Findings at same level same mod", + findings: []*govulncheck.Finding{ + { + OSV: id1, + FixedVersion: "v1.9.3", + Trace: []*govulncheck.Frame{ + { + Module: "github.com/tidwall/gjson", + Version: "v1.6.5", + Package: "github.com/tidwall/gjson", + Function: "Get", + }, + }, + }, + { + OSV: id1, + FixedVersion: "v1.9.3", + Trace: []*govulncheck.Frame{ + { + Module: "github.com/tidwall/gjson", + Version: "v1.6.5", + Package: "github.com/tidwall/gjson", + Function: "Get", + Receiver: "Result", + }, + }, + }, + }, + want: 1, + }, + { + name: "mix of findings for different osvs", + findings: []*govulncheck.Finding{ + { + OSV: id1, + Trace: []*govulncheck.Frame{ + { + Module: "mod1", + Version: "v1.0.0", + }, + }, + }, + { + OSV: id2, + Trace: []*govulncheck.Frame{ + { + Module: "mod2", + Version: "v1.0.0", + Package: "mod2/pkg2", }, }, }, @@ -45,25 +99,84 @@ func TestFinding(t *testing.T) { OSV: id1, Trace: []*govulncheck.Frame{ { - Module: "mod", + Module: "mod2", + Version: "v1.2.1", }, }, }, }, - want: called, + want: 3, }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - h := NewHandler(nil) - for _, f := range tt.findings { - if err := h.Finding(f); err != nil { - t.Errorf("handler.Finding() error = %v", err) - } + unique := subcomponentSet(tt.findings) + if len(unique) != tt.want { + t.Errorf("want %v findings, got %v instead", tt.want, unique) } - got := h.levels[tt.id] - if got != tt.want { - t.Errorf("want %v; got %v", tt.want, got) + }) + } +} + +// Copied from internal/sarif +func TestMoreSpecific(t *testing.T) { + frame := func(m, p, f string) *govulncheck.Frame { + return &govulncheck.Frame{ + Module: m, + Package: p, + Function: f, + } + } + + for _, tc := range []struct { + name string + want int + trace1 []*govulncheck.Frame + trace2 []*govulncheck.Frame + }{ + {"sym-vs-sym", 0, + []*govulncheck.Frame{ + frame("m1", "p1", "v1"), frame("m1", "p1", "f2")}, + []*govulncheck.Frame{ + frame("m1", "p1", "v2"), frame("m1", "p1", "f1"), frame("m2", "p2", "f2")}, + }, + {"sym-vs-pkg", -1, + []*govulncheck.Frame{ + frame("m1", "p1", "v1"), frame("m1", "p1", "f2")}, + []*govulncheck.Frame{ + frame("m1", "p1", "")}, + }, + {"pkg-vs-sym", 1, + []*govulncheck.Frame{ + frame("m1", "p1", "")}, + []*govulncheck.Frame{ + frame("m1", "p1", "v1"), frame("m2", "p2", "v2")}, + }, + {"pkg-vs-mod", -1, + []*govulncheck.Frame{ + frame("m1", "p1", "")}, + []*govulncheck.Frame{ + frame("m1", "", "")}, + }, + {"mod-vs-sym", 1, + []*govulncheck.Frame{ + frame("m1", "", "")}, + []*govulncheck.Frame{ + frame("m1", "p1", "v2"), frame("m1", "p1", "f1")}, + }, + {"mod-vs-mod", 0, + []*govulncheck.Frame{ + frame("m1", "", "")}, + []*govulncheck.Frame{ + frame("m2", "", "")}, + }, + } { + tc := tc + t.Run(tc.name, func(t *testing.T) { + f1 := &govulncheck.Finding{Trace: tc.trace1} + f2 := &govulncheck.Finding{Trace: tc.trace2} + if got := moreSpecific(f1, f2); got != tc.want { + t.Errorf("want %d; got %d", tc.want, got) } }) } diff --git a/internal/openvex/purl.go b/internal/openvex/purl.go new file mode 100644 index 0000000..f9c4c4f --- /dev/null +++ b/internal/openvex/purl.go @@ -0,0 +1,28 @@ +// Copyright 2024 The Go Authors. All rights reserved. +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE file. + +package openvex + +import ( + "net/url" + "strings" + + "golang.org/x/vuln/internal/govulncheck" +) + +// The PURL is printed as: pkg:golang/MODULE_PATH@VERSION +// Conceptually there is no namespace and the name is entirely defined by +// the module path. See https://github.com/package-url/purl-spec/issues/63 +// for further disucssion. + +// purlFromFinding takes a govulncheck finding and generates a purl to the +// vulnerable dependency. +func purlFromFinding(f *govulncheck.Finding) string { + var b strings.Builder + b.WriteString("pkg:golang/") + mod := f.Trace[0].Module + b.WriteString(url.PathEscape(mod)) + b.WriteString("@" + f.Trace[0].Version) + return b.String() +} diff --git a/internal/openvex/purl_test.go b/internal/openvex/purl_test.go new file mode 100644 index 0000000..3a72a36 --- /dev/null +++ b/internal/openvex/purl_test.go @@ -0,0 +1,65 @@ +// Copyright 2024 The Go Authors. All rights reserved. +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE file. + +package openvex + +import ( + "testing" + + "golang.org/x/vuln/internal/govulncheck" +) + +func TestPurlFromFinding(t *testing.T) { + for _, tt := range []struct { + name string + finding *govulncheck.Finding + wantPurl string + }{ + { + name: "module no package", + finding: &govulncheck.Finding{ + Trace: []*govulncheck.Frame{ + { + Module: "github.com/user/module", + Version: "v0.5.7", + }, + }, + }, + wantPurl: "pkg:golang/github.com%2Fuser%2Fmodule@v0.5.7", + }, + { + name: "module w/ package", + finding: &govulncheck.Finding{ + Trace: []*govulncheck.Frame{ + { + Module: "github.com/user/module", + Version: "v0.5.7", + Package: "github.com/user/module/pkg", + }, + }, + }, + wantPurl: "pkg:golang/github.com%2Fuser%2Fmodule@v0.5.7", + }, + { + name: "submodule", + finding: &govulncheck.Finding{ + Trace: []*govulncheck.Frame{ + { + Module: "github.com/user/module/submodule", + Version: "v0.5.7", + Package: "github.com/user/module/submodule/pkg", + }, + }, + }, + wantPurl: "pkg:golang/github.com%2Fuser%2Fmodule%2Fsubmodule@v0.5.7", + }, + } { + t.Run(tt.name, func(t *testing.T) { + gotPurl := purlFromFinding(tt.finding) + if gotPurl != tt.wantPurl { + t.Errorf("want: %v, got: %v", tt.wantPurl, gotPurl) + } + }) + } +} diff --git a/internal/openvex/vex.go b/internal/openvex/vex.go index e60ce75..18779f7 100644 --- a/internal/openvex/vex.go +++ b/internal/openvex/vex.go @@ -102,7 +102,12 @@ type Vulnerability struct { // Product identifies the products associated with the given vuln. type Product struct { - // For now, the ID will always be "Unknown product". - // This is temporary and is subject to change. + // The main product ID will remian default for now. + Component + // The subcomponent ID will be a PURL to the vulnerable dependency. + Subcomponents []Component `json:"subcomponents,omitempty"` +} + +type Component struct { ID string `json:"@id,omitempty"` }