Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 31 additions & 2 deletions internal/utility/vulns/vulnerability.go
Original file line number Diff line number Diff line change
Expand Up @@ -108,9 +108,34 @@ func rangeAffectsVersion(vulnID string, a []*osvschema.Range, pkg *extractor.Pac
return false
}

// parseAffectedEcosystem parses an advisory affected[] entry's ecosystem
// without panicking. The vendored osv-schema ecosystem set is pinned per
// release while osv.dev's set keeps growing, so a multi-ecosystem advisory can
// carry an ecosystem this build doesn't recognize. Skip that entry instead of
// crashing the whole scan, mirroring the "skip, don't panic on unsupported
// ecosystems" handling used for version parsing in rangeContainsVersion.
func parseAffectedEcosystem(affected *osvschema.Affected) (osvecosystem.Parsed, bool) {
ecosystem := affected.GetPackage().GetEcosystem()
Comment thread
Lougarou marked this conversation as resolved.
parsed, err := osvecosystem.Parse(ecosystem)
if err != nil {
cmdlogger.Debugf("skipping affected entry with unrecognized ecosystem %q: %v", ecosystem, err)
return osvecosystem.Parsed{}, false
}

return parsed, true
}

func AffectsEcosystem(v *osvschema.Vulnerability, ecosystemAffected osvecosystem.Parsed) bool {
for _, affected := range v.GetAffected() {
if osvecosystem.MustParse(affected.GetPackage().GetEcosystem()).Equal(ecosystemAffected) {
// Assume vulnerability has already been validated
if affected.GetPackage() == nil {
continue
}
affectedEcosystem, ok := parseAffectedEcosystem(affected)
if !ok {
continue
}
if affectedEcosystem.Equal(ecosystemAffected) {
return true
}
}
Expand Down Expand Up @@ -157,7 +182,11 @@ func IsAffected(v *osvschema.Vulnerability, pkg *extractor.Package) bool {
if affected.GetPackage() == nil {
continue
}
if osvecosystem.MustParse(affected.GetPackage().GetEcosystem()).Equal(imodels.Ecosystem(pkg)) &&
affectedEcosystem, ok := parseAffectedEcosystem(affected)
if !ok {
continue
}
if affectedEcosystem.Equal(imodels.Ecosystem(pkg)) &&
affected.GetPackage().GetName() == imodels.Name(pkg) {
if len(affected.GetRanges()) == 0 && len(affected.GetVersions()) == 0 {
cmdlogger.Warnf("%s does not have any ranges or versions - this is probably a mistake!", v.GetId())
Expand Down
61 changes: 61 additions & 0 deletions internal/utility/vulns/vulnerability_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,26 @@ func TestOSV_AffectsEcosystem(t *testing.T) {
Ecosystem: "npm",
Expected: true,
},
{
// an unrecognized ecosystem entry must be skipped, not panic, while a
// recognized sibling entry is still matched - see issue #2867.
Affected: []*osvschema.Affected{
{Package: &osvschema.Package{Ecosystem: "SomeFutureEcosystem"}},
{Package: &osvschema.Package{Ecosystem: "npm"}},
},
Ecosystem: "npm",
Expected: true,
},
{
// a nil package entry must be skipped (as IsAffected already does),
// while a recognized sibling entry is still matched.
Affected: []*osvschema.Affected{
{Package: nil},
{Package: &osvschema.Package{Ecosystem: "npm"}},
},
Ecosystem: "npm",
Expected: true,
},
}

for i, tt := range tests {
Expand Down Expand Up @@ -736,6 +756,47 @@ func TestOSV_EcosystemsWithSuffix(t *testing.T) {
}
}

// TestOSV_IsAffected_UnrecognizedEcosystemDoesNotPanic is a regression test for
// https://github.com/google/osv-scanner/issues/2867, where IsAffected panicked
// (`osvecosystem.MustParse`) on a multi-ecosystem advisory carrying an ecosystem
// the build's pinned osv-schema doesn't recognize. The unrecognized affected[]
// entry must be skipped rather than crash the scan, and a sibling entry in a
// recognized ecosystem must still match.
func TestOSV_IsAffected_UnrecognizedEcosystemDoesNotPanic(t *testing.T) {
t.Parallel()

defer func() {
if r := recover(); r != nil {
t.Errorf("IsAffected panicked on unrecognized ecosystem: %v", r)
}
}()

vuln := buildOSVWithAffected(
// an ecosystem this build's vendored osv-schema does not know - reached
// because the package is not in range for the recognized sibling below.
&osvschema.Affected{
Package: &osvschema.Package{Ecosystem: "SomeFutureEcosystem", Name: "my-package"},
Ranges: []*osvschema.Range{
buildEcosystemAffectsRange(&osvschema.Event{Introduced: "0"}),
},
},
&osvschema.Affected{
Package: &osvschema.Package{Ecosystem: string(osvconstants.EcosystemNPM), Name: "my-package"},
Ranges: []*osvschema.Range{
buildEcosystemAffectsRange(
&osvschema.Event{Introduced: "5.0.0"},
&osvschema.Event{Fixed: "6.0.0"},
),
},
},
)

// the unrecognized entry is skipped; the npm entry does not cover 1.0.0
expectIsAffected(t, vuln, "1.0.0", false)
// the npm entry does cover 5.1.0, proving matching still works post-skip
expectIsAffected(t, vuln, "5.1.0", true)
}

// TestOSV_IsAffected_UnsupportedEcosystemDoesNotPanic is a regression test for
// https://github.com/google/osv-scanner/issues/2831, where an SBOM containing
// a `pkg:github/...` component crashed the offline matcher because
Expand Down