internal/vulns: handle osv.Entry with multiple modules correctly
VulnsForPackage returns all vulnerabilities affecting a package
in a module version, but when the package param is empty, this
is supposed to return all vulnerabilities affecting "any" package
in a module version. Pkgsite uses this mode when computing the
vulnerability affecting a module (for the version tab).
An osv.Entry may carry information about a vulnerability that
affects multiple modules. For example, GO-2022-0229 affects
a package in Go standard library (stdlib module) and a package
in golang.org/x/crypto module.
Unfortunately, VulnsForPackage did not check the module name
equality -- the module name info was added to VulnDB osv.Entry
much later after this code was written. When VulnsForPackage
was used to retrieve vulns for a given package, it wasn't an
issue because the package name equality check would prevent
picking a wrong osv.Affected entry. However, when it was used
to retrieve vulns for a module, this bug caused it to select
a vulnerability only based on the range info and behave incorrectly
when multiple modules with different version ranges exist.
This change adds the missing check on module name equality.
Also, it makes TestVulnsForPackage a table-driven test
and adds the case where osv.Entry carries packages from different
modules.
Fixes golang/go#56357
Change-Id: I58c7a21d543e510030e1ebddec907ebbd303f70f
Reviewed-on: https://go-review.googlesource.com/c/pkgsite/+/444679
Reviewed-by: Jamal Carvalho <jamal@golang.org>
Run-TryBot: Hyang-Ah Hana Kim <hyangah@gmail.com>
TryBot-Result: kokoro <noreply+kokoro@google.com>
diff --git a/internal/frontend/versions_test.go b/internal/frontend/versions_test.go
index 62d0e12..f82df0f 100644
--- a/internal/frontend/versions_test.go
+++ b/internal/frontend/versions_test.go
@@ -92,10 +92,18 @@
vulnEntry := &osv.Entry{
Details: "vuln",
Affected: []osv.Affected{{
+ Package: osv.Package{
+ Name: modulePath1,
+ },
Ranges: []osv.AffectsRange{{
Type: osv.TypeSemver,
Events: []osv.RangeEvent{{Introduced: "1.2.0"}, {Fixed: vulnFixedVersion}},
}},
+ EcosystemSpecific: osv.EcosystemSpecific{
+ Imports: []osv.EcosystemSpecificImport{{
+ Path: v1Path,
+ }},
+ },
}},
}
getVulnEntries := func(_ context.Context, m string) ([]*osv.Entry, error) {
diff --git a/internal/vulns/vulns.go b/internal/vulns/vulns.go
index 80df880..9265609 100644
--- a/internal/vulns/vulns.go
+++ b/internal/vulns/vulns.go
@@ -68,7 +68,7 @@
// package at this version.
var vulns []Vuln
for _, e := range entries {
- if vuln, ok := entryVuln(e, packagePath, version); ok {
+ if vuln, ok := entryVuln(e, modulePath, packagePath, version); ok {
vulns = append(vulns, vuln)
}
}
@@ -107,9 +107,10 @@
return affected
}
-func entryVuln(e *osv.Entry, packagePath, version string) (Vuln, bool) {
+func entryVuln(e *osv.Entry, modulePath, packagePath, version string) (Vuln, bool) {
for _, a := range e.Affected {
- if !a.Ranges.AffectsSemver(version) {
+ // a.Package.Name is Go "module" name. Go package path is a.EcosystemSpecific.Imports.Path.
+ if a.Package.Name != modulePath || !a.Ranges.AffectsSemver(version) {
continue
}
if packageMatches := func() bool {
diff --git a/internal/vulns/vulns_test.go b/internal/vulns/vulns_test.go
index 1007c7d..dd27adc 100644
--- a/internal/vulns/vulns_test.go
+++ b/internal/vulns/vulns_test.go
@@ -18,18 +18,30 @@
func TestVulnsForPackage(t *testing.T) {
ctx := context.Background()
e := osv.Entry{
- Details: "bad",
+ ID: "GO-1",
Affected: []osv.Affected{{
Package: osv.Package{Name: "bad.com"},
Ranges: []osv.AffectsRange{{
Type: osv.TypeSemver,
- Events: []osv.RangeEvent{{Introduced: "0"}, {Fixed: "1.2.3"}},
+ Events: []osv.RangeEvent{{Introduced: "0"}, {Fixed: "1.2.3"}}, // fixed at v1.2.3
}},
EcosystemSpecific: osv.EcosystemSpecific{
Imports: []osv.EcosystemSpecificImport{{
Path: "bad.com",
}},
},
+ }, {
+ Package: osv.Package{Name: "unfixable.com"},
+ Ranges: []osv.AffectsRange{{
+ Type: osv.TypeSemver,
+ Events: []osv.RangeEvent{{Introduced: "0"}}, // no fix
+ }},
+ DatabaseSpecific: osv.DatabaseSpecific{},
+ EcosystemSpecific: osv.EcosystemSpecific{
+ Imports: []osv.EcosystemSpecificImport{{
+ Path: "unfixable.com",
+ }},
+ },
}},
}
@@ -37,28 +49,52 @@
switch modulePath {
case "good.com":
return nil, nil
- case "bad.com":
+ case "bad.com", "unfixable.com":
return []*osv.Entry{&e}, nil
default:
return nil, fmt.Errorf("unknown module %q", modulePath)
}
}
- got := VulnsForPackage(ctx, "good.com", "v1.0.0", "good.com", get)
- if got != nil {
- t.Errorf("got %v, want nil", got)
+ testCases := []struct {
+ mod, pkg, version string
+ want []Vuln
+ }{
+ // Vulnerabilities for a package
+ {
+ "good.com", "good.com", "v1.0.0", nil,
+ },
+ {
+ "bad.com", "bad.com", "v1.0.0", []Vuln{{ID: "GO-1"}},
+ },
+ {
+ "bad.com", "bad.com/ok", "v1.0.0", nil, // bad.com/ok isn't affected.
+ },
+ {
+ "bad.com", "bad.com", "v1.3.0", nil,
+ },
+ {
+ "unfixable.com", "unfixable.com", "v1.999.999", []Vuln{{ID: "GO-1"}},
+ },
+ // Vulnerabilities for a module (package == "")
+ {
+ "good.com", "", "v1.0.0", nil,
+ },
+ {
+ "bad.com", "", "v1.0.0", []Vuln{{ID: "GO-1"}},
+ },
+ {
+ "bad.com", "", "v1.3.0", nil,
+ },
+ {
+ "unfixable.com", "", "v1.999.999", []Vuln{{ID: "GO-1"}},
+ },
}
- got = VulnsForPackage(ctx, "bad.com", "v1.0.0", "bad.com", get)
- want := []Vuln{{
- Details: "bad",
- }}
- if diff := cmp.Diff(want, got); diff != "" {
- t.Errorf("mismatch (-want, +got):\n%s", diff)
- }
-
- got = VulnsForPackage(ctx, "bad.com", "v1.3.0", "bad.com", get)
- if got != nil {
- t.Errorf("got %v, want nil", got)
+ for _, tc := range testCases {
+ got := VulnsForPackage(ctx, tc.mod, tc.version, tc.pkg, get)
+ if diff := cmp.Diff(tc.want, got); diff != "" {
+ t.Errorf("VulnsForPackage(%q, %q, %q) = %+v, mismatch (-want, +got):\n%s", tc.mod, tc.version, tc.pkg, tc.want, diff)
+ }
}
}
diff --git a/tests/screentest/testdata/ci/vuln-nonstdlib-module-540x1080.a.png b/tests/screentest/testdata/ci/vuln-nonstdlib-module-540x1080.a.png
index 6992ede..c7f9a69 100644
--- a/tests/screentest/testdata/ci/vuln-nonstdlib-module-540x1080.a.png
+++ b/tests/screentest/testdata/ci/vuln-nonstdlib-module-540x1080.a.png
Binary files differ
diff --git a/tests/screentest/testdata/ci/vuln-nonstdlib-module.a.png b/tests/screentest/testdata/ci/vuln-nonstdlib-module.a.png
index 9608f2d..12a4ac7 100644
--- a/tests/screentest/testdata/ci/vuln-nonstdlib-module.a.png
+++ b/tests/screentest/testdata/ci/vuln-nonstdlib-module.a.png
Binary files differ
