feat: add go symbol matching qualifier for better go vuln matching#3509
feat: add go symbol matching qualifier for better go vuln matching#3509spiffcs wants to merge 22 commits into
Conversation
f11bb65 to
896f135
Compare
govulndb's per-symbol reachability information used as a go-imports package qualifier called at match time to reduce false positives for Go binaries. When a Go binary is cataloged with symbol capture enabled, the qualifier only matches a vulnerability if at least one of the vulnerable symbols is actually present in the binary. Packages without symbol evidence continue to match at module granularity, preserving existing behavior. Signed-off-by: Christopher Phillips <32073428+spiffcs@users.noreply.github.com>
Signed-off-by: Christopher Phillips <32073428+spiffcs@users.noreply.github.com>
a63589f to
b621b65
Compare
Signed-off-by: Christopher Phillips <32073428+spiffcs@users.noreply.github.com>
Signed-off-by: Christopher Phillips <32073428+spiffcs@users.noreply.github.com>
Signed-off-by: Will Murphy <willmurphyscode@users.noreply.github.com>
Signed-off-by: Christopher Phillips <32073428+spiffcs@users.noreply.github.com>
Signed-off-by: Christopher Phillips <32073428+spiffcs@users.noreply.github.com>
Signed-off-by: Christopher Phillips <32073428+spiffcs@users.noreply.github.com>
Signed-off-by: Christopher Phillips <32073428+spiffcs@users.noreply.github.com>
* origin/main: chore(deps): bump gorm.io/gorm from 1.31.1 to 1.31.2 (#3557) chore(deps): bump actions/cache in /.github/actions/bootstrap (#3558) chore(deps): bump anchore/go-make/.github/actions/setup (#3555) chore(deps): bump actions/cache from 5.0.5 to 6.1.0 (#3551) chore(deps): bump actions/cache/restore in /.github/actions/bootstrap (#3556) chore(deps): bump golang.org/x/tools from 0.46.0 to 0.47.0 (#3550) chore(deps): bump anchore/workflows/.github/workflows/codeql.yaml (#3548) chore(deps): bump zizmorcore/zizmor-action from 0.5.6 to 0.5.7 (#3549) chore(deps): bump anchore/workflows/.github/workflows/check-version-available.yaml (#3547) fix(zarf): swap warn messages for debug (#3545) feat: populate package architecture for matching (#3504) chore(deps): update quality gate database (#3543)
Signed-off-by: Christopher Phillips <32073428+spiffcs@users.noreply.github.com>
Signed-off-by: Christopher Phillips <32073428+spiffcs@users.noreply.github.com>
Signed-off-by: Christopher Phillips <32073428+spiffcs@users.noreply.github.com>
| // GoImports lists the packages and symbols within an affected Go module that contain the vulnerability | ||
| // (from govulndb's ecosystem_specific.imports). When set, packages carrying binary symbol evidence only | ||
| // match if at least one of the listed symbols is present in the binary. | ||
| GoImports []GoImport `json:"go_imports,omitempty"` |
There was a problem hiding this comment.
In the grype report what of the symbol data should we end up including? Just what the intersection was that caused the match or the full table for the package?
| var added []db.GoImport | ||
| for _, imp := range imports { | ||
| if existing[imp.Path] { | ||
| continue |
There was a problem hiding this comment.
I doubt this is likely, but I'll mention it anyway: if there are two go records aliasing the same GHSA that list the same import path with different symbol sets then the second set is silently dropped instead of being combined.
| // writeEntryToBatch fills in missing severity and adds the entry to the write | ||
| // batches. Held govulndb/GHSA entries also come back through here at Close | ||
| // time, after reconciliation (see govulndb_merge.go), so severity fill always | ||
| // runs with the complete NVD cache. | ||
| func (w *writer) writeEntryToBatch(entry transformers.RelatedEntries) error { |
There was a problem hiding this comment.
why hold all of the go and ghsa records? We have a mechanism for ordering providers one before another (today we have NVD before others). I feel that we could write ghsa's as usual then patch overlapping ghsa records when we write the go vuln records --which doesn't requiring holding any/all records in memory and write them at close.
I did not look at the *provider caches that we have and how they are affected.
Was there specific frictions/reasons why to hold writes for all records?
There was a problem hiding this comment.
We talked about this on a sync, but to record the decision here:
- We're not holding all the ghsa records, just the ones from the Go ecosystem
- Write once was deemed a better path than write then patch because of the additional complexity of adding new patch code.
If we run into issues with write once we can come back on a follow up to add the PATCH methods to the db builder/reconciler
Signed-off-by: Alex Goodman <wagoodman@users.noreply.github.com>
| if !govulndbEmits(affected.Package.Name) { | ||
| continue | ||
| var qualifiers *db.PackageQualifiers | ||
| if imports := govulndbImports(affected, vuln.ID); len(imports) > 0 { |
There was a problem hiding this comment.
Is it worth also capturing an architecture qualifier, e.g. https://vuln.go.dev/ID/GO-2022-0187.json is only affected on amd64. That's probably out of scope here but i wanted to check what others think. My vote would be to add it as a fast follow but still wanted to mention it.
Signed-off-by: Christopher Phillips <32073428+spiffcs@users.noreply.github.com>
Signed-off-by: Christopher Phillips <32073428+spiffcs@users.noreply.github.com>
Signed-off-by: Will Murphy <willmurphyscode@users.noreply.github.com>
Signed-off-by: Christopher Phillips <32073428+spiffcs@users.noreply.github.com>
Signed-off-by: Christopher Phillips <32073428+spiffcs@users.noreply.github.com>
Signed-off-by: Christopher Phillips <32073428+spiffcs@users.noreply.github.com>
govulndb fix versions source from ecosystem_specific.custom_ranges withdrawn records written without GHSA reconciliation (NO GHSA) reach the writer with a fixed range but no availability dates This fails failOnMissingFixDate builds: (e.g. GO-2024-2442 / github.com/gravitational/teleport: `missing fix date for version "13.4.13"`). This fix mirrors the upstream providers' fallback we stamp the fix date as the advisory's own published date (then modified) with kind "first-observed-record" on any reconciled go record whose fixed range lacks one. Signed-off-by: Christopher Phillips <32073428+spiffcs@users.noreply.github.com>
Go symbol matching: stdlib and golang.org/x advisories match correctly based on new symbol data; merge govulndb symbol data onto GHSA records
The [Go Vuln DB] (https://vuln.go.dev/) is the only source for Go standard library advisories. It is also an authoritative source for a share of golang.org/x/* advisories.
Grype recently started hosting GoVulnDB records in its DB (#3527, #3534, #3527).
Keeping these records exposed without symbol matching lead to matching inaccuracies. We also would see inaccuracies from cases in GHSA where we did not have symbol data and only matched at the module/version range level.
PR Features
Data Decisions
Where a GHSA covers the same module, the GHSA becomes the single, symbol-enriched record.
affected[].package.nameecosystem_specific.custom_rangeswe patch in the custom_rangeSchema:
Known limitations / follow-ups (tracked separately)