From 4f91689156b836e0fecdaec21dc33411759a7012 Mon Sep 17 00:00:00 2001 From: Arnout Engelen Date: Tue, 3 Oct 2023 13:22:58 +0200 Subject: [PATCH] feat: suppress by 'includedBy' Add an option to suppress a vulnerability when it is only found as a transitive dependency via the listed dependencies. (WIP, needs further testing) Related to #5686 --- .../xml/suppression/SuppressionHandler.java | 7 ++ .../xml/suppression/SuppressionRule.java | 64 ++++++++++++++++++- .../src/main/resources/schema/suppression.xsd | 3 +- .../xml/suppression/SuppressionRuleTest.java | 40 ++++++++++++ 4 files changed, 112 insertions(+), 2 deletions(-) diff --git a/core/src/main/java/org/owasp/dependencycheck/xml/suppression/SuppressionHandler.java b/core/src/main/java/org/owasp/dependencycheck/xml/suppression/SuppressionHandler.java index 3ea7aebbf3c..5cdb47aefbc 100644 --- a/core/src/main/java/org/owasp/dependencycheck/xml/suppression/SuppressionHandler.java +++ b/core/src/main/java/org/owasp/dependencycheck/xml/suppression/SuppressionHandler.java @@ -88,6 +88,10 @@ public class SuppressionHandler extends DefaultHandler { * The cvssBelow element name. */ public static final String CVSS_BELOW = "cvssBelow"; + /** + * The includedBy element name. + */ + public static final String INCLUDED_BY = "includedBy"; /** * A list of suppression rules. */ @@ -197,6 +201,9 @@ public void endElement(String uri, String localName, String qName) throws SAXExc final float cvss = Float.parseFloat(currentText.toString().trim()); rule.addCvssBelow(cvss); break; + case INCLUDED_BY: + rule.addIncludedBy(currentText.toString().trim()); + break; default: break; } diff --git a/core/src/main/java/org/owasp/dependencycheck/xml/suppression/SuppressionRule.java b/core/src/main/java/org/owasp/dependencycheck/xml/suppression/SuppressionRule.java index 3fd508ab05e..6f50f47cf94 100644 --- a/core/src/main/java/org/owasp/dependencycheck/xml/suppression/SuppressionRule.java +++ b/core/src/main/java/org/owasp/dependencycheck/xml/suppression/SuppressionRule.java @@ -26,6 +26,7 @@ import javax.annotation.concurrent.NotThreadSafe; import org.apache.commons.lang3.time.DateFormatUtils; import org.owasp.dependencycheck.dependency.Dependency; +import org.owasp.dependencycheck.dependency.IncludedByReference; import org.owasp.dependencycheck.dependency.Vulnerability; import org.owasp.dependencycheck.dependency.naming.CpeIdentifier; import org.owasp.dependencycheck.dependency.naming.Identifier; @@ -71,6 +72,13 @@ public class SuppressionRule { * The list of CVE entries to suppress. */ private List cve = new ArrayList<>(); + /** + * The list of parent dependencies to suppress this warning in + * + * Vulnerabilities are suppressed when all values in their 'includedBy' list + * are found in the suppression. + */ + private List includedBy = new ArrayList<>(); /** * The list of vulnerability name entries to suppress. */ @@ -297,6 +305,42 @@ public boolean hasNotes() { return !cve.isEmpty(); } + /** + * Get the value of includedBy. + * + * @return the value of includedBy + */ + public List getIncludedBy() { + return includedBy; + } + + /** + * Set the value of includedBy. + * + * @param includedBy new value of includedBy + */ + public void setIncludedBy(List includedBy) { + this.includedBy = includedBy; + } + + /** + * Adds the dependency to the includedBy list. + * + * @param includedBy the dependency to add + */ + public void addIncludedBy(String includedBy) { + this.includedBy.add(includedBy); + } + + /** + * Returns whether this suppression rule has includedBy entries. + * + * @return whether this suppression rule has includedBy entries + */ + public boolean hasIncludedBy() { + return !includedBy.isEmpty(); + } + /** * Get the value of CWE. * @@ -503,7 +547,7 @@ public void process(Dependency dependency) { } removalList.forEach(dependency::removeVulnerableSoftwareIdentifier); } - if (hasCve() || hasVulnerabilityName() || hasCwe() || hasCvssBelow()) { + if (hasCve() || hasVulnerabilityName() || hasCwe() || hasCvssBelow() || hasIncludedBy()) { final Set removeVulns = new HashSet<>(); for (Vulnerability v : dependency.getVulnerabilities()) { boolean remove = false; @@ -524,6 +568,24 @@ public void process(Dependency dependency) { } } } + if (!remove && this.hasIncludedBy() && !dependency.getIncludedBy().isEmpty()) { + boolean allExcluded = true; + for (IncludedByReference seen : dependency.getIncludedBy()) { + boolean seenExcluded = false; + for (String excluded : this.includedBy) { + if (seen.getReference().equalsIgnoreCase(excluded)) { + seenExcluded = true; + } + } + if (!seenExcluded) { + allExcluded = false; + } + } + if (allExcluded) { + remove = true; + removeVulns.add(v); + } + } if (!remove && v.getName() != null) { for (PropertyType entry : this.vulnerabilityNames) { if (entry.matches(v.getName())) { diff --git a/core/src/main/resources/schema/suppression.xsd b/core/src/main/resources/schema/suppression.xsd index 5a5b483abd2..bdd9d339eaa 100644 --- a/core/src/main/resources/schema/suppression.xsd +++ b/core/src/main/resources/schema/suppression.xsd @@ -48,6 +48,7 @@ + @@ -56,4 +57,4 @@ - \ No newline at end of file + diff --git a/core/src/test/java/org/owasp/dependencycheck/xml/suppression/SuppressionRuleTest.java b/core/src/test/java/org/owasp/dependencycheck/xml/suppression/SuppressionRuleTest.java index e23822ae84c..cad1c6c3fd4 100644 --- a/core/src/test/java/org/owasp/dependencycheck/xml/suppression/SuppressionRuleTest.java +++ b/core/src/test/java/org/owasp/dependencycheck/xml/suppression/SuppressionRuleTest.java @@ -85,6 +85,22 @@ public void testCpe() { } + /** + * Test of IncludedBy property, of class SuppressionRule. + */ + @Test + public void testGetIncludedBy() { + SuppressionRule instance = new SuppressionRule(); + List includedBy = new ArrayList<>(); + instance.setIncludedBy(includedBy); + assertFalse(instance.hasIncludedBy()); + instance.addIncludedBy("pkg:maven/com.github.spotbugs/spotbugs@4.7.3"); + assertTrue(instance.hasIncludedBy()); + List result = instance.getIncludedBy(); + assertEquals(includedBy, result); + } + + /** * Test of CvssBelow property, of class SuppressionRule. */ @@ -130,6 +146,17 @@ public void testCve() { assertEquals(cve, result); } + public void testIncludedBy() { + SuppressionRule instance = new SuppressionRule(); + List includedBy = new ArrayList<>(); + instance.setIncludedBy(includedBy); + assertFalse(instance.hasIncludedBy()); + instance.addIncludedBy("pkg:maven/com.github.spotbugs/spotbugs@4.7.3"); + assertTrue(instance.hasIncludedBy()); + List result = instance.getIncludedBy(); + assertEquals(includedBy, result); + } + /** * Test of base property, of class SuppressionRule. */ @@ -418,6 +445,8 @@ public void testProcess() throws CpeValidationException { dependency.addVulnerableSoftwareIdentifier(cpeId); String sha1 = dependency.getSha1sum(); dependency.setSha1sum("384FAA82E193D4E4B0546059CA09572654BC3970"); + dependency.addIncludedBy("pkg:maven/com.github.spotbugs/spotbugs@4.7.3"); + dependency.addIncludedBy("pkg:maven/org.apache.kafka/mirror@3.7.0-SNAPSHOT"); Vulnerability v = createVulnerability(); dependency.addVulnerability(v); @@ -454,6 +483,17 @@ public void testProcess() throws CpeValidationException { assertTrue(dependency.getVulnerabilities().isEmpty()); assertEquals(1, dependency.getSuppressedVulnerabilities().size()); + //includedBy + dependency.addVulnerability(v); + instance = new SuppressionRule(); + instance.addIncludedBy("pkg:maven/com.github.spotbugs/spotbugs@4.7.3"); + instance.process(dependency); + assertEquals(1, dependency.getVulnerabilities().size()); + instance.addIncludedBy("pkg:maven/org.apache.kafka/mirror@3.7.0-SNAPSHOT"); + instance.process(dependency); + assertTrue(dependency.getVulnerabilities().isEmpty()); + assertEquals(1, dependency.getSuppressedVulnerabilities().size()); + //cpe instance = new SuppressionRule(); PropertyType pt = new PropertyType();