added support for suppression by GAV (issue #124), created base suppression.xml (issue #123), and fixed false positives related to spring security (issue #130)

Former-commit-id: 330134211d022fec336dc1ca39205a94a088ee84

dependency-check-core/src/main/java/org/owasp/dependencycheck/analyzer/AbstractSuppressionAnalyzer.java

@@ -98,11 +98,18 @@ public abstract class AbstractSuppressionAnalyzer extends AbstractAnalyzer {
      * @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);
@@ -132,9 +139,9 @@ public abstract class AbstractSuppressionAnalyzer extends AbstractAnalyzer {
             }
 
             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());

dependency-check-core/src/main/java/org/owasp/dependencycheck/suppression/SuppressionHandler.java

@@ -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.
      */
@@ -95,13 +99,10 @@ public class SuppressionHandler extends DefaultHandler {
      */
     @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;
         }
     }
 
@@ -123,6 +124,9 @@ public class SuppressionHandler extends DefaultHandler {
             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);

dependency-check-core/src/main/java/org/owasp/dependencycheck/suppression/SuppressionRule.java

@@ -20,6 +20,7 @@ package org.owasp.dependencycheck.suppression;
 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;
@@ -30,6 +31,10 @@ import org.owasp.dependencycheck.dependency.Vulnerability;
  */
 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.
      */
@@ -280,16 +285,19 @@ public class SuppressionRule {
             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;
             }
         }
@@ -298,8 +306,17 @@ public class SuppressionRule {
             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;
@@ -355,7 +372,7 @@ public class SuppressionRule {
     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;
         }
@@ -390,20 +407,62 @@ public class SuppressionRule {
         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 identifier.getValue().startsWith(suppressionEntry.getValue());
-                        return true;
-                    }
                 } else {
                     final String id = identifier.getValue().toLowerCase();
                     final String check = suppressionEntry.getValue().toLowerCase();
-                    if (id.startsWith(check)) {
+                    return id.startsWith(check);
-                        return true;
-                    }
                 }
             }
         }
         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();
+    }
+
 }

dependency-check-core/src/main/resources/dependencycheck-base-suppression.xml

@@ -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>

dependency-check-core/src/test/java/org/owasp/dependencycheck/analyzer/AbstractSuppressionAnalyzerTest.java

@@ -34,8 +34,8 @@ import java.util.Set;
 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>
@@ -67,7 +67,7 @@ public class AbstractSuppressionAnalyzerTest extends BaseTest {
         instance.initialize();
         int expCount = 5;
         List<SuppressionRule> result = instance.getRules();
-        assertEquals(expCount, result.size());
+        assertTrue(expCount <= result.size());
     }
 
     /**
@@ -79,7 +79,7 @@ public class AbstractSuppressionAnalyzerTest extends BaseTest {
         instance.initialize();
         int expCount = 5;
         List<SuppressionRule> result = instance.getRules();
-        assertEquals(expCount, result.size());
+        assertTrue(expCount <= result.size());
     }
 
     @Test(expected = SuppressionParseException.class)

dependency-check-core/src/test/java/org/owasp/dependencycheck/suppression/SuppressionRuleTest.java

@@ -343,51 +343,66 @@ public class SuppressionRuleTest {
      */
     @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);
     }
 
@@ -467,6 +482,43 @@ public class SuppressionRuleTest {
         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");

src/site/markdown/suppression.md

@@ -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>
 ```