Repository navigation
fix: related-purl ignore + normalize_license overflow + snippet fallback (#120) [main port] - #126
Merged
Merged
Conversation
The ignore list (-b SBOM.json) was only compared against the single primary purl stored in the url record (field 6, purls[0]). A project is often published under several purls (e.g. a github repo also indexed as a golang / nuget / conda package); those sibling identities are materialised only later by fetch_related_purls() at report time. As a result, ignoring a purl that was a *related* purl of the matched component had no effect: the match survived under its primary identity and the ignored purl resurfaced verbatim in the reported "purl" array. Fix: after load_matches() builds the candidate component list, drop every component whose primary OR related purl is in the ignore list, before best-match selection. A legitimate non-ignored candidate for the same region then wins, or the finding is suppressed entirely when none remain. - ignored_purl(): check a single purl string against the ignore list. - component_ignored_by_purl(): resolve related purls and test them all. - component_list_remove_ignored(): filter the component list in place. - load_matches(): invoke the filter, guarded by `ignore_components` so there is zero overhead when -b is not used. Verified against the ossv2 KB: ignoring pkg:nuget/libcurl-v143-shared (a related purl of pkg:github/curl/curl) now suppresses that component, while a scan without -b is byte-identical to before (only elapsed time differs). valgrind is clean for the new code paths. Adds a data-driven regression test that auto-discovers a multi-purl component and asserts suppression. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> (cherry picked from commit 1eaf941)
normalize_license() replaces a license string in place with its canonical SPDX id (e.g. "Apache" -> "Apache-2.0", "GPLv2" -> "GPL-2.0-only"), which can be LONGER than the matched text. License buffers were allocated by license_add_to_list() sized to the ORIGINAL string length (rounded up to an 8-byte boundary), so normalizing a short alias into a longer canonical id wrote past the end of the buffer. valgrind flagged this as an "Invalid write of size 1" in strcpy (license.c) followed by "Invalid read" in string_clean() walking off the end of the corrupted, non-terminated buffer. It reproduces whenever a reported component declares a license whose canonical SPDX id is longer than its stored alias. Fix: - license_add_to_list(): reserve at least MAX_NORMALIZED_LICENSE_LN (32) bytes so any canonical id in license_normalization[] (longest is "LGPL-3.0-or-later", 17 chars) fits even when the original text is shorter. - normalize_license(): defensively skip the in-place copy if the canonical id would not fit, guarding against future table entries. Verified with valgrind on the same scan: the Invalid write/read in the license path are gone, normalization still emits the correct SPDX ids (e.g. GPL-2.0-only, Apache-2.0), and a scan without -b is unchanged. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> (cherry picked from commit 1e31105)
… ignored When a file produces a full-file match (MATCH_FILE) but every candidate component is removed by the ignore list (-b), the result used to collapse to "none": the winnowing/snippet pass is skipped for file matches, so there was nothing left to report once the components were filtered. The engine now detects that case and automatically retries once as a fresh snippet scan over the same file, honoring the same ignore/include lists, so a legitimate snippet match can still surface instead of reporting nothing. Implementation (src/scan.c): - scan_data_reset(): discards the scan RESULTS (matches, matchmap, best match) while keeping the scan identity, config and the reusable hash/line buffers, mirroring the result-freeing half of scan_data_free(). This is the "throw the results away and start fresh" step. - scan_has_reportable_match(): true if any match still has components. - ldb_scan(): the file/snippet/compile body now runs in a loop. If the first pass was a MATCH_FILE, an ignore list is active, and nothing is reportable, it resets and re-runs forcing snippets (once, guarded against re-entry). The trigger is captured before compile_matches(), which resets match_type to MATCH_NONE once the collapsed match has no components. Scope and safety: - Zero behavior change unless -b is used AND a full-file match is fully filtered: verified byte-identical output otherwise. - Verified against the ossv2 KB: a full-file curl match whose purls are all ignored now falls back to a snippet match instead of "none"; valgrind shows no new leaks or invalid accesses from the reset/second pass. - Adds tests/snippet_fallback_on_ignore/ (data-driven, skips without a KB; confirmed to fail pre-change and pass after). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> (cherry picked from commit 263505c)
mscasso-scanoss
changed the title
…ore tests runnable scan_data_reset() freed each matches_list_array[i] but left the dangling pointer in place. On the snippet second pass biggest_snippet() only creates a list for NULL slots, so it wrote into freed lists (use-after-free, segfault when a full-file match was fully filtered by -b). Tests: - add tests/snippet_multicomponent/main.c.wfp (from the crc64 line), the input both ignore tests depend on; it was missing so they always skipped. - ignore_related_purl: drop -M 9, which hits a pre-existing -M crash on main. - snippet_fallback_on_ignore: try a known low-copy full-file hash when the one derived from the probe cannot be peeled (curl has dozens of copies). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.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
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.
Fixes #120
Port to
main(5.4.x) of the two fixes landed for thecrc64line in #125. Cherry-picked cleanly; the ignore and license code is identical between the two branches.1. Ignoring a related PURL must suppress the component (#120)
The ignore list (
-b SBOM.json) was only compared against the single primary purl stored in the url record (field 6 →purls[0]).ignored_asset_match()runs per url record at fetch time and never sees a component's related purls.A project is frequently published under several purls — e.g. a GitHub repo also indexed as a golang / nuget / conda package. Those sibling identities are materialised only at report time by
fetch_related_purls(). So ignoring a purl that was a related purl of the matched component had no effect: the match survived under its primary identity and the ignored purl resurfaced in the reported"purl": [...]array. (In #120: ignoring the first purl worked — it was the primary one — but ignoring a second purl of the same project did nothing.)Fix: after
load_matches()builds the candidate component list, and before best-match selection, drop every component whose primary OR related purl is in the ignore list. A legitimate non-ignored candidate then wins, or the finding is suppressed when none survive.ignored_purl()— check a single purl string against the ignore list.component_ignored_by_purl()— resolve related purls and test every purl of the component.component_list_remove_ignored()— filter the component list in place.load_matches()— invoke the filter, guarded byignore_componentsso there is zero overhead when-bis not used.2.
normalize_license()buffer overflownormalize_license()rewrites a license string in place with its canonical SPDX id (e.g.Apache→Apache-2.0,GPLv2→GPL-2.0-only), which can be longer than the matched text.license_add_to_list()sized the buffer to the original string length, so normalizing a short alias into a longer canonical id wrote past the end (valgrind:Invalid write of size 1instrcpy, then anInvalid readinstring_clean()running off the unterminated buffer).Fix: reserve at least
MAX_NORMALIZED_LICENSE_LN(32) bytes for license buffers (longest canonical id isLGPL-3.0-or-later, 17 chars) and defensively skip the in-place copy if a future table entry would not fit.3. Snippet fallback when a full-file match is fully ignored
When a file produces a full-file match (
MATCH_FILE) but every candidate component is removed by the ignore list, the result used to collapse to"none"— the winnowing/snippet pass is skipped for file matches, so nothing was left to report.The engine now detects that case and automatically retries once as a fresh snippet scan over the same file, honoring the same ignore/include lists, so a legitimate snippet match can still surface.
scan_data_reset()discards the scan results (matches, matchmap, best match) while keeping the scan identity, config and the reusable hash/line buffers — mirroring the result-freeing half ofscan_data_free().ldb_scan()'s file/snippet/compile body now runs in a loop; it resets and re-runs forcing snippets (once) when a full-file match is fully filtered under an active ignore list.Zero behaviour change unless
-bis used and a full-file match is fully filtered. Addstests/snippet_fallback_on_ignore/(data-driven, skips without a KB).Verification (done on the
crc64build, #125)Reproduced #120 with a real multi-purl component:
pkg:github/curl/curlcarriespkg:nuget/libcurl-v143-sharedas a related purl.pkg:nuget/libcurl-v143-shared-belapseddiffers)"none"Apache→Apache-2.0, …)Test
tests/ignore_related_purl/— data-driven regression test: it peels region candidates until a component exposing ≥2 purls surfaces, asks the engine to ignore one of that component's related purls, and asserts the whole component disappears. Skips gracefully without a KB. Confirmed to fail pre-fix and pass after on thecrc64build.🤖 Generated with Claude Code