Log when non-base suppressions rules are unused

ant/src/test/java/org/owasp/dependencycheck/taskdefs/DependencyCheckTaskIT.java

@@ -29,6 +29,7 @@
 import org.owasp.dependencycheck.BaseDBTestCase;
 
 import static org.junit.Assert.assertTrue;
+import org.owasp.dependencycheck.xml.suppression.SuppressionRules;
 
 /**
  *
@@ -131,8 +132,8 @@ public void testNestedBADReportFormat() throws Exception {
     public void testGetFailBuildOnCVSS() {
         Exception exception = Assert.assertThrows(BuildException.class, () -> buildFileRule.executeTarget("failCVSS"));
 
-        String expectedMessage = String.format("One or more dependencies were identified with vulnerabilities that " +
-                "have a CVSS score greater than or equal to '%.1f':", 3.0f);
+        String expectedMessage = String.format("One or more dependencies were identified with vulnerabilities that "
+                + "have a CVSS score greater than or equal to '%.1f':", 3.0f);
 
         Assert.assertTrue(exception.getMessage().contains(expectedMessage));
     }
@@ -144,7 +145,8 @@ public void testGetFailBuildOnCVSS() {
     public void testSuppressingCVE() {
         // GIVEN an ant task with a vulnerability
         final String antTaskName = "suppression";
-
+        //as the suppression rules are now a singleton - we must reset the list to cause the new suppression rules to load
+        SuppressionRules.getInstance().list().clear();
         // WHEN executing the ant task
         buildFileRule.executeTarget(antTaskName);
         if (buildFileRule.getError() != null && !buildFileRule.getError().isEmpty()) {
@@ -168,7 +170,8 @@ public void testSuppressingCVE() {
     public void testSuppressingSingle() {
         // GIVEN an ant task with a vulnerability using the legacy property
         final String antTaskName = "suppression-single";
-
+        //as the suppression rules are now a singleton - we must reset the list to cause the new suppression rules to load
+        SuppressionRules.getInstance().list().clear();
         // WHEN executing the ant task
         buildFileRule.executeTarget(antTaskName);
 
@@ -185,7 +188,8 @@ public void testSuppressingSingle() {
     public void testSuppressingMultiple() {
         // GIVEN an ant task with a vulnerability using multiple was to configure the suppression file
         final String antTaskName = "suppression-multiple";
-
+        //as the suppression rules are now a singleton - we must reset the list to cause the new suppression rules to load
+        SuppressionRules.getInstance().list().clear();
         // WHEN executing the ant task
         buildFileRule.executeTarget(antTaskName);
 

core/src/main/java/org/owasp/dependencycheck/Engine.java

@@ -82,6 +82,7 @@
 import static org.owasp.dependencycheck.analyzer.AnalysisPhase.PRE_IDENTIFIER_ANALYSIS;
 import static org.owasp.dependencycheck.analyzer.AnalysisPhase.PRE_INFORMATION_COLLECTION;
 import org.owasp.dependencycheck.analyzer.DependencyBundlingAnalyzer;
+import org.owasp.dependencycheck.xml.suppression.SuppressionRules;
 
 /**
  * Scans files, directories, etc. for Dependencies. Analyzers are loaded and
@@ -659,6 +660,8 @@ public void analyzeDependencies() throws ExceptionCollection {
                 .map(analyzers::get)
                 .forEach((analyzerList) -> analyzerList.forEach(this::closeAnalyzer));
 
+        SuppressionRules.getInstance().logUnusedRules();
+
         LOGGER.debug("\n----------------------------------------------------\nEND ANALYSIS\n----------------------------------------------------");
         final long analysisDurationSeconds = TimeUnit.MILLISECONDS.toSeconds(System.currentTimeMillis() - analysisStart);
         LOGGER.info("Analysis Complete ({} seconds)", analysisDurationSeconds);

core/src/main/java/org/owasp/dependencycheck/analyzer/AbstractSuppressionAnalyzer.java

@@ -43,7 +43,7 @@
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 import org.xml.sax.SAXException;
-import org.owasp.dependencycheck.xml.suppression.SuppressionRuleFilter;
+import org.owasp.dependencycheck.xml.suppression.SuppressionRules;
 
 /**
  * Abstract base suppression analyzer that contains methods for parsing the
@@ -52,7 +52,7 @@
  * @author Jeremy Long
  */
 @ThreadSafe
