From edaba8f89bc371e3f4c0546cc43ab94fb2b6fb27 Mon Sep 17 00:00:00 2001 From: Jeremy Long Date: Tue, 18 Oct 2022 19:25:04 -0400 Subject: [PATCH] fix: refactor to prevent concurrent modification exception --- .../taskdefs/DependencyCheckTaskIT.java | 8 +- .../org/owasp/dependencycheck/Engine.java | 48 +++++++- .../analyzer/AbstractSuppressionAnalyzer.java | 106 ++++++++++-------- .../dependencycheck/analyzer/CPEAnalyzer.java | 37 +++--- .../UnusedSuppressionRuleAnalyzer.java | 92 +++++++++++++++ .../xml/suppression/SuppressionRules.java | 104 ----------------- ...rg.owasp.dependencycheck.analyzer.Analyzer | 3 +- .../AbstractSuppressionAnalyzerTest.java | 27 ++--- .../analyzer/CPEAnalyzerIT.java | 72 ++++++------ .../analyzer/CpeSuppressionAnalyzerIT.java | 21 ++-- .../VulnerabilitySuppressionAnalyzerIT.java | 23 ++-- .../reporting/ReportGeneratorIT.java | 14 +-- 12 files changed, 290 insertions(+), 265 deletions(-) create mode 100644 core/src/main/java/org/owasp/dependencycheck/analyzer/UnusedSuppressionRuleAnalyzer.java delete mode 100644 core/src/main/java/org/owasp/dependencycheck/xml/suppression/SuppressionRules.java diff --git a/ant/src/test/java/org/owasp/dependencycheck/taskdefs/DependencyCheckTaskIT.java b/ant/src/test/java/org/owasp/dependencycheck/taskdefs/DependencyCheckTaskIT.java index 4e65118ce0e..a020d72092e 100644 --- a/ant/src/test/java/org/owasp/dependencycheck/taskdefs/DependencyCheckTaskIT.java +++ b/ant/src/test/java/org/owasp/dependencycheck/taskdefs/DependencyCheckTaskIT.java @@ -29,7 +29,6 @@ import org.owasp.dependencycheck.BaseDBTestCase; import static org.junit.Assert.assertTrue; -import org.owasp.dependencycheck.xml.suppression.SuppressionRules; /** * @@ -145,8 +144,7 @@ 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()) { @@ -170,8 +168,6 @@ 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); @@ -188,8 +184,6 @@ 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); diff --git a/core/src/main/java/org/owasp/dependencycheck/Engine.java b/core/src/main/java/org/owasp/dependencycheck/Engine.java index 5fbf2ea0070..b3826d47a3c 100644 --- a/core/src/main/java/org/owasp/dependencycheck/Engine.java +++ b/core/src/main/java/org/owasp/dependencycheck/Engine.java @@ -55,6 +55,7 @@ import java.util.Collection; import java.util.Collections; import java.util.EnumMap; +import java.util.HashMap; import java.util.HashSet; import java.util.Iterator; import java.util.List; @@ -82,7 +83,6 @@ 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 @@ -125,6 +125,10 @@ public class Engine implements FileFilter, AutoCloseable { * The configured settings. */ private final Settings settings; + /** + * A storage location to persist objects throughout the execution of ODC. + */ + private final Map objects = new HashMap<>(); /** * The external view of the dependency list. */ @@ -660,8 +664,6 @@ 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); @@ -1068,6 +1070,46 @@ public Settings getSettings() { return settings; } + /** + * Retrieve an object from the objects collection. + * + * @param key the key to retrieve the object + * @return the object + */ + public Object getObject(String key) { + return objects.get(key); + } + + /** + * Put an object in the object collection. + * + * @param key the key to store the object + * @param object the object to store + */ + public void putObject(String key, Object object) { + objects.put(key, object); + } + + /** + * Verifies if the object exists in the object store. + * + * @param key the key to retrieve the object + * @return true if the object exists; otherwise + * false + */ + public boolean hasObject(String key) { + return objects.containsKey(key); + } + + /** + * Removes an object from the object store. + * + * @param key the key to the object + */ + public void removeObject(String key) { + objects.remove(key); + } + /** * Returns the mode of the engine. * diff --git a/core/src/main/java/org/owasp/dependencycheck/analyzer/AbstractSuppressionAnalyzer.java b/core/src/main/java/org/owasp/dependencycheck/analyzer/AbstractSuppressionAnalyzer.java index c32591fdae6..b4984fc2479 100644 --- a/core/src/main/java/org/owasp/dependencycheck/analyzer/AbstractSuppressionAnalyzer.java +++ b/core/src/main/java/org/owasp/dependencycheck/analyzer/AbstractSuppressionAnalyzer.java @@ -43,7 +43,6 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; import org.xml.sax.SAXException; -import org.owasp.dependencycheck.xml.suppression.SuppressionRules; /** * Abstract base suppression analyzer that contains methods for parsing the @@ -63,31 +62,9 @@ public abstract class AbstractSuppressionAnalyzer extends AbstractAnalyzer { */ private static final String BASE_SUPPRESSION_FILE = "dependencycheck-base-suppression.xml"; /** - * Settings flag to indicate if the suppression rules have been previously loaded. + * The key used to store and retrieve the suppression files. */ - private static final String SUPPRESSION_LOADED = "SUPPRESSION_LOADED"; - /** - * The collection of suppression rules. - */ - 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. - * - * @return the number of suppression rules - */ - protected int getRuleCount() { - return rules.size(); - } + public static final String SUPPRESSION_OBJECT_KEY = "suppression.rules"; /** * Returns a list of file EXTENSIONS supported by this analyzer. @@ -107,33 +84,33 @@ public Set getSupportedExtensions() { */ @Override public synchronized void prepareAnalyzer(Engine engine) throws InitializationException { - //check if we have a brand new settings object - if we do the suppression rules could be different - final boolean loaded = getSettings().getBoolean(SUPPRESSION_LOADED, false); - if (!loaded) { - SuppressionRules.getInstance().list().clear(); + if (engine.hasObject(SUPPRESSION_OBJECT_KEY)) { + return; + } + try { + loadSuppressionBaseData(engine); + } catch (SuppressionParseException ex) { + throw new InitializationException("Error initializing the suppression analyzer: " + ex.getLocalizedMessage(), ex, true); } - if (rules.isEmpty()) { - try { - loadSuppressionBaseData(); - } catch (SuppressionParseException ex) { - throw new InitializationException("Error initializing the suppression analyzer: " + ex.getLocalizedMessage(), ex, true); - } - try { - loadSuppressionData(); - } catch (SuppressionParseException ex) { - throw new InitializationException("Warn initializing the suppression analyzer: " + ex.getLocalizedMessage(), ex, false); - } - getSettings().setBoolean(SUPPRESSION_LOADED, true); + try { + loadSuppressionData(engine); + } catch (SuppressionParseException ex) { + throw new InitializationException("Warn initializing the suppression analyzer: " + ex.getLocalizedMessage(), ex, false); } } @Override protected void analyzeDependency(Dependency dependency, Engine engine) throws AnalysisException { + if (engine == null) { + return; + } + @SuppressWarnings("unchecked") + final List rules = (List) engine.getObject(SUPPRESSION_OBJECT_KEY); if (rules.isEmpty()) { return; } - for (SuppressionRule rule : rules.list()) { + for (SuppressionRule rule : rules) { if (filter(rule)) { rule.process(dependency); } @@ -141,7 +118,8 @@ protected void analyzeDependency(Dependency dependency, Engine engine) throws An } /** - * Determines whether a suppression rule should be retained when filtering a set of suppression rules for a concrete suppression analyzer. + * 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 true if the rule should be retained; otherwise @@ -152,9 +130,10 @@ protected void analyzeDependency(Dependency dependency, Engine engine) throws An /** * Loads all the suppression rules files configured in the {@link Settings}. * + * @param engine a reference to the ODC engine. * @throws SuppressionParseException thrown if the XML cannot be parsed. */ - private void loadSuppressionData() throws SuppressionParseException { + private void loadSuppressionData(Engine engine) throws SuppressionParseException { final List ruleList = new ArrayList<>(); final SuppressionParser parser = new SuppressionParser(); final String[] suppressionFilePaths = getSettings().getArray(Settings.KEYS.SUPPRESSION_FILE); @@ -170,8 +149,17 @@ private void loadSuppressionData() throws SuppressionParseException { } } } + LOGGER.debug("{} suppression rules were loaded.", ruleList.size()); - rules.addAll(ruleList); + if (!ruleList.isEmpty()) { + if (engine.hasObject(SUPPRESSION_OBJECT_KEY)) { + @SuppressWarnings("unchecked") + final List rules = (List) engine.getObject(SUPPRESSION_OBJECT_KEY); + rules.addAll(ruleList); + } else { + engine.putObject(SUPPRESSION_OBJECT_KEY, ruleList); + } + } if (!failedLoadingFiles.isEmpty()) { LOGGER.debug("{} suppression files failed to load.", failedLoadingFiles.size()); final StringBuilder sb = new StringBuilder(); @@ -183,9 +171,10 @@ private void loadSuppressionData() throws SuppressionParseException { /** * Loads all the base suppression rules files. * + * @param engine a reference to the ODC engine * @throws SuppressionParseException thrown if the XML cannot be parsed. */ - private void loadSuppressionBaseData() throws SuppressionParseException { + private void loadSuppressionBaseData(Engine engine) throws SuppressionParseException { final SuppressionParser parser = new SuppressionParser(); final List ruleList; try (InputStream in = FileUtils.getResourceAsStream(BASE_SUPPRESSION_FILE)) { @@ -196,7 +185,15 @@ private void loadSuppressionBaseData() throws SuppressionParseException { } catch (SAXException | IOException ex) { throw new SuppressionParseException("Unable to parse the base suppression data file", ex); } - rules.addAll(ruleList); + if (!ruleList.isEmpty()) { + if (engine.hasObject(SUPPRESSION_OBJECT_KEY)) { + @SuppressWarnings("unchecked") + final List rules = (List) engine.getObject(SUPPRESSION_OBJECT_KEY); + rules.addAll(ruleList); + } else { + engine.putObject(SUPPRESSION_OBJECT_KEY, ruleList); + } + } } /** @@ -304,4 +301,19 @@ private void throwSuppressionParseException(String message, Exception exception, LOGGER.debug("", exception); throw new SuppressionParseException(message, exception); } + + /** + * Returns the number of suppression rules currently loaded in the engine. + * + * @param engine a reference to the ODC engine + * @return the count of rules loaded + */ + public static int getRuleCount(Engine engine) { + if (engine.hasObject(SUPPRESSION_OBJECT_KEY)) { + @SuppressWarnings("unchecked") + final List rules = (List) engine.getObject(SUPPRESSION_OBJECT_KEY); + return rules.size(); + } + return 0; + } } diff --git a/core/src/main/java/org/owasp/dependencycheck/analyzer/CPEAnalyzer.java b/core/src/main/java/org/owasp/dependencycheck/analyzer/CPEAnalyzer.java index 6852bd277a8..1e15d327e72 100644 --- a/core/src/main/java/org/owasp/dependencycheck/analyzer/CPEAnalyzer.java +++ b/core/src/main/java/org/owasp/dependencycheck/analyzer/CPEAnalyzer.java @@ -136,6 +136,10 @@ public class CPEAnalyzer extends AbstractAnalyzer { * The CVE Database. */ private CveDB cve; + /** + * A reference to the ODC engine. + */ + private Engine engine; /** * The list of ecosystems to skip during analysis. These are skipped because * there is generally a more accurate vulnerability analyzer in the @@ -185,6 +189,7 @@ public AnalysisPhase getAnalysisPhase() { @Override public void prepareAnalyzer(Engine engine) throws InitializationException { super.prepareAnalyzer(engine); + this.engine = engine; try { this.open(engine.getDatabase()); } catch (IOException ex) { @@ -375,24 +380,24 @@ private void addMajorVersionToTerms(Set majorVersions, Map term.getKey() != null) .forEach(term -> majorVersions.stream() - .filter(version -> version != null - && (!term.getKey().endsWith(version) - && !Character.isDigit(term.getKey().charAt(term.getKey().length() - 1)) - && !products.containsKey(term.getKey() + version))) - .forEach(version -> { - addTerm(temp, term.getKey() + version); - })); + .filter(version -> version != null + && (!term.getKey().endsWith(version) + && !Character.isDigit(term.getKey().charAt(term.getKey().length() - 1)) + && !products.containsKey(term.getKey() + version))) + .forEach(version -> { + addTerm(temp, term.getKey() + version); + })); products.entrySet().stream() .filter(term -> term.getKey() != null) .forEach(term -> majorVersions.stream() - .filter(Objects::nonNull) - .map(version -> "v" + version) - .filter(version -> (!term.getKey().endsWith(version) - && !Character.isDigit(term.getKey().charAt(term.getKey().length() - 1)) - && !products.containsKey(term.getKey() + version))) - .forEach(version -> { - addTerm(temp, term.getKey() + version); - })); + .filter(Objects::nonNull) + .map(version -> "v" + version) + .filter(version -> (!term.getKey().endsWith(version) + && !Character.isDigit(term.getKey().charAt(term.getKey().length() - 1)) + && !products.containsKey(term.getKey() + version))) + .forEach(version -> { + addTerm(temp, term.getKey() + version); + })); products.putAll(temp); } @@ -957,7 +962,7 @@ protected boolean determineIdentifiers(Dependency dependency, String vendor, Str //TODO - while this gets the job down it is slow; consider refactoring dependency.addVulnerableSoftwareIdentifier(i); - suppression.analyze(dependency, null); + suppression.analyze(dependency, engine); if (dependency.getVulnerableSoftwareIdentifiers().contains(i)) { identifierAdded = true; if (!addedNonGuess && bestIdentifierQuality != IdentifierConfidence.BEST_GUESS) { diff --git a/core/src/main/java/org/owasp/dependencycheck/analyzer/UnusedSuppressionRuleAnalyzer.java b/core/src/main/java/org/owasp/dependencycheck/analyzer/UnusedSuppressionRuleAnalyzer.java new file mode 100644 index 00000000000..4c95fdbb78d --- /dev/null +++ b/core/src/main/java/org/owasp/dependencycheck/analyzer/UnusedSuppressionRuleAnalyzer.java @@ -0,0 +1,92 @@ +/* + * This file is part of dependency-check-core. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + * + * Copyright (c) 2022 Jeremy Long. All Rights Reserved. + */ +package org.owasp.dependencycheck.analyzer; + +import java.util.List; +import org.owasp.dependencycheck.Engine; +import static org.owasp.dependencycheck.analyzer.AbstractSuppressionAnalyzer.SUPPRESSION_OBJECT_KEY; +import org.owasp.dependencycheck.analyzer.exception.AnalysisException; +import org.owasp.dependencycheck.dependency.Dependency; +import org.owasp.dependencycheck.utils.Settings; +import org.owasp.dependencycheck.xml.suppression.SuppressionRule; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +/** + * Log the unused suppression rules. + * + * @author Jeremy Long + */ +public class UnusedSuppressionRuleAnalyzer extends AbstractAnalyzer { + + /** + * The Logger for use throughout the class. + */ + private static final Logger LOGGER = LoggerFactory.getLogger(UnusedSuppressionRuleAnalyzer.class); + /** + * A flag indicating whether or not the unused vulnerabilities have already + * been reported. + */ + private boolean reported = false; + + @Override + protected void analyzeDependency(Dependency dependency, Engine engine) throws AnalysisException { + if (!reported) { + logUnusedRules(engine); + reported = true; + } + } + + /** + * Logs unused suppression RULES. + * + * @param engine a reference to the ODC engine + */ + private void logUnusedRules(Engine engine) { + if (engine.hasObject(SUPPRESSION_OBJECT_KEY)) { + @SuppressWarnings("unchecked") + final List rules = (List) engine.getObject(SUPPRESSION_OBJECT_KEY); + rules.forEach((rule) -> { + if (!rule.isMatched() && !rule.isBase()) { + LOGGER.info("Suppression Rule had zero matches: {}", rule.toString()); + } + }); + } + } + + @Override + protected String getAnalyzerEnabledSettingKey() { + //technically incorrect - but we will reuse the enabled key for this analyzer + return Settings.KEYS.ANALYZER_VULNERABILITY_SUPPRESSION_ENABLED; + } + + @Override + public String getName() { + return "Unused Suppression Rule Analyzer"; + } + + @Override + public AnalysisPhase getAnalysisPhase() { + return AnalysisPhase.FINAL; + } + + @Override + public boolean supportsParallelProcessing() { + return false; + } +} diff --git a/core/src/main/java/org/owasp/dependencycheck/xml/suppression/SuppressionRules.java b/core/src/main/java/org/owasp/dependencycheck/xml/suppression/SuppressionRules.java deleted file mode 100644 index 9010e96f43b..00000000000 --- a/core/src/main/java/org/owasp/dependencycheck/xml/suppression/SuppressionRules.java +++ /dev/null @@ -1,104 +0,0 @@ -/* - * This file is part of dependency-check-core. - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - * - * Copyright (c) 2022 Jeremy Long. All Rights Reserved. - */ -package org.owasp.dependencycheck.xml.suppression; - -import edu.umd.cs.findbugs.annotations.SuppressFBWarnings; -import java.util.ArrayList; -import java.util.List; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; - -/** - * - * @author Jeremy Long - */ -public final class SuppressionRules { - - /** - * Singleton. - */ - private static final SuppressionRules INSTANCE = new SuppressionRules(); - /** - * The Logger for use throughout the class. - */ - private static final Logger LOGGER = LoggerFactory.getLogger(SuppressionRules.class); - /** - * The list of suppression RULES. - */ - private static final List RULES = new ArrayList<>(); - - private SuppressionRules() { - } - - /** - * Returns the instance of SuppressionRules. - * - * @return the instance of SuppressionRules - */ - @SuppressFBWarnings(justification = "Intended", value = {"MS_EXPOSE_REP"}) - public static SuppressionRules getInstance() { - return INSTANCE; - } - - /** - * Get the number of suppression RULES. - * - * @return the number of suppression RULES - */ - public int size() { - return RULES.size(); - } - - /** - * Returns true if there are no suppression RULES; otherwise false. - * - * @return true if there are no suppression RULES; otherwise false - */ - public boolean isEmpty() { - return RULES.isEmpty(); - } - - /** - * Returns the list of suppression RULES. - * - * @return the list of suppression RULES - */ - public List list() { - return RULES; - } - - /** - * Appends the new suppression RULES to the list of RULES. - * - * @param newRules the new suppression RULES - */ - public void addAll(List newRules) { - RULES.addAll(newRules); - } - - /** - * Logs unused suppression RULES. - */ - public void logUnusedRules() { - RULES.forEach((rule) -> { - if (!rule.isMatched() && !rule.isBase()) { - LOGGER.debug("Suppression Rule had zero matches: {}", rule.toString()); - } - }); - } -} diff --git a/core/src/main/resources/META-INF/services/org.owasp.dependencycheck.analyzer.Analyzer b/core/src/main/resources/META-INF/services/org.owasp.dependencycheck.analyzer.Analyzer index 9e0b88b0995..5175f4301c7 100644 --- a/core/src/main/resources/META-INF/services/org.owasp.dependencycheck.analyzer.Analyzer +++ b/core/src/main/resources/META-INF/services/org.owasp.dependencycheck.analyzer.Analyzer @@ -43,4 +43,5 @@ org.owasp.dependencycheck.analyzer.VersionFilterAnalyzer org.owasp.dependencycheck.analyzer.OssIndexAnalyzer org.owasp.dependencycheck.analyzer.PerlCpanfileAnalyzer org.owasp.dependencycheck.analyzer.PinnedMavenInstallAnalyzer -org.owasp.dependencycheck.analyzer.DartAnalyzer \ No newline at end of file +org.owasp.dependencycheck.analyzer.DartAnalyzer +org.owasp.dependencycheck.analyzer.UnusedSuppressionRuleAnalyzer \ No newline at end of file diff --git a/core/src/test/java/org/owasp/dependencycheck/analyzer/AbstractSuppressionAnalyzerTest.java b/core/src/test/java/org/owasp/dependencycheck/analyzer/AbstractSuppressionAnalyzerTest.java index 29d5b2e8042..36009f59781 100644 --- a/core/src/test/java/org/owasp/dependencycheck/analyzer/AbstractSuppressionAnalyzerTest.java +++ b/core/src/test/java/org/owasp/dependencycheck/analyzer/AbstractSuppressionAnalyzerTest.java @@ -28,13 +28,13 @@ import org.junit.Test; import org.owasp.dependencycheck.BaseTest; import org.owasp.dependencycheck.Engine; +import org.owasp.dependencycheck.Engine.Mode; import org.owasp.dependencycheck.analyzer.exception.AnalysisException; import org.owasp.dependencycheck.dependency.Dependency; import org.owasp.dependencycheck.exception.InitializationException; import org.owasp.dependencycheck.utils.Settings; import org.owasp.dependencycheck.utils.Settings.KEYS; import org.owasp.dependencycheck.xml.suppression.SuppressionRule; -import org.owasp.dependencycheck.xml.suppression.SuppressionRules; /** * @author Jeremy Long @@ -55,7 +55,6 @@ public class AbstractSuppressionAnalyzerTest extends BaseTest { @Before public void createObjectUnderTest() throws Exception { - SuppressionRules.getInstance().list().clear(); instance = new AbstractSuppressionAnalyzerImpl(); } @@ -108,18 +107,20 @@ public void testGetRulesFromMultipleSuppressionFiles() throws Exception { final String[] suppressionFiles = {SUPPRESSIONS_FILE, OTHER_SUPPRESSIONS_FILE}; getSettings().setArrayIfNotEmpty(KEYS.SUPPRESSION_FILE, suppressionFiles); instance.initialize(getSettings()); - instance.prepare(null); + Engine engine = new Engine(getSettings()); + instance.prepare(engine); // THEN rules from both files were loaded final int expectedSize = rulesInFirstFile + rulesInSecondFile + rulesInCoreFile; - assertThat("Expected suppressions from both files", instance.getRuleCount(), is(expectedSize)); + assertThat("Expected suppressions from both files", instance.getRuleCount(engine), is(expectedSize)); } @Test(expected = InitializationException.class) public void testFailureToLocateSuppressionFileAnywhere() throws Exception { getSettings().setString(Settings.KEYS.SUPPRESSION_FILE, "doesnotexist.xml"); instance.initialize(getSettings()); - instance.prepare(null); + Engine engine = new Engine(Mode.EVIDENCE_COLLECTION, getSettings()); + instance.prepare(engine); } /** @@ -133,9 +134,9 @@ private int getNumberOfRulesLoadedInCoreFile() throws Exception { getSettings().removeProperty(KEYS.SUPPRESSION_FILE); final AbstractSuppressionAnalyzerImpl coreFileAnalyzer = new AbstractSuppressionAnalyzerImpl(); coreFileAnalyzer.initialize(getSettings()); - coreFileAnalyzer.prepare(null); - int count = coreFileAnalyzer.getRuleCount(); - coreFileAnalyzer.reset(); + Engine engine = new Engine(Mode.EVIDENCE_COLLECTION, getSettings()); + coreFileAnalyzer.prepare(engine); + int count = AbstractSuppressionAnalyzer.getRuleCount(engine); return count; } @@ -151,9 +152,9 @@ private int getNumberOfRulesLoadedFromPath(final String path) throws Exception { getSettings().setString(KEYS.SUPPRESSION_FILE, path); final AbstractSuppressionAnalyzerImpl fileAnalyzer = new AbstractSuppressionAnalyzerImpl(); fileAnalyzer.initialize(getSettings()); - fileAnalyzer.prepare(null); - int count = fileAnalyzer.getRuleCount(); - fileAnalyzer.reset(); + Engine engine = new Engine(Mode.EVIDENCE_COLLECTION, getSettings()); + fileAnalyzer.prepare(engine); + int count = AbstractSuppressionAnalyzer.getRuleCount(engine); return count; } @@ -183,10 +184,6 @@ protected String getAnalyzerEnabledSettingKey() { public boolean filter(SuppressionRule rule) { return false; } - - public void reset() { - getSuppressionRules().list().clear(); - } } } diff --git a/core/src/test/java/org/owasp/dependencycheck/analyzer/CPEAnalyzerIT.java b/core/src/test/java/org/owasp/dependencycheck/analyzer/CPEAnalyzerIT.java index 08034220100..30829316718 100644 --- a/core/src/test/java/org/owasp/dependencycheck/analyzer/CPEAnalyzerIT.java +++ b/core/src/test/java/org/owasp/dependencycheck/analyzer/CPEAnalyzerIT.java @@ -170,44 +170,44 @@ public void testDetermineCPE() throws Exception { File file = BaseTest.getResourceAsFile(this, "struts2-core-2.1.2.jar"); //File file = new File(this.getClass().getClassLoader().getResource("axis2-adb-1.4.1.jar").getPath()); Dependency struts = new Dependency(file); + try (Engine engine = new Engine(getSettings())) { + CpeSuppressionAnalyzer suppressionAnalyzer = new CpeSuppressionAnalyzer(); + suppressionAnalyzer.initialize(getSettings()); + suppressionAnalyzer.prepare(engine); - CpeSuppressionAnalyzer suppressionAnalyzer = new CpeSuppressionAnalyzer(); - suppressionAnalyzer.initialize(getSettings()); - suppressionAnalyzer.prepare(null); - - FileNameAnalyzer fnAnalyzer = new FileNameAnalyzer(); - fnAnalyzer.analyze(struts, null); - - HintAnalyzer hintAnalyzer = new HintAnalyzer(); - hintAnalyzer.initialize(getSettings()); - hintAnalyzer.prepare(null); - JarAnalyzer jarAnalyzer = new JarAnalyzer(); - jarAnalyzer.initialize(getSettings()); - jarAnalyzer.accept(new File("test.jar"));//trick analyzer into "thinking it is active" - jarAnalyzer.prepare(null); - - jarAnalyzer.analyze(struts, null); - hintAnalyzer.analyze(struts, null); - //File fileCommonValidator = new File(this.getClass().getClassLoader().getResource("commons-validator-1.4.0.jar").getPath()); - File fileCommonValidator = BaseTest.getResourceAsFile(this, "commons-validator-1.4.0.jar"); - Dependency commonValidator = new Dependency(fileCommonValidator); - jarAnalyzer.analyze(commonValidator, null); - hintAnalyzer.analyze(commonValidator, null); - - //File fileSpring = new File(this.getClass().getClassLoader().getResource("spring-core-2.5.5.jar").getPath()); - File fileSpring = BaseTest.getResourceAsFile(this, "spring-core-2.5.5.jar"); - Dependency spring = new Dependency(fileSpring); - jarAnalyzer.analyze(spring, null); - hintAnalyzer.analyze(spring, null); - - //File fileSpring3 = new File(this.getClass().getClassLoader().getResource("spring-core-3.0.0.RELEASE.jar").getPath()); - File fileSpring3 = BaseTest.getResourceAsFile(this, "spring-core-3.0.0.RELEASE.jar"); - Dependency spring3 = new Dependency(fileSpring3); - jarAnalyzer.analyze(spring3, null); - hintAnalyzer.analyze(spring3, null); + FileNameAnalyzer fnAnalyzer = new FileNameAnalyzer(); + fnAnalyzer.analyze(struts, engine); + + HintAnalyzer hintAnalyzer = new HintAnalyzer(); + hintAnalyzer.initialize(getSettings()); + hintAnalyzer.prepare(engine); + JarAnalyzer jarAnalyzer = new JarAnalyzer(); + jarAnalyzer.initialize(getSettings()); + jarAnalyzer.accept(new File("test.jar"));//trick analyzer into "thinking it is active" + jarAnalyzer.prepare(engine); + + jarAnalyzer.analyze(struts, engine); + hintAnalyzer.analyze(struts, engine); + //File fileCommonValidator = new File(this.getClass().getClassLoader().getResource("commons-validator-1.4.0.jar").getPath()); + File fileCommonValidator = BaseTest.getResourceAsFile(this, "commons-validator-1.4.0.jar"); + Dependency commonValidator = new Dependency(fileCommonValidator); + jarAnalyzer.analyze(commonValidator, engine); + hintAnalyzer.analyze(commonValidator, engine); + + //File fileSpring = new File(this.getClass().getClassLoader().getResource("spring-core-2.5.5.jar").getPath()); + File fileSpring = BaseTest.getResourceAsFile(this, "spring-core-2.5.5.jar"); + Dependency spring = new Dependency(fileSpring); + jarAnalyzer.analyze(spring, engine); + hintAnalyzer.analyze(spring, engine); + + //File fileSpring3 = new File(this.getClass().getClassLoader().getResource("spring-core-3.0.0.RELEASE.jar").getPath()); + File fileSpring3 = BaseTest.getResourceAsFile(this, "spring-core-3.0.0.RELEASE.jar"); + Dependency spring3 = new Dependency(fileSpring3); + jarAnalyzer.analyze(spring3, engine); + hintAnalyzer.analyze(spring3, engine); + + CPEAnalyzer instance = new CPEAnalyzer(); - CPEAnalyzer instance = new CPEAnalyzer(); - try (Engine engine = new Engine(getSettings())) { engine.openDatabase(true, true); instance.initialize(getSettings()); instance.prepare(engine); diff --git a/core/src/test/java/org/owasp/dependencycheck/analyzer/CpeSuppressionAnalyzerIT.java b/core/src/test/java/org/owasp/dependencycheck/analyzer/CpeSuppressionAnalyzerIT.java index 71fd0e9fca3..6e49c507d2d 100644 --- a/core/src/test/java/org/owasp/dependencycheck/analyzer/CpeSuppressionAnalyzerIT.java +++ b/core/src/test/java/org/owasp/dependencycheck/analyzer/CpeSuppressionAnalyzerIT.java @@ -26,7 +26,6 @@ import org.owasp.dependencycheck.utils.Settings; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertTrue; -import org.owasp.dependencycheck.xml.suppression.SuppressionRules; /** * Testing the CPE suppression analyzer. @@ -64,24 +63,26 @@ public void testGetAnalysisPhase() { */ @Test public void testAnalyze() throws Exception { - //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(); + File file = BaseTest.getResourceAsFile(this, "commons-fileupload-1.2.1.jar"); File suppression = BaseTest.getResourceAsFile(this, "commons-fileupload-1.2.1.suppression.xml"); getSettings().setBoolean(Settings.KEYS.AUTO_UPDATE, false); getSettings().setBoolean(Settings.KEYS.ANALYZER_NEXUS_ENABLED, false); getSettings().setBoolean(Settings.KEYS.ANALYZER_CENTRAL_ENABLED, false); + Dependency dependency; + int cveSize = 0; + int cpeSize = 0; try (Engine engine = new Engine(getSettings())) { engine.scan(file); engine.analyzeDependencies(); - Dependency dependency = getDependency(engine, file); - int cveSize = dependency.getVulnerabilities().size(); - int cpeSize = dependency.getVulnerableSoftwareIdentifiers().size(); + dependency = getDependency(engine, file); + cveSize = dependency.getVulnerabilities().size(); + cpeSize = dependency.getVulnerableSoftwareIdentifiers().size(); assertTrue(cveSize > 0); assertTrue(cpeSize > 0); - //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(); - getSettings().setString(Settings.KEYS.SUPPRESSION_FILE, suppression.getAbsolutePath()); + } + getSettings().setString(Settings.KEYS.SUPPRESSION_FILE, suppression.getAbsolutePath()); + try (Engine engine = new Engine(getSettings())) { CpeSuppressionAnalyzer instance = new CpeSuppressionAnalyzer(); instance.initialize(getSettings()); instance.prepare(engine); @@ -91,8 +92,6 @@ public void testAnalyze() throws Exception { assertEquals(cveSize, dependency.getVulnerabilities().size()); assertEquals(cpeSize, dependency.getVulnerableSoftwareIdentifiers().size()); } - //be kind to other tests and cleanup any custom loaded suppression rules for your test. - SuppressionRules.getInstance().list().clear(); } /** diff --git a/core/src/test/java/org/owasp/dependencycheck/analyzer/VulnerabilitySuppressionAnalyzerIT.java b/core/src/test/java/org/owasp/dependencycheck/analyzer/VulnerabilitySuppressionAnalyzerIT.java index 6162276befe..26d17183ffc 100644 --- a/core/src/test/java/org/owasp/dependencycheck/analyzer/VulnerabilitySuppressionAnalyzerIT.java +++ b/core/src/test/java/org/owasp/dependencycheck/analyzer/VulnerabilitySuppressionAnalyzerIT.java @@ -26,7 +26,6 @@ import org.owasp.dependencycheck.utils.Settings; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertTrue; -import org.owasp.dependencycheck.xml.suppression.SuppressionRules; /** * Testing the vulnerability suppression analyzer. @@ -65,27 +64,27 @@ public void testGetAnalysisPhase() { */ @Test public void testAnalyze() throws Exception { - //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(); File file = BaseTest.getResourceAsFile(this, "commons-fileupload-1.2.1.jar"); File suppression = BaseTest.getResourceAsFile(this, "commons-fileupload-1.2.1.suppression.xml"); getSettings().setBoolean(Settings.KEYS.AUTO_UPDATE, false); getSettings().setBoolean(Settings.KEYS.ANALYZER_NEXUS_ENABLED, false); getSettings().setBoolean(Settings.KEYS.ANALYZER_CENTRAL_ENABLED, false); + int cveSize=0; + int cpeSize=0; + Dependency dependency; try (Engine engine = new Engine(getSettings())) { - //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(); engine.scan(file); engine.analyzeDependencies(); - Dependency dependency = getDependency(engine, file); - int cveSize = dependency.getVulnerabilities().size(); - int cpeSize = dependency.getVulnerableSoftwareIdentifiers().size(); + dependency = getDependency(engine, file); + cveSize = dependency.getVulnerabilities().size(); + cpeSize = dependency.getVulnerableSoftwareIdentifiers().size(); assertTrue(cveSize > 0); assertTrue(cpeSize > 0); - getSettings().setString(Settings.KEYS.SUPPRESSION_FILE, suppression.getAbsolutePath()); - //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(); + + } + getSettings().setString(Settings.KEYS.SUPPRESSION_FILE, suppression.getAbsolutePath()); + try (Engine engine = new Engine(getSettings())) { VulnerabilitySuppressionAnalyzer instance = new VulnerabilitySuppressionAnalyzer(); instance.initialize(getSettings()); instance.prepare(engine); @@ -97,8 +96,6 @@ public void testAnalyze() throws Exception { assertEquals(cveSize, dependency.getVulnerabilities().size()); assertEquals(cpeSize, dependency.getVulnerableSoftwareIdentifiers().size()); } - //be kind to other tests and cleanup any custom loaded suppression rules for your test. - SuppressionRules.getInstance().list().clear(); } /** diff --git a/core/src/test/java/org/owasp/dependencycheck/reporting/ReportGeneratorIT.java b/core/src/test/java/org/owasp/dependencycheck/reporting/ReportGeneratorIT.java index bbe87171305..5434df5b13c 100644 --- a/core/src/test/java/org/owasp/dependencycheck/reporting/ReportGeneratorIT.java +++ b/core/src/test/java/org/owasp/dependencycheck/reporting/ReportGeneratorIT.java @@ -44,7 +44,6 @@ import org.owasp.dependencycheck.data.update.exception.UpdateException; import org.owasp.dependencycheck.dependency.Dependency; import org.owasp.dependencycheck.utils.DownloadFailedException; -import org.owasp.dependencycheck.xml.suppression.SuppressionRules; /** * @@ -75,10 +74,7 @@ public void testGenerateReport() { settings.setBoolean(Settings.KEYS.ANALYZER_CENTRAL_ENABLED, false); settings.setBoolean(Settings.KEYS.PRETTY_PRINT, true); - generateReport(settings, writeTo, writeJsonTo, writeHtmlTo, writeJunitTo, writeCsvTo, writeSarifTo, suppressionFile); - - //be kind to other tests and cleanup any custom loaded suppression rules for your test. - SuppressionRules.getInstance().list().clear(); + generateReport(settings, writeTo, writeJsonTo, writeHtmlTo, writeJunitTo, writeCsvTo, writeSarifTo, suppressionFile); } /** @@ -104,8 +100,6 @@ public void testGenerateNodeAuditReport() { settings.setBoolean(Settings.KEYS.ANALYZER_CENTRAL_ENABLED, false); generateReport(settings, writeTo, writeJsonTo, writeHtmlTo, writeJunitTo, writeCsvTo, writeSarifTo, suppressionFile); - //be kind to other tests and cleanup any custom loaded suppression rules for your test. - SuppressionRules.getInstance().list().clear(); } @@ -132,8 +126,6 @@ public void testGenerateRetireJsReport() { settings.setBoolean(Settings.KEYS.ANALYZER_CENTRAL_ENABLED, false); generateReport(settings, writeTo, writeJsonTo, writeHtmlTo, writeJunitTo, writeCsvTo, writeSarifTo, suppressionFile); - //be kind to other tests and cleanup any custom loaded suppression rules for your test. - SuppressionRules.getInstance().list().clear(); } /** * Generates an XML report containing known vulnerabilities and realistic @@ -158,8 +150,6 @@ public void testGenerateNodePackageReport() { settings.setBoolean(Settings.KEYS.ANALYZER_CENTRAL_ENABLED, false); generateReport(settings, writeTo, writeJsonTo, writeHtmlTo, writeJunitTo, writeCsvTo, writeSarifTo, suppressionFile); - //be kind to other tests and cleanup any custom loaded suppression rules for your test. - SuppressionRules.getInstance().list().clear(); } @@ -227,7 +217,7 @@ public void generateReport(Settings settings, File writeTo, File writeJsonTo, Fi /** * create the parent folder if doesn't exist * @param file the file - * @return boolean is all fine ? + * @return true if all fine ? */ private boolean createParentFolder(File file){ if (!file.getParentFile().exists()) {