[tinobino]

Description of Change

The UnusedSuppressionRuleAnalyzer only reports a suppression rule if it did not match anything at all (SuppressionRule.isMatched() is a single flag per rule). A rule with several <cve>, <cwe>, <vulnerabilityName> or <cpe> entries counts as used as soon as one of its entries matches. Entries that became obsolete in the meantime, e.g. because the dependency was updated and the vulnerability is gone, stay unnoticed and pile up in the suppression file.

This change tracks usage per entry:

• SuppressionRule remembers which cve, cwe, vulnerabilityName and cpe entries matched at least once. New method getUnusedEntries() returns the others.
• UnusedSuppressionRuleAnalyzer reports a rule that was used but still has unused entries (Suppression Rule had unused entries [...]: <rule>), with the same severity and counting as for completely unused rules. Rules that did not match at all are reported as before.

Notes:

• Behavior change: with analyzer.suppression.unused.fail enabled, rules that are only partly unused now cause a failure too.
• Entries are tracked by kind and position in the rule (small immutable RuleEntry), not by value, so duplicate entries and entries with the same value but different matching (e.g. case-sensitive or not) are told apart.
• The used entries are stored in a concurrent set, because VulnerabilitySuppressionAnalyzer supports parallel processing and so processes one rule from several threads at once. The set is only read in a later analysis phase, after all analysis tasks have completed.
• If several entries of a rule match the same vulnerability (or identifier), only the first one is counted as used, the others are reported. These are redundant anyway.
• Only the vulnerability info of a rule (cpe, cwe, cve, vulnerabilityName) is tracked per entry. The scope conditions (packageUrl, filePath, sha1, gav) select the dependency, and cvssBelow is a noise filter rather than a false-positive filter. None of them is reported as unused.
• In the Ant task the new message (like the existing "zero matches" one) is only visible with the logging fix in fix(ant): log messages from analyzer worker threads #8880, since analyzer threads currently drop all log output there.

Related issues

None.

Have test cases been added to cover the new functionality?

yes

The tests were added in separate commits before the change that makes them pass.

• UnusedSuppressionRuleAnalyzerTest.testCheckUnusedEntriesOfMatchedRule: a rule that suppressed a vulnerability, but has a second obsolete entry, is counted as unused.
• SuppressionRuleTest.testGetUnusedEntries: lists exactly the unused cve and vulnerabilityName entries.
• SuppressionRuleTest.testGetUnusedEntriesWithCpe: lists an unused cpe entry.
• SuppressionRuleTest.testGetUnusedEntriesWithDuplicates: the second of two identical entries is reported.
• SuppressionRuleTest.testGetUnusedEntriesWithSameValueButDifferentMatching: an entry with the same value as a matching one, but which does not match itself, is reported.
• The existing UnusedSuppressionRuleAnalyzerTest.getSuppressionRule set matched by hand without the entry suppressing anything. It now lets the entry suppress a vulnerability, as in a real run, and no longer adds a cpe entry that never matched anything.

[Copilot]

🟡 Changes recommended

Value-only tracking conflates distinct suppression entries, incorrectly marking unmatched entries as used and undermining unused-entry reporting and fail-on-unused enforcement.

1 open issue

• Track duplicate vulnerability entries by position · New

What changed in this PR

This PR extends suppression usage tracking so DependencyCheck can report obsolete entries within otherwise-used rules, rather than only completely unused rules.

Changes:

• Tracks usage of individual CVE, CWE, and vulnerability-name entries with concurrent storage.
• Reports partially unused rules and counts them toward fail-on-unused enforcement.
• Adds regression tests and updates test fixtures to exercise actual suppression.

File	Description
core/​src/​test/​java/​org/​owasp/​dependencycheck/​xml/​suppression/​SuppressionRuleTest.java	Tests unused-entry reporting after suppression.
core/​src/​test/​java/​org/​owasp/​dependencycheck/​analyzer/​UnusedSuppressionRuleAnalyzerTest.java	Tests partially unused rule counting and updates fixtures.
core/​src/​main/​java/​org/​owasp/​dependencycheck/​xml/​suppression/​SuppressionRule.java	Records matched entries and exposes unused entries.
core/​src/​main/​java/​org/​owasp/​dependencycheck/​analyzer/​UnusedSuppressionRuleAnalyzer.java	Logs and counts partially unused non-base rules.

_{Review effort: Balanced}

[tinobino]

I haven't yet looked at this in detail but it's logically incorrect to exclude cpe and I don't know why this was done.

