Skip to content

Commit

Permalink
added support for suppression by GAV (issue #124), created base suppr…
Browse files Browse the repository at this point in the history 
…ession.xml (issue #123), and fixed false positives related to spring security (issue #130)
  • Loading branch information
@jeremylong
jeremylong committed Jun 22, 2014
1 parent a27e9b8 commit e78ac09
Show file tree
Hide file tree
Showing 7 changed files with 172 additions and 29 deletions.
13 changes: 10 additions & 3 deletions ...ck-core/src/main/java/org/owasp/dependencycheck/analyzer/AbstractSuppressionAnalyzer.java
View file
Expand Up @@ -98,11 +98,18 @@ public void setRules(List<SuppressionRule> rules) {
* @throws SuppressionParseException thrown if the XML cannot be parsed.
*/
private void loadSuppressionData() throws SuppressionParseException {
final SuppressionParser parser = new SuppressionParser();
File file = null;
file = new File(this.getClass().getClassLoader().getResource("dependencycheck-base-suppression.xml").getPath());
try {
rules = parser.parseSuppressionRules(file);
} catch (SuppressionParseException ex) {
LOGGER.log(Level.FINE, "Unable to parse the base suppression data file", ex);
}
final String suppressionFilePath = Settings.getString(Settings.KEYS.SUPPRESSION_FILE);
if (suppressionFilePath == null) {
return;
}
File file = null;
boolean deleteTempFile = false;
try {
final Pattern uriRx = Pattern.compile("^(https?|file)\\:.*", Pattern.CASE_INSENSITIVE);
Expand Down Expand Up @@ -132,9 +139,9 @@ private void loadSuppressionData() throws SuppressionParseException {
}

if (file != null) {
final SuppressionParser parser = new SuppressionParser();
try {
rules = parser.parseSuppressionRules(file);
//rules = parser.parseSuppressionRules(file);
rules.addAll(parser.parseSuppressionRules(file));
LOGGER.log(Level.FINE, rules.size() + " suppression rules were loaded.");
} catch (SuppressionParseException ex) {
final String msg = String.format("Unable to parse suppression xml file '%s'", file.getPath());
Expand Down
12 changes: 8 additions & 4 deletions ...cy-check-core/src/main/java/org/owasp/dependencycheck/suppression/SuppressionHandler.java
View file
Expand Up @@ -54,6 +54,10 @@ public class SuppressionHandler extends DefaultHandler {
* The CWE element name.
*/
public static final String CWE = "cwe";
/**
* The GAV element name.
*/
public static final String GAV = "gav";
/**
* The cvssBelow element name.
*/
Expand Down Expand Up @@ -95,13 +99,10 @@ public List<SuppressionRule> getSuppressionRules() {
*/
@Override
public void startElement(String uri, String localName, String qName, Attributes attributes) throws SAXException {
currentAttributes = null;
currentAttributes = attributes;
currentText = new StringBuffer();

if (SUPPRESS.equals(qName)) {
rule = new SuppressionRule();
} else if (FILE_PATH.equals(qName)) {
currentAttributes = attributes;
}
}

Expand All @@ -123,6 +124,9 @@ public void endElement(String uri, String localName, String qName) throws SAXExc
rule.setFilePath(pt);
} else if (SHA1.equals(qName)) {
rule.setSha1(currentText.toString());
} else if (GAV.equals(qName)) {
final PropertyType pt = processPropertyType();
rule.setGav(pt);
} else if (CPE.equals(qName)) {
final PropertyType pt = processPropertyType();
rule.addCpe(pt);
Expand Down
81 changes: 70 additions & 11 deletions ...dency-check-core/src/main/java/org/owasp/dependencycheck/suppression/SuppressionRule.java
View file
Expand Up @@ -20,6 +20,7 @@
import java.util.ArrayList;
import java.util.Iterator;
import java.util.List;
import java.util.logging.Logger;
import org.owasp.dependencycheck.dependency.Dependency;
import org.owasp.dependencycheck.dependency.Identifier;
import org.owasp.dependencycheck.dependency.Vulnerability;
Expand All @@ -30,6 +31,10 @@
*/
public class SuppressionRule {

/**
* The Logger for use throughout the class
*/
private static final Logger LOGGER = Logger.getLogger(SuppressionRule.class.getName());
/**
* The file path for the suppression.
*/
Expand Down Expand Up @@ -280,16 +285,19 @@ public void process(Dependency dependency) {
return;
}
if (gav != null) {
LOGGER.info(this.toString());
final Iterator<Identifier> itr = dependency.getIdentifiers().iterator();
boolean hasMatch = false;
boolean gavFound = false;
while (itr.hasNext()) {
final Identifier i = itr.next();
LOGGER.info(String.format("%nChecking %s for gav:%s", i.getValue(), this.gav));
if (identifierMatches("maven", this.gav, i)) {
hasMatch = true;
LOGGER.info("GAV Matched!");
gavFound = true;
break;
}
}
if (!hasMatch) {
if (!gavFound) {
return;
}
}
Expand All @@ -298,8 +306,17 @@ public void process(Dependency dependency) {
final Iterator<Identifier> itr = dependency.getIdentifiers().iterator();
while (itr.hasNext()) {
final Identifier i = itr.next();
if (this.gav != null) {
LOGGER.info(String.format("%nProcessesing %s", i.getValue()));
}
for (PropertyType c : this.cpe) {
if (this.gav != null) {
LOGGER.info(String.format("%nChecking %s for cpe:%s", i.getValue(), c.getValue()));
}
if (identifierMatches("cpe", c, i)) {
if (this.gav != null) {
LOGGER.info(String.format("%nRemoving %s", i.getValue()));
}
dependency.addSuppressedIdentifier(i);
itr.remove();
break;
Expand Down Expand Up @@ -355,7 +372,7 @@ public void process(Dependency dependency) {
boolean cpeHasNoVersion(PropertyType c) {
if (c.isRegex()) {
return false;
} // cpe:/a:jboss:jboss:1.0.0:
} // cpe:/a:jboss:jboss:1.0.0
if (countCharacter(c.getValue(), ':') == 3) {
return true;
}
Expand Down Expand Up @@ -390,20 +407,62 @@ boolean identifierMatches(String identifierType, PropertyType suppressionEntry,
if (identifierType.equals(identifier.getType())) {
if (suppressionEntry.matches(identifier.getValue())) {
return true;
} else if (cpeHasNoVersion(suppressionEntry)) {
} else if ("cpe".equals(identifierType) && cpeHasNoVersion(suppressionEntry)) {
if (suppressionEntry.isCaseSensitive()) {
if (identifier.getValue().startsWith(suppressionEntry.getValue())) {
return true;
}
return identifier.getValue().startsWith(suppressionEntry.getValue());
} else {
final String id = identifier.getValue().toLowerCase();
final String check = suppressionEntry.getValue().toLowerCase();
if (id.startsWith(check)) {
return true;
}
return id.startsWith(check);
}
}
}
return false;
}

@Override
public String toString() {
StringBuilder sb = new StringBuilder();
sb.append("SuppressionRule{");
if (filePath != null) {
sb.append("filePath=").append(filePath).append(",");
}
if (sha1 != null) {
sb.append("sha1=").append(sha1).append(",");
}
if (gav != null) {
sb.append("gav=").append(gav).append(",");
}
if (cpe != null && cpe.size() > 0) {
sb.append("cpe={");
for (PropertyType pt : cpe) {
sb.append(pt).append(",");
}
sb.append("}");
}
if (cwe != null && cwe.size() > 0) {
sb.append("cwe={");
for (String s : cwe) {
sb.append(s).append(",");
}
sb.append("}");
}
if (cve != null && cve.size() > 0) {
sb.append("cve={");
for (String s : cve) {
sb.append(s).append(",");
}
sb.append("}");
}
if (cvssBelow != null && cvssBelow.size() > 0) {
sb.append("cvssBelow={");
for (Float s : cvssBelow) {
sb.append(s).append(",");
}
sb.append("}");
}
sb.append("}");
return sb.toString();
}

}
12 changes: 12 additions & 0 deletions dependency-check-core/src/main/resources/dependencycheck-base-suppression.xml
View file
@@ -0,0 +1,12 @@
<?xml version="1.0" encoding="UTF-8"?>
<suppressions xmlns="https://www.owasp.org/index.php/OWASP_Dependency_Check_Suppression">
<suppress>
<notes><![CDATA[
This suppresses false positives identified on spring security.
]]></notes>
<gav regex="true">org\.springframework\.security:spring.*</gav>
<cpe>cpe:/a:mod_security:mod_security</cpe>
<cpe>cpe:/a:springsource:spring_framework</cpe>
<cpe>cpe:/a:vmware:springsource_spring_framework</cpe>
</suppress>
</suppressions>
6 changes: 3 additions & 3 deletions ...ore/src/test/java/org/owasp/dependencycheck/analyzer/AbstractSuppressionAnalyzerTest.java
View file
Expand Up @@ -34,8 +34,8 @@
import java.util.logging.Level;
import java.util.logging.Logger;

import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertNull;
import static org.junit.Assert.assertTrue;

/**
* @author Jeremy Long <jeremy.long@owasp.org>
Expand Down Expand Up @@ -67,7 +67,7 @@ public void testGetRulesFromSuppressionFileFromURL() throws Exception {
instance.initialize();
int expCount = 5;
List<SuppressionRule> result = instance.getRules();
assertEquals(expCount, result.size());
assertTrue(expCount <= result.size());
}

/**
Expand All @@ -79,7 +79,7 @@ public void testGetRulesFromSuppressionFileInClasspath() throws Exception {
instance.initialize();
int expCount = 5;
List<SuppressionRule> result = instance.getRules();
assertEquals(expCount, result.size());
assertTrue(expCount <= result.size());
}

@Test(expected = SuppressionParseException.class)
Expand Down
68 changes: 60 additions & 8 deletions ...y-check-core/src/test/java/org/owasp/dependencycheck/suppression/SuppressionRuleTest.java
View file
Expand Up @@ -343,51 +343,66 @@ public void testCountCharacter() {
*/
@Test
public void testCpeMatches() {
Identifier identifier = new Identifier("cwe", "cpe:/a:microsoft:.net_framework:4.5", "some url not needed for this test");
Identifier identifier = new Identifier("cpe", "cpe:/a:microsoft:.net_framework:4.5", "some url not needed for this test");

PropertyType cpe = new PropertyType();
cpe.setValue("cpe:/a:microsoft:.net_framework:4.5");

SuppressionRule instance = new SuppressionRule();
boolean expResult = true;
boolean result = instance.identifierMatches(cpe, identifier);
boolean result = instance.identifierMatches("cpe", cpe, identifier);
assertEquals(expResult, result);

cpe.setValue("cpe:/a:microsoft:.net_framework:4.0");
expResult = false;
result = instance.identifierMatches(cpe, identifier);
result = instance.identifierMatches("cpe", cpe, identifier);
assertEquals(expResult, result);

cpe.setValue("CPE:/a:microsoft:.net_framework:4.5");
cpe.setCaseSensitive(true);
expResult = false;
result = instance.identifierMatches(cpe, identifier);
result = instance.identifierMatches("cpe", cpe, identifier);
assertEquals(expResult, result);

cpe.setValue("cpe:/a:microsoft:.net_framework");
cpe.setCaseSensitive(false);
expResult = true;
result = instance.identifierMatches(cpe, identifier);
result = instance.identifierMatches("cpe", cpe, identifier);
assertEquals(expResult, result);

cpe.setValue("cpe:/a:microsoft:.*");
cpe.setRegex(true);
expResult = true;
result = instance.identifierMatches(cpe, identifier);
result = instance.identifierMatches("cpe", cpe, identifier);
assertEquals(expResult, result);

cpe.setValue("CPE:/a:microsoft:.*");
cpe.setRegex(true);
cpe.setCaseSensitive(true);
expResult = false;
result = instance.identifierMatches(cpe, identifier);
result = instance.identifierMatches("cpe", cpe, identifier);
assertEquals(expResult, result);

cpe.setValue("cpe:/a:apache:.*");
cpe.setRegex(true);
cpe.setCaseSensitive(false);
expResult = false;
result = instance.identifierMatches(cpe, identifier);
result = instance.identifierMatches("cpe", cpe, identifier);
assertEquals(expResult, result);

identifier = new Identifier("maven", "org.springframework:spring-core:2.5.5", "https://repository.sonatype.org/service/local/artifact/maven/redirect?r=central-proxy&g=org.springframework&a=spring-core&v=2.5.5&e=jar");
cpe.setValue("org.springframework:spring-core:2.5.5");
cpe.setRegex(false);
cpe.setCaseSensitive(false);
expResult = true;
result = instance.identifierMatches("maven", cpe, identifier);
assertEquals(expResult, result);

cpe.setValue("org\\.springframework\\.security:spring.*");
cpe.setRegex(true);
cpe.setCaseSensitive(false);
expResult = false;
result = instance.identifierMatches("maven", cpe, identifier);
assertEquals(expResult, result);
}

Expand Down Expand Up @@ -467,6 +482,43 @@ public void testProcess() {
assertTrue(dependency.getSuppressedIdentifiers().size() == 3);
}

/**
* Test of process method, of class SuppressionRule.
*/
@Test
public void testProcessGAV() {
File spring = new File(this.getClass().getClassLoader().getResource("spring-security-web-3.0.0.RELEASE.jar").getPath());
Dependency dependency = new Dependency(spring);
dependency.addIdentifier("cpe", "cpe:/a:vmware:springsource_spring_framework:3.0.0", "some url not needed for this test");
dependency.addIdentifier("cpe", "cpe:/a:springsource:spring_framework:3.0.0", "some url not needed for this test");
dependency.addIdentifier("cpe", "cpe:/a:mod_security:mod_security:3.0.0", "some url not needed for this test");
dependency.addIdentifier("cpe", "cpe:/a:vmware:springsource_spring_security:3.0.0", "some url not needed for this test");
dependency.addIdentifier("maven", "org.springframework.security:spring-security-web:3.0.0.RELEASE", "some url not needed for this test");

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

pt.setValue("org\\.springframework\\.security:spring.*");
pt.setRegex(true);
pt.setCaseSensitive(false);
instance.setGav(pt);

pt = new PropertyType();
pt.setValue("cpe:/a:mod_security:mod_security");
instance.addCpe(pt);
pt = new PropertyType();
pt.setValue("cpe:/a:springsource:spring_framework");
instance.addCpe(pt);
pt = new PropertyType();
pt.setValue("cpe:/a:vmware:springsource_spring_framework");
instance.addCpe(pt);

instance.process(dependency);
assertEquals(2, dependency.getIdentifiers().size());

}

private Vulnerability createVulnerability() {
Vulnerability v = new Vulnerability();
v.setCwe("CWE-287 Improper Authentication");
Expand Down
9 changes: 9 additions & 0 deletions src/site/markdown/suppression.md
View file
Expand Up @@ -64,6 +64,15 @@ HTML version of the report. The other common scenario would be to ignore all CVE
]]></notes>
<cvssBelow>7</cvssBelow>
</suppress>
<suppress>
<notes><![CDATA[
This suppresses false positives identified on spring security.
]]></notes>
<gav regex="true">org\.springframework\.security:spring.*</gav>
<cpe>cpe:/a:vmware:springsource_spring_framework</cpe>
<cpe>cpe:/a:springsource:spring_framework</cpe>
<cpe>cpe:/a:mod_security:mod_security</cpe>
</suppress>
</suppressions>
```

Expand Down

0 comments on commit e78ac09

Please sign in to comment.