-public abstract class AbstractSuppressionAnalyzer extends AbstractAnalyzer implements SuppressionRuleFilter {
+public abstract class AbstractSuppressionAnalyzer extends AbstractAnalyzer {
 
     /**
      * The Logger for use throughout the class.
@@ -63,9 +63,18 @@ public abstract class AbstractSuppressionAnalyzer extends AbstractAnalyzer imple
      */
     private static final String BASE_SUPPRESSION_FILE = "dependencycheck-base-suppression.xml";
     /**
-     * The list of suppression rules.
+     * The collection of suppression rules.
      */
-    private final List<SuppressionRule> rules = new ArrayList<>();
+    private final SuppressionRules rules = SuppressionRules.getInstance();
+
+    /**
+     * Returns the suppression rules.
+     *
+     * @return the suppression rules
+     */
+    protected SuppressionRules getSuppressionRules() {
+        return rules;
+    }
 
     /**
      * Get the number of suppression rules.
@@ -114,9 +123,22 @@ protected void analyzeDependency(Dependency dependency, Engine engine) throws An
         if (rules.isEmpty()) {
             return;
         }
-        rules.forEach((rule) -> rule.process(dependency));
+        for (SuppressionRule rule : rules.list()) {
+            if (filter(rule)) {
+                rule.process(dependency);
+            }
+        }
     }
 
+    /**
+     * Determines whether a suppression rule should be retained when filtering a set of suppression rules for a concrete suppression analyzer.
+     *
+     * @param rule the suppression rule to evaluate
+     * @return <code>true</code> if the rule should be retained; otherwise
+     * <code>false</code>
+     */
+    abstract boolean filter(SuppressionRule rule);
+
     /**
      * Loads all the suppression rules files configured in the {@link Settings}.
      *
@@ -160,7 +182,7 @@ private void loadSuppressionBaseData() throws SuppressionParseException {
             if (in == null) {
                 throw new SuppressionParseException("Suppression rules `" + BASE_SUPPRESSION_FILE + "` could not be found");
             }
-            ruleList = parser.parseSuppressionRules(in, this);
+            ruleList = parser.parseSuppressionRules(in);
         } catch (SAXException | IOException ex) {
             throw new SuppressionParseException("Unable to parse the base suppression data file", ex);
         }
@@ -235,7 +257,7 @@ private List<SuppressionRule> loadSuppressionFile(final SuppressionParser parser
                     throw new SuppressionParseException(msg);
                 }
                 try {
-                    list.addAll(parser.parseSuppressionRules(file, this));
+                    list.addAll(parser.parseSuppressionRules(file));
                 } catch (SuppressionParseException ex) {
                     LOGGER.warn("Unable to parse suppression xml file '{}'", file.getPath());
                     LOGGER.warn(ex.getMessage());

core/src/main/java/org/owasp/dependencycheck/analyzer/CpeSuppressionAnalyzer.java

@@ -82,7 +82,7 @@ protected String getAnalyzerEnabledSettingKey() {
 
     @Override
     public boolean filter(SuppressionRule rule) {
-        return !rule.hasCpe();
+        return rule.hasCpe();
     }
 
     @Override

core/src/main/java/org/owasp/dependencycheck/analyzer/VulnerabilitySuppressionAnalyzer.java

@@ -76,7 +76,7 @@ protected String getAnalyzerEnabledSettingKey() {
 
     @Override
     public boolean filter(SuppressionRule rule) {
-        return !(rule.hasCve() || rule.hasCvssBelow() || rule.hasCwe() || rule.hasVulnerabilityName());
+        return rule.hasCve() || rule.hasCvssBelow() || rule.hasCwe() || rule.hasVulnerabilityName();
     }
 
     @Override

core/src/main/java/org/owasp/dependencycheck/xml/suppression/SuppressionHandler.java

@@ -104,10 +104,6 @@ public class SuppressionHandler extends DefaultHandler {
      * The current node text being extracted from the element.
      */
     private StringBuilder currentText;
-    /**
-     * The suppression rule filter.
-     */
-    private final SuppressionRuleFilter filter;
 
     /**
      * Get the value of suppressionRules.
@@ -118,17 +114,6 @@ public List<SuppressionRule> getSuppressionRules() {
         return suppressionRules;
     }
 
-    /**
-     * Constructs a Suppression Handler.
-     *
-     * @param filter The suppression rule filter used when loading the
-     * suppression rules. This is used to differentiate vulnerability
-     * suppression rules from CPE suppression rules.
-     */
-    public SuppressionHandler(SuppressionRuleFilter filter) {
-        this.filter = filter;
-    }
-
     /**
      * Handles the start element event.
      *
@@ -176,8 +161,6 @@ public void endElement(String uri, String localName, String qName) throws SAXExc
                 case SUPPRESS:
                     if (rule.getUntil() != null && rule.getUntil().before(Calendar.getInstance())) {
                         LOGGER.info("Suppression is expired for rule: {}", rule);
-                    } else if (filter != null && filter.filter(rule)) {
-                        LOGGER.debug("Filtering {} for {}", rule.toString(), filter.getName());
                     } else {
                         suppressionRules.add(rule);
                     }

core/src/main/java/org/owasp/dependencycheck/xml/suppression/SuppressionParser.java

@@ -77,14 +77,13 @@ public class SuppressionParser {
      * contained.
      *
      * @param file an XML file containing suppression rules
-     * @param filter the suppression rule filter
      * @return a list of suppression rules
      * @throws SuppressionParseException thrown if the XML file cannot be parsed
      */
     @SuppressFBWarnings(justification = "try with resource will clenaup the resources", value = {"OBL_UNSATISFIED_OBLIGATION"})
-    public List<SuppressionRule> parseSuppressionRules(File file, SuppressionRuleFilter filter) throws SuppressionParseException {
+    public List<SuppressionRule> parseSuppressionRules(File file) throws SuppressionParseException {
         try (FileInputStream fis = new FileInputStream(file)) {
-            return parseSuppressionRules(fis, filter);
+            return parseSuppressionRules(fis);
         } catch (SAXException | IOException ex) {
             LOGGER.debug("", ex);
             throw new SuppressionParseException(ex);
@@ -96,12 +95,11 @@ public List<SuppressionRule> parseSuppressionRules(File file, SuppressionRuleFil
      * contained.
      *
      * @param inputStream an InputStream containing suppression rules
-     * @param filter a filter to use when loading suppression rules
      * @return a list of suppression rules
      * @throws SuppressionParseException thrown if the XML cannot be parsed
      * @throws SAXException thrown if the XML cannot be parsed
      */
-    public List<SuppressionRule> parseSuppressionRules(InputStream inputStream, SuppressionRuleFilter filter)
+    public List<SuppressionRule> parseSuppressionRules(InputStream inputStream)
             throws SuppressionParseException, SAXException {
         try (
                 InputStream schemaStream13 = FileUtils.getResourceAsStream(SUPPRESSION_SCHEMA_1_3);
@@ -114,7 +112,7 @@ public List<SuppressionRule> parseSuppressionRules(InputStream inputStream, Supp
             final String defaultEncoding = StandardCharsets.UTF_8.name();
             final String charsetName = bom == null ? defaultEncoding : bom.getCharsetName();
 
-            final SuppressionHandler handler = new SuppressionHandler(filter);
+            final SuppressionHandler handler = new SuppressionHandler();
             final SAXParser saxParser = XmlUtils.buildSecureSaxParser(schemaStream13, schemaStream12, schemaStream11, schemaStream10);
             final XMLReader xmlReader = saxParser.getXMLReader();
             xmlReader.setErrorHandler(new SuppressionErrorHandler());

core/src/main/java/org/owasp/dependencycheck/xml/suppression/SuppressionRule.java

@@ -103,6 +103,29 @@ public class SuppressionRule {
      */
     private Calendar until;
 
+    /**
+     * A flag whether or not the rule matched a dependency & CPE.
+     */
+    private boolean matched = false;
+
+    /**
+     * Get the value of matched.
+     *
+     * @return the value of matched
+     */
+    public boolean isMatched() {
+        return matched;
+    }
+
+    /**
+     * Set the value of matched.
+     *
+     * @param matched new value of matched
+     */
+    public void setMatched(boolean matched) {
+        this.matched = matched;
+    }
+
     /**
      * Get the (@code{nullable}) value of until.
      *
@@ -467,6 +490,7 @@ public void process(Dependency dependency) {
                 for (PropertyType c : this.cpe) {
                     if (identifierMatches(c, i)) {
                         if (!isBase()) {
+                            matched = true;
                             if (this.notes != null) {
                                 i.setNotes(this.notes);
                             }
@@ -507,7 +531,6 @@ public void process(Dependency dependency) {
                             removeVulns.add(v);
                             break;
                         }
-
                     }
                 }
                 if (!remove) {
@@ -524,13 +547,12 @@ public void process(Dependency dependency) {
                         }
                     }
                 }
-                if (remove) {
-                    if (!isBase()) {
-                        if (this.notes != null) {
-                            v.setNotes(this.notes);
-                        }
-                        dependency.addSuppressedVulnerability(v);
+                if (remove && !isBase()) {
+                    matched = true;
+                    if (this.notes != null) {
+                        v.setNotes(this.notes);
                     }
+                    dependency.addSuppressedVulnerability(v);
                 }
             }
             removeVulns.forEach(dependency::removeVulnerability);
@@ -644,6 +666,9 @@ public String toString() {
         if (sha1 != null) {
             sb.append("sha1=").append(sha1).append(',');
         }
+        if (packageUrl != null) {
+            sb.append("packageUrl=").append(packageUrl).append(',');
+        }
         if (gav != null) {
             sb.append("gav=").append(gav).append(',');
         }