Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

fix concurrent modification exception #4935

Merged
merged 1 commit into from
Oct 19, 2022
Merged

fix concurrent modification exception #4935

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 1 addition & 7 deletions ant/src/test/java/org/owasp/dependencycheck/taskdefs/DependencyCheckTaskIT.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,6 @@
import org.owasp.dependencycheck.BaseDBTestCase;

import static org.junit.Assert.assertTrue;
import org.owasp.dependencycheck.xml.suppression.SuppressionRules;

/**
*
Expand Down Expand Up @@ -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()) {
Expand All @@ -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);

Expand All @@ -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);

Expand Down
48 changes: 45 additions & 3 deletions core/src/main/java/org/owasp/dependencycheck/Engine.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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<String, Object> objects = new HashMap<>();
/**
* The external view of the dependency list.
*/
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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 <code>true</code> if the object exists; otherwise
* <code>false</code>
*/
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.
*
Expand Down
106 changes: 59 additions & 47 deletions core/src/main/java/org/owasp/dependencycheck/analyzer/AbstractSuppressionAnalyzer.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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.
Expand All @@ -107,41 +84,42 @@ public Set<String> 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<SuppressionRule> rules = (List<SuppressionRule>) engine.getObject(SUPPRESSION_OBJECT_KEY);
if (rules.isEmpty()) {
return;
}
for (SuppressionRule rule : rules.list()) {
for (SuppressionRule rule : rules) {
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.
* 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
Expand All @@ -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<SuppressionRule> ruleList = new ArrayList<>();
final SuppressionParser parser = new SuppressionParser();
final String[] suppressionFilePaths = getSettings().getArray(Settings.KEYS.SUPPRESSION_FILE);
Expand All @@ -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<SuppressionRule> rules = (List<SuppressionRule>) 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();
Expand All @@ -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<SuppressionRule> ruleList;
try (InputStream in = FileUtils.getResourceAsStream(BASE_SUPPRESSION_FILE)) {
Expand All @@ -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<SuppressionRule> rules = (List<SuppressionRule>) engine.getObject(SUPPRESSION_OBJECT_KEY);
rules.addAll(ruleList);
} else {
engine.putObject(SUPPRESSION_OBJECT_KEY, ruleList);
}
}
}

/**
Expand Down Expand Up @@ -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<SuppressionRule> rules = (List<SuppressionRule>) engine.getObject(SUPPRESSION_OBJECT_KEY);
return rules.size();
}
return 0;
}
}
37 changes: 21 additions & 16 deletions core/src/main/java/org/owasp/dependencycheck/analyzer/CPEAnalyzer.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -375,24 +380,24 @@ private void addMajorVersionToTerms(Set<String> majorVersions, Map<String, Mutab
products.entrySet().stream()
.filter(term -> 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);
}

Expand Down Expand Up @@ -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) {
Expand Down
Loading