You're right. cpe acts on the identifiers of the dependency rather than on its vulnerabilities, which is where I stopped, but there was no good reason to leave it out: a <cpe> entry that never matches is just as obsolete as an unused <cve>.

Tracked now in cef575a, with the test that fails without it in e054201.

[chadlwilson]

Cpe does act on identifiers, but it's important to understand a key distinction - the source of the CPE for a given artifact is part of the vulnerability information, it cannot be directly derived from any software dependency. It is essentially "affected versions" metadata alongside a CVE from NVD.

ODC guesses a bunch of different CPE identifiers based on evidence it gathers and then tries to find any vulns that match. It removes any guessed CPEs that are not known from NVD reported data, so the CPE content shifts over time even if the dependency metadata, file etc stays the same. The biggest source of false positives and thus suppressions are bad CPE matches.

This is why in the suppression schema CPE is part of the second "vulnerability info" 1..many chunk (alongside cwe, CVE, vulnerabilityName) rather than the first optional 0..many chunk (packageUrl, file, sha) which is applied to the dependency directly ("scope", essentially)

I think I agree with not tracking cvssBelow as these are more like "noise/risk filters" rather than "false positive filters", as long as they are not reported as "unused" in the current logic.

[tinobino]

Cpe does act on identifiers, but it's important to understand a key distinction - the source of the CPE for a given artifact is part of the vulnerability information, it cannot be directly derived from any software dependency. It is essentially "affected versions" metadata alongside a CVE from NVD.

ODC guesses a bunch of different CPE identifiers based on evidence it gathers and then tries to find any vulns that match. It removes any guesses CPEs that are not known from NVD reported data, so the CPE content shifts over time even if the dependency metadata, file etc stays the aame. The biggest source of false positives and thus suppressions are bad CPE matches.

This is why in the suppression schema CPE is part of the second "vulnerability info" 1..many chunk (alongside cwe, CVE, vulnerabilityName) rather than the first optional 0..many chunk (packageUrl, file, sha) which is applied to the dependency directly ("scope", essentially)

I think I agree with not tracking cvssBelow as these are more like "noise/risk filters" rather than "false positive filters", as long as they are not reported as "unused" in the current logic.

Thanks for the explanation, that makes sense: cpe belongs to the vulnerability info part of the rule (like cwe, cve and vulnerabilityName), not to the scope part (packageUrl, file, sha) that selects the dependency. My earlier reasoning that it acts on the dependency's identifiers was wrong. It is tracked now either way.

cvssBelow and the scope conditions are not tracked, and getUnusedEntries() never reports them, so a rule is not flagged because of those.

[Copilot]

🟢 Approval recommended

No active review issues are recorded for the suppression usage reporting changes.

No open issues 🎉

1 resolved since last review

• Track duplicate vulnerability entries by position

_{Review effort: Balanced}

[chadlwilson]

This looks better, thanks! I'll take a deeper look later and give it a try myself on a real repo, to see how it does.

I have a mild concern that if two possible RuleEntrys match, that only reporting the first match as used might lead to some undesirable consequences, that might be annoying for users if there is no easy way to opt out of the individual tracking. But I'll think about it a bit more. Perhaps it is already possible to get into this situation if you have two separate SuppressionRules that match a given vuln - the first will match and the second will report as unused? If so, then it probably doesn't make anything worse.

[tinobino]

This looks better, thanks! I'll take a deeper look later and give it a try myself on a real repo, to see how it does.

I have a mild concern that if two possible RuleEntrys match, that only reporting the first match as used might lead to some undesirable consequences, that might be annoying for users if there is no easy way to opt out of the individual tracking. But I'll think about it a bit more. Perhaps it is already possible to get into this situation if you have two separate SuppressionRules that match a given vuln - the first will match and the second will report as unused? If so, then it probably doesn't make anything worse.

Thanks for taking a look, and no rush.

On your concern: I believe that situation already exists today. AbstractSuppressionAnalyzer applies the rules one after another, and SuppressionRule.process removes the suppressed vulnerability from the dependency at the end (dependency.removeVulnerability). So a second rule that matches the same vulnerability never sees it and is reported with "zero matches". The per-entry tracking applies the same "first match wins" logic to the entries within one rule, so it should not make anything worse. I added testGetUnusedEntriesWithTwoRulesForSameVulnerability (d501981) to document this existing behavior.

On opting out: there is nothing per entry. Without analyzer.suppression.unused.fail the new message is only logged at INFO, so it does not break builds by default. If you would prefer, I can make the per-entry reporting independent of the rule-level one (e.g. a separate setting), but I would wait for how it behaves on a real repo first.