Skip to content

Commit

Permalink
feat: suppress by 'includedBy'
Browse files Browse the repository at this point in the history
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
  • Loading branch information
raboof committed Oct 3, 2023
1 parent 24f4ddf commit 4f91689
Show file tree
Hide file tree
Showing 4 changed files with 112 additions and 2 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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.
*/
Expand Down Expand Up @@ -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;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -71,6 +72,13 @@ public class SuppressionRule {
* The list of CVE entries to suppress.
*/
private List<String> 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<String> includedBy = new ArrayList<>();
/**
* The list of vulnerability name entries to suppress.
*/
Expand Down Expand Up @@ -297,6 +305,42 @@ public boolean hasNotes() {
return !cve.isEmpty();
}

/**
* Get the value of includedBy.
*
* @return the value of includedBy
*/
public List<String> getIncludedBy() {
return includedBy;
}

/**
* Set the value of includedBy.
*
* @param includedBy new value of includedBy
*/
public void setIncludedBy(List<String> 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.
*
Expand Down Expand Up @@ -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<Vulnerability> removeVulns = new HashSet<>();
for (Vulnerability v : dependency.getVulnerabilities()) {
boolean remove = false;
Expand All @@ -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())) {
Expand Down
3 changes: 2 additions & 1 deletion core/src/main/resources/schema/suppression.xsd
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@
<xs:element name="cve" type="dc:cveType"/>
<xs:element name="cwe" type="xs:positiveInteger"/>
<xs:element name="cvssBelow" type="dc:cvssScoreType"/>
<xs:element name="includedBy" type="xs:string"/>
</xs:choice>
</xs:sequence>
<xs:attribute name="base" use="optional" type="xs:boolean" default="false"/>
Expand All @@ -56,4 +57,4 @@
</xs:sequence>
</xs:complexType>
</xs:element>
</xs:schema>
</xs:schema>
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,22 @@ public void testCpe() {

}

/**
* Test of IncludedBy property, of class SuppressionRule.
*/
@Test
public void testGetIncludedBy() {
SuppressionRule instance = new SuppressionRule();
List<String> includedBy = new ArrayList<>();
instance.setIncludedBy(includedBy);
assertFalse(instance.hasIncludedBy());
instance.addIncludedBy("pkg:maven/com.github.spotbugs/[email protected]");
assertTrue(instance.hasIncludedBy());
List<String> result = instance.getIncludedBy();
assertEquals(includedBy, result);
}


/**
* Test of CvssBelow property, of class SuppressionRule.
*/
Expand Down Expand Up @@ -130,6 +146,17 @@ public void testCve() {
assertEquals(cve, result);
}

public void testIncludedBy() {
SuppressionRule instance = new SuppressionRule();
List<String> includedBy = new ArrayList<>();
instance.setIncludedBy(includedBy);
assertFalse(instance.hasIncludedBy());
instance.addIncludedBy("pkg:maven/com.github.spotbugs/[email protected]");
assertTrue(instance.hasIncludedBy());
List<String> result = instance.getIncludedBy();
assertEquals(includedBy, result);
}

/**
* Test of base property, of class SuppressionRule.
*/
Expand Down Expand Up @@ -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/[email protected]");
dependency.addIncludedBy("pkg:maven/org.apache.kafka/[email protected]");
Vulnerability v = createVulnerability();
dependency.addVulnerability(v);

Expand Down Expand Up @@ -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/[email protected]");
instance.process(dependency);
assertEquals(1, dependency.getVulnerabilities().size());
instance.addIncludedBy("pkg:maven/org.apache.kafka/[email protected]");
instance.process(dependency);
assertTrue(dependency.getVulnerabilities().isEmpty());
assertEquals(1, dependency.getSuppressedVulnerabilities().size());

//cpe
instance = new SuppressionRule();
PropertyType pt = new PropertyType();
Expand Down

0 comments on commit 4f91689

Please sign in to comment.