fix(enrichment): VEX correctness, OSV attribution, DoS hardening, network trust surface#303
Merged
Conversation
…m silently suppressing real vulnerabilities First cluster from the enrichment security audit — the VEX overlay path, where a bug silently misreports a real vulnerability. All four defects converge on that failure mode and are verified end-to-end through VexEnricher::from_files().enrich_sbom() against the previous commit. CSAF status-mapping inversions (csaf.rs): - product_status.last_affected mapped to VexState::Fixed, but CSAF 2.0 §3.2.3.4 defines it as the LAST versions KNOWN AFFECTED — those versions ARE vulnerable. Fixed is non-actionable, so a genuinely-affected component was silently suppressed. Now maps to Affected. - product_status.recommended mapped to Affected, but it is the vendor-recommended FIXED version — a safe component was flagged as affected (false positive). Now maps to Fixed. - Added the missing first_affected field (→ Affected); documented the full 8-member product_status mapping inline. CSAF global-suppression (csaf.rs + mod.rs): - A product_status entry whose product_id did not resolve to a PURL was promoted to the global vuln-only table, which applies to EVERY component carrying that CVE. A CPE-only or unresolvable product under known_not_affected thus suppressed the CVE across unrelated components. A/B: OLD applied NotAffected to both of two unrelated components; NEW to neither. Unresolvable products are now dropped (counted + warned), matching the OpenVEX path, which already skipped them with a comment explaining exactly this risk — the two paths had been contradicting each other. CSAF has no document-wide statement, so nothing legitimate is dropped. CycloneDX VEX (cyclonedx_vex.rs + mod.rs): - Component-scoped statements are keyed by affects[].ref (a bom-ref, which lands in the component's format_id), but enrich_sbom looked them up only by PURL — so opaque-bom-ref statements silently never applied. The lookup now tries (vuln, purl) then (vuln, format_id). Empty bom-refs are rejected at the producer. - analysis.state "exploitable" collapsed to UnderInvestigation; it is a live actionable threat. Now maps to Affected (with resolved_with_pedigree → Fixed), byte-identical to the SBOM parser's impactAnalysisState mapping. Verified: end-to-end suppression A/B (6/6 cases fixed, no regression on correctly-resolved CSAF or the OpenVEX path) and a CSAF-2.0/CycloneDX spec-conformance audit (all 8 product_status members spec-correct, CDX mapping matches the parser). 5 new/updated tests; full suite green (1646 tests), clippy -D warnings and fmt clean, no snapshot churn. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…fan-out
Second cluster from the enrichment security audit — OSV attachment
integrity. The same false-negative failure mode as the VEX cluster: a
component silently gets the wrong vulnerabilities, or none.
Positional-zip misattribution (client.rs + mod.rs):
- enrich() attached querybatch results to components by array POSITION
(to_fetch.zip(flatten(results))) with no count or identity check — the
sole determinant of vuln→component attachment, and there is no local
version matching to catch a mistake. A truncated response silently
under-enriched (dropped a component's vulns); an extra/shifted response
attached a vulnerability to the WRONG component with no error, and the
wrong result was cached for the TTL. The OSV querybatch contract is
exactly one result per query, in order, and pagination never changes
result cardinality — so query_batch now validates results.len() ==
chunk.len() per chunk and rejects a violation as InvalidResponse
(fail-safe: no enrichment beats wrong enrichment). enrich() keeps a
defensive total-count guard that skips all attachment on any mismatch
and records an error rather than guessing by position.
A/B: OLD attached a vuln to a SAFE component with no error signal; NEW
refuses and records the contract violation.
Hydration fan-out cap (mod.rs):
- hydrate_vulns issued one sequential /v1/vulns/{id} GET per vuln stub,
and the stub count is supplied by the (untrusted) querybatch response —
unbounded request amplification. A per-run MAX_HYDRATION_REQUESTS
budget (10_000, shared across all components) now bounds it; on
exhaustion the remaining advisories fall back to id-only stubs and the
result is marked incomplete so it is not cached as authoritative. A
dedicated budget_hit flag drives the warning so it never false-fires on
exactly-N legitimate hydrations.
Negative-cache correctness (verified, no behavior change):
- The existing `complete` flag already gates caching correctly — a
genuine empty result (no vulns) is cached (correct negative caching)
while a failed hydration (404/5xx) is not cached and is retried.
Fixing the positional zip closed the remaining "wrong/empty result
cached for the wrong component" vector. Added tests pinning both.
Verified: misattribution A/B via httpmock (truncation + extra-result
classes both fail-safe in NEW, were misattributing/suppressing in OLD;
well-formed control unchanged; genuine empties cached, failures retried)
and an adversarial audit confirming the count check matches the real OSV
querybatch API contract. 3 new tests; full suite green, clippy
-D warnings and fmt clean.
Known follow-up (pre-existing, out of scope): OSV next_page_token
pagination is not followed, so a component with >1000 advisories
truncates to page 1 (does not affect the result-count contract).
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…p decompression Third cluster from the enrichment security audit — DoS hardening. Closes the OOM/hang vectors reachable from a hostile, MITM'd, or config-overridden endpoint (or a compromised registry mirror). Audit confirms no unbounded untrusted-body read remains in src/enrichment. Bounded response reads (route through source::read_bounded, the 256 MiB cap KEV/EPSS/HuggingFace already used): - OSV was the one client that skipped it: the querybatch body, the get_vulnerability body, and the error body (previously an unbounded response.text()) now read_bounded + serde_json::from_slice (osv/client.rs). - Dependency-staleness registry queries — npm/PyPI/crates.io — parsed the full response into serde_json::Value unbounded (registry.rs); now bounded. - endoflife.date product-list and cycles reads (eol/client.rs); now bounded. EPSS decompression bomb: - decode_maybe_gzip capped the COMPRESSED body (256 MiB) but then GzDecoder::read_to_string with no output cap — gzip expands up to ~1000:1, so a compliant compressed body could decompress into a ~250 GiB allocation (OOM). It now reads through Take(max+1) and rejects an overrun, bounding the DECOMPRESSED allocation. Split out decode_maybe_gzip_with_max (mirroring source::read_bounded_with_max) so the bomb rejection is testable with a small payload: the new test gzips 1 MiB of zeros (~1000:1) and confirms rejection under a small cap and clean decode under a large one. OSV retry backoff: - The retry delay was `Duration::from_secs(1 << (attempt - 1))` — an unchecked shift that overflows (uncapped multi-year sleep, and a debug panic at large attempt counts) for a big configured max_retries. Now uses the shared checked+capped backoff_delay (checked_shl + MAX_BACKOFF). read_bounded (oversized rejection) and backoff_delay (cap/overflow) are already unit-tested in source.rs, so these are wiring changes to proven helpers; the existing OSV/EOL/registry/EPSS http tests confirm the happy paths are unchanged. Verified: DoS-bound A/B (gzip bomb rejected + legit gzip still loads; all four unbounded-read families now bounded; backoff capped) and an adversarial audit confirming no unbounded untrusted-body read remains. 1 new test; full suite green (1650 tests), clippy -D warnings and fmt clean, no snapshot churn. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…sh provenance, cache integrity) Final cluster from the enrichment security audit — the network trust surface. Untrusted config, untrusted SBOM-derived request paths, and untrusted network/cache data crossing into trusted outputs. Config-URL validation (config/validation.rs): - api_base/kev_url/epss_url/huggingface_url overrides were unvalidated, accepting non-http schemes (file://, …) and malformed URLs for endpoints that supply security data. validate() now requires each to be a well-formed http(s) URL with a host. Private/self-hosted hosts stay allowed (legitimate internal mirrors); the check is dependency-light so it runs without the enrichment feature. HuggingFace hash provenance — stop network hashes becoming the verify baseline (model/metadata.rs, verification/model_dir.rs, huggingface): - `verify --model-dir` treated ALL component.hashes as the trusted integrity baseline, and the HF enricher injected registry/cache-sourced sha256 hashes into that list — so the tool verified a local file against a hash it fetched from the same place (circular trust; a poisoned cache or overridden URL sets the hash it is checked against). Hash gains a HashProvenance marker (Authored vs Enriched); PartialEq/ Eq/Hash IGNORE it and it is #[serde(skip)], so dedup, content-hashing, and SBOM emission are byte-identical (verified: content_hash and CycloneDX/SPDX emit unchanged). verify now trusts only Authored hashes; enriched hashes still satisfy AI-010. HuggingFace model_id injection (huggingface/client.rs): - model_id (from untrusted SBOM PURL/URL fields) was interpolated unencoded into the request URL and the cache filename. resolve() — the single choke point for both — now validates it against HuggingFace's repo-id grammar and skips anything malformed (../ , extra /, ?, #, …). Cache-filename collisions (huggingface, staleness/registry, eol): - The per-source cache_filename did replace(['/',':'],"_"), which is non-injective: npm:x / npm/x / npm_x — and EleutherAI/gpt-neo vs EleutherAI_gpt-neo — collapsed to one file, mis-attributing one entry's data to another. All three now hash the key (source::key_to_filename, SHA256 hex): collision-free and traversal-safe. Offline staleness bound (source.rs): - Offline mode served arbitrarily-expired cache entries (stale-if-offline) with only a log line — presenting months-old vuln data as current. It now bounds offline staleness at 90 days past TTL: air-gapped runs still work, but data too old to trust is a hard miss. Verified: trust-boundary A/B (verify no longer trusts enriched hashes — was circular; content-hash + CycloneDX/SPDX emit byte-identical; URL/ model_id/collision/staleness all fixed) and an adversarial audit (which caught the HF cache still colliding for distinct valid ids — fixed here and pinned by a test). 7 new tests; full suite green (1656 tests), clippy -D warnings and fmt clean, no snapshot churn. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes the 4 verified clusters from the enrichment subsystem audit (23 confirmed findings, 7 high). The dominant theme: enrichment could silently mark a user not affected by a real CVE.
1. VEX suppression correctness (
6cce6cd)product_statussemantics fixed:last_affected→ Affected (was ignored),recommended→ Fixed (was ignored), addedfirst_affectedunresolved_products) instead of being promoted to global unscoped statements that suppressed vulnerabilities across every componentexploitable→ Affected,resolved_with_pedigree→ Fixed (both previously fell through to UnderInvestigation); empty bom-refs rejected2. OSV attachment integrity (
dd78dad)querybatchresponses are validated per chunk:results.len()must equal the query count, otherwise the whole chunk errors instead of positionally misattributing vulnerabilities to the wrong components3. DoS hardening (
84a38cc)read_bounded(256 MiB cap)Take(max+1)+ overrun check) — kills the decompression-bomb vector4. Network trust surface (
b542bdd)api_base,kev_url,epss_url,huggingface_url) validated as http(s) with a non-empty hostHashProvenance::Enriched(serde-skipped, equality-ignored) soverify --model-dironly trusts authored hashes and emit stays byte-identicalEleutherAI/gpt-neovsEleutherAI_gpt-neocollisionVerification
Every cluster was ground-truth A/B'd against its parent commit (probe crates + httpmock mock servers for OSV/EPSS/KEV) and adversarially audited before commit; audit-found defects in the fixes themselves (e.g. the HF cache collision) were fixed pre-commit. Full suite, clippy
-D warnings, fmt clean at every commit.🤖 Generated with Claude Code
https://claude.ai/code/session_01CQvKahGY6VMYWSGMPBh77L