Skip to content

fix: skip advisory affected entries with unrecognized ecosystems#2882

Open
Lougarou wants to merge 2 commits into
google:mainfrom
Lougarou:fix/2867-isaffected-unrecognized-ecosystem
Open

fix: skip advisory affected entries with unrecognized ecosystems#2882
Lougarou wants to merge 2 commits into
google:mainfrom
Lougarou:fix/2867-isaffected-unrecognized-ecosystem

Conversation

@Lougarou

Copy link
Copy Markdown

Overview

vulns.IsAffected and vulns.AffectsEcosystem panic when an OSV advisory's affected[].package.ecosystem is a value that osvecosystem.MustParse does not recognize. Both run on the match path for every advisory, and the per-ecosystem local-db zips store the full advisory record (every affected[] entry, not just the matching one), so a single multi-ecosystem advisory that lists an ecosystem newer than the build's pinned osv-schema aborts the entire offline scan.

Fixes #2867.

Details

osvecosystem.MustParse at internal/utility/vulns/vulnerability.go:113 (AffectsEcosystem) and :160 (IsAffected) panics with Failed MustParse: base ecosystem does not exist in osvschema: "…" on any unrecognized ecosystem string.

This is reachable without an attacker: osv.dev's ecosystem set grows over time while each osv-scanner release pins a vendored osv-schema, so once a new ecosystem appears in a multi-ecosystem advisory, older builds still pinned in CI panic on it. It is also deterministically triggerable today with a local advisory database.

This is the ecosystem-name analog of #2837, which fixed the version-parser panic in rangeContainsVersion; the ecosystem-name MustParse sites were not touched there.

The fix adds a small parseAffectedEcosystem helper that uses the non-panicking osvecosystem.Parse and skips the affected[] entry on error (logging at debug level), mirroring the existing "skip, don't panic on unsupported ecosystems" handling already used for version parsing. Recognized entries are matched exactly as before; the default path is unaffected.

Testing

  • Added TestOSV_IsAffected_UnrecognizedEcosystemDoesNotPanic: a multi-ecosystem advisory whose first entry uses an unrecognized ecosystem and whose second is a recognized npm range — asserts no panic, that the unrecognized entry is skipped, and that the recognized sibling still matches. The test fails (panics) without the fix.
  • Extended TestOSV_AffectsEcosystem with an unrecognized + recognized ecosystem case.
  • go test ./internal/utility/vulns/ passes.
  • Ran the pinned golangci-lint (v2.11.4): 0 issues.

Checklist

IsAffected and AffectsEcosystem called osvecosystem.MustParse on every
affected[] entry's ecosystem, which panics on any string the vendored
osv-schema registry doesn't recognize. Because the per-ecosystem local-db
zips store the full advisory record, a single multi-ecosystem advisory that
lists an ecosystem newer than the build's pinned schema crashes the entire
offline scan.

Parse the ecosystem with the non-panicking osvecosystem.Parse and skip the
affected[] entry on error, mirroring the "skip, don't panic on unsupported
ecosystems" handling already used for version parsing in rangeContainsVersion
(google#2837). Recognized entries are matched exactly as before.

Fixes google#2867.
Comment thread internal/utility/vulns/vulnerability.go
Mirror the nil-package guard IsAffected already has so AffectsEcosystem
skips affected[] entries without a package and does so silently, rather
than relying on the nil-safe getter. Addresses review feedback on google#2867.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

IsAffected panics on an advisory with an unrecognized ecosystem, crashing the whole scan

2 participants