diff --git a/dependency-check-core/src/main/java/org/owasp/dependencycheck/analyzer/ArchiveAnalyzer.java b/dependency-check-core/src/main/java/org/owasp/dependencycheck/analyzer/ArchiveAnalyzer.java index 07ee70e50..4792c4509 100644 --- a/dependency-check-core/src/main/java/org/owasp/dependencycheck/analyzer/ArchiveAnalyzer.java +++ b/dependency-check-core/src/main/java/org/owasp/dependencycheck/analyzer/ArchiveAnalyzer.java @@ -266,7 +266,7 @@ public class ArchiveAnalyzer extends AbstractFileTypeAnalyzer { d.getFileName()); d.setFilePath(displayPath); d.setFileName(displayName); - d.setProjectReferences(dependency.getProjectReferences()); + d.addAllProjectReferences(dependency.getProjectReferences()); //TODO - can we get more evidence from the parent? EAR contains module name, etc. //analyze the dependency (i.e. extract files) if it is a supported type. diff --git a/dependency-check-core/src/main/java/org/owasp/dependencycheck/analyzer/DependencyBundlingAnalyzer.java b/dependency-check-core/src/main/java/org/owasp/dependencycheck/analyzer/DependencyBundlingAnalyzer.java index 4ff7fd58f..fbad6c0e7 100644 --- a/dependency-check-core/src/main/java/org/owasp/dependencycheck/analyzer/DependencyBundlingAnalyzer.java +++ b/dependency-check-core/src/main/java/org/owasp/dependencycheck/analyzer/DependencyBundlingAnalyzer.java @@ -120,11 +120,11 @@ public class DependencyBundlingAnalyzer extends AbstractDependencyComparingAnaly } else if (isShadedJar(dependency, nextDependency)) { if (dependency.getFileName().toLowerCase().endsWith("pom.xml")) { mergeDependencies(nextDependency, dependency, dependenciesToRemove); - nextDependency.getRelatedDependencies().remove(dependency); + nextDependency.removeRelatedDependencies(dependency); return true; } else { mergeDependencies(dependency, nextDependency, dependenciesToRemove); - dependency.getRelatedDependencies().remove(nextDependency); + dependency.removeRelatedDependencies(nextDependency); } } else if (cpeIdentifiersMatch(dependency, nextDependency) && hasSameBasePath(dependency, nextDependency) @@ -152,10 +152,9 @@ public class DependencyBundlingAnalyzer extends AbstractDependencyComparingAnaly */ private void mergeDependencies(final Dependency dependency, final Dependency relatedDependency, final Set dependenciesToRemove) { dependency.addRelatedDependency(relatedDependency); - final Iterator i = relatedDependency.getRelatedDependencies().iterator(); - while (i.hasNext()) { - dependency.addRelatedDependency(i.next()); - i.remove(); + for (Dependency d : relatedDependency.getRelatedDependencies()) { + dependency.addRelatedDependency(d); + relatedDependency.removeRelatedDependencies(d); } if (dependency.getSha1sum().equals(relatedDependency.getSha1sum())) { dependency.addAllProjectReferences(relatedDependency.getProjectReferences()); diff --git a/dependency-check-core/src/main/java/org/owasp/dependencycheck/analyzer/DependencyMergingAnalyzer.java b/dependency-check-core/src/main/java/org/owasp/dependencycheck/analyzer/DependencyMergingAnalyzer.java index 341856649..b6ac2839a 100644 --- a/dependency-check-core/src/main/java/org/owasp/dependencycheck/analyzer/DependencyMergingAnalyzer.java +++ b/dependency-check-core/src/main/java/org/owasp/dependencycheck/analyzer/DependencyMergingAnalyzer.java @@ -132,10 +132,9 @@ public class DependencyMergingAnalyzer extends AbstractDependencyComparingAnalyz dependency.addEvidence(EvidenceType.VERSION, e); } - final Iterator i = relatedDependency.getRelatedDependencies().iterator(); - while (i.hasNext()) { - dependency.addRelatedDependency(i.next()); - i.remove(); + for (Dependency d : relatedDependency.getRelatedDependencies()) { + dependency.addRelatedDependency(d); + relatedDependency.removeRelatedDependencies(d); } if (dependency.getSha1sum().equals(relatedDependency.getSha1sum())) { dependency.addAllProjectReferences(relatedDependency.getProjectReferences()); diff --git a/dependency-check-core/src/main/java/org/owasp/dependencycheck/analyzer/FalsePositiveAnalyzer.java b/dependency-check-core/src/main/java/org/owasp/dependencycheck/analyzer/FalsePositiveAnalyzer.java index f71d95ae7..bb9decec8 100644 --- a/dependency-check-core/src/main/java/org/owasp/dependencycheck/analyzer/FalsePositiveAnalyzer.java +++ b/dependency-check-core/src/main/java/org/owasp/dependencycheck/analyzer/FalsePositiveAnalyzer.java @@ -22,6 +22,7 @@ import java.io.UnsupportedEncodingException; import java.net.URLEncoder; import java.util.ArrayList; import java.util.Collections; +import java.util.HashSet; import java.util.Iterator; import java.util.List; import java.util.ListIterator; @@ -160,17 +161,18 @@ public class FalsePositiveAnalyzer extends AbstractAnalyzer { } } if (mustContain != null) { - final Iterator itr = dependency.getIdentifiers().iterator(); - while (itr.hasNext()) { - final Identifier i = itr.next(); + final Set removalSet = new HashSet<>(); + for (Identifier i : dependency.getIdentifiers()) { if ("cpe".contains(i.getType()) && i.getValue() != null && i.getValue().startsWith("cpe:/a:springsource:") && !i.getValue().toLowerCase().contains(mustContain)) { - itr.remove(); - //dependency.getIdentifiers().remove(i); + removalSet.add(i); } } + for (Identifier i : removalSet) { + dependency.removeIdentifier(i); + } } } @@ -221,15 +223,15 @@ public class FalsePositiveAnalyzer extends AbstractAnalyzer { //how did we get here? LOGGER.debug("currentVersion and nextVersion are both null?"); } else if (currentVersion == null && nextVersion != null) { - dependency.getIdentifiers().remove(currentId); + dependency.removeIdentifier(currentId); } else if (nextVersion == null && currentVersion != null) { - dependency.getIdentifiers().remove(nextId); + dependency.removeIdentifier(nextId); } else if (currentVersion.length() < nextVersion.length()) { if (nextVersion.startsWith(currentVersion) || "-".equals(currentVersion)) { - dependency.getIdentifiers().remove(currentId); + dependency.removeIdentifier(currentId); } } else if (currentVersion.startsWith(nextVersion) || "-".equals(nextVersion)) { - dependency.getIdentifiers().remove(nextId); + dependency.removeIdentifier(nextId); } } } @@ -244,21 +246,22 @@ public class FalsePositiveAnalyzer extends AbstractAnalyzer { * @param dependency the dependency to remove JRE CPEs from */ private void removeJreEntries(Dependency dependency) { - final Set identifiers = dependency.getIdentifiers(); - final Iterator itr = identifiers.iterator(); - while (itr.hasNext()) { - final Identifier i = itr.next(); + final Set removalSet = new HashSet<>(); + for (Identifier i : dependency.getIdentifiers()) { final Matcher coreCPE = CORE_JAVA.matcher(i.getValue()); final Matcher coreFiles = CORE_FILES.matcher(dependency.getFileName()); if (coreCPE.matches() && !coreFiles.matches()) { - itr.remove(); + removalSet.add(i); } final Matcher coreJsfCPE = CORE_JAVA_JSF.matcher(i.getValue()); final Matcher coreJsfFiles = CORE_JSF_FILES.matcher(dependency.getFileName()); if (coreJsfCPE.matches() && !coreJsfFiles.matches()) { - itr.remove(); + removalSet.add(i); } } + for (Identifier i : removalSet) { + dependency.removeIdentifier(i); + } } /** @@ -290,8 +293,6 @@ public class FalsePositiveAnalyzer extends AbstractAnalyzer { * @param dependency the dependency to analyze */ protected void removeBadMatches(Dependency dependency) { - final Set identifiers = dependency.getIdentifiers(); - final Iterator itr = identifiers.iterator(); /* TODO - can we utilize the pom's groupid and artifactId to filter??? most of * these are due to low quality data. Other idea would be to say any CPE @@ -300,8 +301,7 @@ public class FalsePositiveAnalyzer extends AbstractAnalyzer { */ //Set groupId = dependency.getVendorEvidence().getEvidence("pom", "groupid"); //Set artifactId = dependency.getVendorEvidence().getEvidence("pom", "artifactid"); - while (itr.hasNext()) { - final Identifier i = itr.next(); + for (Identifier i : dependency.getIdentifiers()) { //TODO move this startsWith expression to the base suppression file if ("cpe".equals(i.getType())) { if ((i.getValue().matches(".*c\\+\\+.*") @@ -325,7 +325,8 @@ public class FalsePositiveAnalyzer extends AbstractAnalyzer { || dependency.getFileName().toLowerCase().endsWith(".tgz") || dependency.getFileName().toLowerCase().endsWith(".ear") || dependency.getFileName().toLowerCase().endsWith(".war"))) { - itr.remove(); + //itr.remove(); + dependency.removeIdentifier(i); } else if ((i.getValue().startsWith("cpe:/a:jquery:jquery") || i.getValue().startsWith("cpe:/a:prototypejs:prototype") || i.getValue().startsWith("cpe:/a:yahoo:yui")) @@ -333,7 +334,8 @@ public class FalsePositiveAnalyzer extends AbstractAnalyzer { || dependency.getFileName().toLowerCase().endsWith("pom.xml") || dependency.getFileName().toLowerCase().endsWith(".dll") || dependency.getFileName().toLowerCase().endsWith(".exe"))) { - itr.remove(); + //itr.remove(); + dependency.removeIdentifier(i); } else if ((i.getValue().startsWith("cpe:/a:microsoft:excel") || i.getValue().startsWith("cpe:/a:microsoft:word") || i.getValue().startsWith("cpe:/a:microsoft:visio") @@ -344,10 +346,12 @@ public class FalsePositiveAnalyzer extends AbstractAnalyzer { || dependency.getFileName().toLowerCase().endsWith(".ear") || dependency.getFileName().toLowerCase().endsWith(".war") || dependency.getFileName().toLowerCase().endsWith("pom.xml"))) { - itr.remove(); + //itr.remove(); + dependency.removeIdentifier(i); } else if (i.getValue().startsWith("cpe:/a:apache:maven") && !dependency.getFileName().toLowerCase().matches("maven-core-[\\d\\.]+\\.jar")) { - itr.remove(); + //itr.remove(); + dependency.removeIdentifier(i); } else if (i.getValue().startsWith("cpe:/a:m-core:m-core")) { boolean found = false; for (Evidence e : dependency.getEvidence(EvidenceType.PRODUCT)) { @@ -365,11 +369,13 @@ public class FalsePositiveAnalyzer extends AbstractAnalyzer { } } if (!found) { - itr.remove(); + //itr.remove(); + dependency.removeIdentifier(i); } } else if (i.getValue().startsWith("cpe:/a:jboss:jboss") && !dependency.getFileName().toLowerCase().matches("jboss-?[\\d\\.-]+(GA)?\\.jar")) { - itr.remove(); + //itr.remove(); + dependency.removeIdentifier(i); } } } @@ -382,31 +388,30 @@ public class FalsePositiveAnalyzer extends AbstractAnalyzer { * @param dependency the dependency to analyze */ private void removeWrongVersionMatches(Dependency dependency) { - final Set identifiers = dependency.getIdentifiers(); - final Iterator itr = identifiers.iterator(); - + final Set identifiersToRemove = new HashSet<>(); final String fileName = dependency.getFileName(); if (fileName != null && fileName.contains("axis2")) { - while (itr.hasNext()) { - final Identifier i = itr.next(); + for (Identifier i : dependency.getIdentifiers()) { if ("cpe".equals(i.getType())) { final String cpe = i.getValue(); if (cpe != null && (cpe.startsWith("cpe:/a:apache:axis:") || "cpe:/a:apache:axis".equals(cpe))) { - itr.remove(); + identifiersToRemove.add(i); } } } } else if (fileName != null && fileName.contains("axis")) { - while (itr.hasNext()) { - final Identifier i = itr.next(); + for (Identifier i : dependency.getIdentifiers()) { if ("cpe".equals(i.getType())) { final String cpe = i.getValue(); if (cpe != null && (cpe.startsWith("cpe:/a:apache:axis2:") || "cpe:/a:apache:axis2".equals(cpe))) { - itr.remove(); + identifiersToRemove.add(i); } } } } + for (Identifier i : identifiersToRemove) { + dependency.removeIdentifier(i); + } } /** @@ -430,17 +435,13 @@ public class FalsePositiveAnalyzer extends AbstractAnalyzer { final String newCpe3 = String.format("cpe:/a:sun:opensso:%s", identifier.getValue().substring(22)); final String newCpe4 = String.format("cpe:/a:oracle:opensso:%s", identifier.getValue().substring(22)); try { - dependency.addIdentifier("cpe", - newCpe, + dependency.addIdentifier("cpe", newCpe, String.format(CPEAnalyzer.NVD_SEARCH_URL, URLEncoder.encode(newCpe, "UTF-8"))); - dependency.addIdentifier("cpe", - newCpe2, + dependency.addIdentifier("cpe", newCpe2, String.format(CPEAnalyzer.NVD_SEARCH_URL, URLEncoder.encode(newCpe2, "UTF-8"))); - dependency.addIdentifier("cpe", - newCpe3, + dependency.addIdentifier("cpe", newCpe3, String.format(CPEAnalyzer.NVD_SEARCH_URL, URLEncoder.encode(newCpe3, "UTF-8"))); - dependency.addIdentifier("cpe", - newCpe4, + dependency.addIdentifier("cpe", newCpe4, String.format(CPEAnalyzer.NVD_SEARCH_URL, URLEncoder.encode(newCpe4, "UTF-8"))); } catch (UnsupportedEncodingException ex) { LOGGER.debug("", ex); diff --git a/dependency-check-core/src/main/java/org/owasp/dependencycheck/analyzer/NspAnalyzer.java b/dependency-check-core/src/main/java/org/owasp/dependencycheck/analyzer/NspAnalyzer.java index d5aaafa4a..01a4aa929 100644 --- a/dependency-check-core/src/main/java/org/owasp/dependencycheck/analyzer/NspAnalyzer.java +++ b/dependency-check-core/src/main/java/org/owasp/dependencycheck/analyzer/NspAnalyzer.java @@ -198,7 +198,7 @@ public class NspAnalyzer extends AbstractFileTypeAnalyzer { vuln.setVulnerableSoftware(new HashSet<>(Arrays.asList(vs))); // Add the vulnerability to package.json - dependency.getVulnerabilities().add(vuln); + dependency.addVulnerability(vuln); } /* @@ -323,9 +323,12 @@ public class NspAnalyzer extends AbstractFileTypeAnalyzer { * dependency will not actually exist but needs to be unique (due to the use of Set in Dependency). * The use of related dependencies is a way to specify the actual software BOM in package.json. */ + //TODO is this actually correct? or should these be transitive dependencies? final Dependency nodeModule = new Dependency(new File(dependency.getActualFile() + "#" + entry.getKey()), true); nodeModule.setDisplayFileName(entry.getKey()); - nodeModule.setIdentifiers(new HashSet<>(Arrays.asList(moduleName, moduleVersion, moduleDepType))); + nodeModule.addIdentifier(moduleName); + nodeModule.addIdentifier(moduleVersion); + nodeModule.addIdentifier(moduleDepType); dependency.addRelatedDependency(nodeModule); } } diff --git a/dependency-check-core/src/main/java/org/owasp/dependencycheck/analyzer/NvdCveAnalyzer.java b/dependency-check-core/src/main/java/org/owasp/dependencycheck/analyzer/NvdCveAnalyzer.java index f1f2c45c3..6a6e82060 100644 --- a/dependency-check-core/src/main/java/org/owasp/dependencycheck/analyzer/NvdCveAnalyzer.java +++ b/dependency-check-core/src/main/java/org/owasp/dependencycheck/analyzer/NvdCveAnalyzer.java @@ -59,7 +59,7 @@ public class NvdCveAnalyzer extends AbstractAnalyzer { try { final String value = id.getValue(); final List vulns = cveDB.getVulnerabilities(value); - dependency.getVulnerabilities().addAll(vulns); + dependency.addVulnerabilities(vulns); } catch (DatabaseException ex) { throw new AnalysisException(ex); } @@ -70,7 +70,7 @@ public class NvdCveAnalyzer extends AbstractAnalyzer { try { final String value = id.getValue(); final List vulns = cveDB.getVulnerabilities(value); - dependency.getSuppressedVulnerabilities().addAll(vulns); + dependency.addSuppressedVulnerabilities(vulns); } catch (DatabaseException ex) { throw new AnalysisException(ex); } diff --git a/dependency-check-core/src/main/java/org/owasp/dependencycheck/analyzer/RubyBundleAuditAnalyzer.java b/dependency-check-core/src/main/java/org/owasp/dependencycheck/analyzer/RubyBundleAuditAnalyzer.java index 707392c74..29483a430 100644 --- a/dependency-check-core/src/main/java/org/owasp/dependencycheck/analyzer/RubyBundleAuditAnalyzer.java +++ b/dependency-check-core/src/main/java/org/owasp/dependencycheck/analyzer/RubyBundleAuditAnalyzer.java @@ -365,7 +365,7 @@ public class RubyBundleAuditAnalyzer extends AbstractFileTypeAnalyzer { vulnerability.setName(advisory); } if (null != dependency) { - dependency.getVulnerabilities().add(vulnerability); // needed to wait for vulnerability name to avoid NPE + dependency.addVulnerability(vulnerability); } LOGGER.debug("bundle-audit ({}): {}", parentName, nextLine); } diff --git a/dependency-check-core/src/main/java/org/owasp/dependencycheck/dependency/Dependency.java b/dependency-check-core/src/main/java/org/owasp/dependencycheck/dependency/Dependency.java index ab89d76d3..95f336c0a 100644 --- a/dependency-check-core/src/main/java/org/owasp/dependencycheck/dependency/Dependency.java +++ b/dependency-check-core/src/main/java/org/owasp/dependencycheck/dependency/Dependency.java @@ -22,12 +22,13 @@ import java.io.IOException; import java.io.Serializable; import java.security.NoSuchAlgorithmException; import java.util.ArrayList; +import java.util.Collections; import java.util.HashSet; import java.util.List; import java.util.Set; import java.util.SortedSet; import java.util.TreeSet; -import javax.annotation.concurrent.NotThreadSafe; +import javax.annotation.concurrent.ThreadSafe; import org.apache.commons.lang3.builder.EqualsBuilder; import org.apache.commons.lang3.builder.HashCodeBuilder; @@ -44,7 +45,7 @@ import org.slf4j.LoggerFactory; * * @author Jeremy Long */ -@NotThreadSafe +@ThreadSafe public class Dependency extends EvidenceCollection implements Serializable, Comparable { /** @@ -82,7 +83,7 @@ public class Dependency extends EvidenceCollection implements Serializable, Comp /** * A list of Identifiers. */ - private Set identifiers = new TreeSet<>(); + private final Set identifiers = new TreeSet<>(); /** * The file name to display in reports. */ @@ -90,11 +91,11 @@ public class Dependency extends EvidenceCollection implements Serializable, Comp /** * A set of identifiers that have been suppressed. */ - private Set suppressedIdentifiers = new TreeSet<>(); + private final Set suppressedIdentifiers = new TreeSet<>(); /** * A set of vulnerabilities that have been suppressed. */ - private SortedSet suppressedVulnerabilities = new TreeSet<>(new VulnerabilityComparator()); + private final SortedSet suppressedVulnerabilities = new TreeSet<>(new VulnerabilityComparator()); /** * The description of the JAR file. */ @@ -106,19 +107,19 @@ public class Dependency extends EvidenceCollection implements Serializable, Comp /** * A list of vulnerabilities for this dependency. */ - private SortedSet vulnerabilities = new TreeSet<>(new VulnerabilityComparator()); + private final SortedSet vulnerabilities = new TreeSet<>(new VulnerabilityComparator()); /** * A collection of related dependencies. */ - private Set relatedDependencies = new TreeSet<>(); + private final Set relatedDependencies = new TreeSet<>(); /** * A list of projects that reference this dependency. */ - private Set projectReferences = new HashSet<>(); + private final Set projectReferences = new HashSet<>(); /** * A list of available versions. */ - private List availableVersions = new ArrayList<>(); + private final List availableVersions = new ArrayList<>(); /** * Defines an actual or virtual dependency. @@ -322,21 +323,22 @@ public class Dependency extends EvidenceCollection implements Serializable, Comp } /** - * Returns a List of Identifiers. + * Returns an unmodifiable List of Identifiers. * - * @return an ArrayList of Identifiers + * @return an unmodifiable List of Identifiers */ - public Set getIdentifiers() { - return this.identifiers; + public synchronized Set getIdentifiers() { + return Collections.unmodifiableSet(new HashSet<>(identifiers)); } /** - * Sets a List of Identifiers. + * Adds a set of Identifiers to the current list of identifiers. Only used + * for testing. * - * @param identifiers A list of Identifiers + * @param identifiers A set of Identifiers */ - public void setIdentifiers(Set identifiers) { - this.identifiers = identifiers; + protected synchronized void addIdentifiers(Set identifiers) { + this.identifiers.addAll(identifiers); } /** @@ -347,7 +349,7 @@ public class Dependency extends EvidenceCollection implements Serializable, Comp * @param value the value of the identifier * @param url the URL of the identifier */ - public void addIdentifier(String type, String value, String url) { + public synchronized void addIdentifier(String type, String value, String url) { final Identifier i = new Identifier(type, value, url); this.identifiers.add(i); } @@ -361,12 +363,21 @@ public class Dependency extends EvidenceCollection implements Serializable, Comp * @param url the URL of the identifier * @param confidence the confidence in the Identifier being accurate */ - public void addIdentifier(String type, String value, String url, Confidence confidence) { + public synchronized void addIdentifier(String type, String value, String url, Confidence confidence) { final Identifier i = new Identifier(type, value, url); i.setConfidence(confidence); this.identifiers.add(i); } + /** + * Removes an identifier from the list of identifiers. + * + * @param i the identifier to remove + */ + public synchronized void removeIdentifier(Identifier i) { + this.identifiers.remove(i); + } + /** * Adds the maven artifact as evidence. * @@ -386,15 +397,17 @@ public class Dependency extends EvidenceCollection implements Serializable, Comp } if (mavenArtifact.getArtifactUrl() != null && !mavenArtifact.getArtifactUrl().isEmpty()) { boolean found = false; - for (Identifier i : this.getIdentifiers()) { - if ("maven".equals(i.getType()) && i.getValue().equals(mavenArtifact.toString())) { - found = true; - i.setConfidence(Confidence.HIGHEST); - final String url = "http://search.maven.org/#search|ga|1|1%3A%22" + this.getSha1sum() + "%22"; - i.setUrl(url); - //i.setUrl(mavenArtifact.getArtifactUrl()); - LOGGER.debug("Already found identifier {}. Confidence set to highest", i.getValue()); - break; + synchronized (this) { + for (Identifier i : this.identifiers) { + if ("maven".equals(i.getType()) && i.getValue().equals(mavenArtifact.toString())) { + found = true; + i.setConfidence(Confidence.HIGHEST); + final String url = "http://search.maven.org/#search|ga|1|1%3A%22" + this.getSha1sum() + "%22"; + i.setUrl(url); + //i.setUrl(mavenArtifact.getArtifactUrl()); + LOGGER.debug("Already found identifier {}. Confidence set to highest", i.getValue()); + break; + } } } if (!found) { @@ -410,26 +423,17 @@ public class Dependency extends EvidenceCollection implements Serializable, Comp * * @param identifier the identifier to add */ - public void addIdentifier(Identifier identifier) { + public synchronized void addIdentifier(Identifier identifier) { this.identifiers.add(identifier); } /** - * Get the value of suppressedIdentifiers. + * Get the unmodifiable set of suppressedIdentifiers. * * @return the value of suppressedIdentifiers */ - public Set getSuppressedIdentifiers() { - return suppressedIdentifiers; - } - - /** - * Set the value of suppressedIdentifiers. - * - * @param suppressedIdentifiers new value of suppressedIdentifiers - */ - public void setSuppressedIdentifiers(Set suppressedIdentifiers) { - this.suppressedIdentifiers = suppressedIdentifiers; + public synchronized Set getSuppressedIdentifiers() { + return Collections.unmodifiableSet(new HashSet<>(suppressedIdentifiers)); } /** @@ -437,26 +441,17 @@ public class Dependency extends EvidenceCollection implements Serializable, Comp * * @param identifier an identifier that was suppressed. */ - public void addSuppressedIdentifier(Identifier identifier) { + public synchronized void addSuppressedIdentifier(Identifier identifier) { this.suppressedIdentifiers.add(identifier); } /** - * Get the value of suppressedVulnerabilities. + * Get an unmodifiable sorted set of suppressedVulnerabilities. * - * @return the value of suppressedVulnerabilities + * @return the unmodifiable sorted set of suppressedVulnerabilities */ - public SortedSet getSuppressedVulnerabilities() { - return suppressedVulnerabilities; - } - - /** - * Set the value of suppressedVulnerabilities. - * - * @param suppressedVulnerabilities new value of suppressedVulnerabilities - */ - public void setSuppressedVulnerabilities(SortedSet suppressedVulnerabilities) { - this.suppressedVulnerabilities = suppressedVulnerabilities; + public synchronized SortedSet getSuppressedVulnerabilities() { + return Collections.unmodifiableSortedSet(new TreeSet<>(suppressedVulnerabilities)); } /** @@ -464,7 +459,7 @@ public class Dependency extends EvidenceCollection implements Serializable, Comp * * @param vulnerability the vulnerability that was suppressed */ - public void addSuppressedVulnerability(Vulnerability vulnerability) { + public synchronized void addSuppressedVulnerability(Vulnerability vulnerability) { this.suppressedVulnerabilities.add(vulnerability); } @@ -505,21 +500,12 @@ public class Dependency extends EvidenceCollection implements Serializable, Comp } /** - * Get the list of vulnerabilities. + * Get the unmodifiable sorted set of vulnerabilities. * - * @return the list of vulnerabilities + * @return the unmodifiable sorted set of vulnerabilities */ - public SortedSet getVulnerabilities() { - return vulnerabilities; - } - - /** - * Set the value of vulnerabilities. - * - * @param vulnerabilities new value of vulnerabilities - */ - public void setVulnerabilities(SortedSet vulnerabilities) { - this.vulnerabilities = vulnerabilities; + public synchronized SortedSet getVulnerabilities() { + return Collections.unmodifiableSortedSet(new TreeSet<>(vulnerabilities)); } /** @@ -550,39 +536,48 @@ public class Dependency extends EvidenceCollection implements Serializable, Comp /** * Adds a vulnerability to the dependency. * - * @param vulnerability a vulnerability outlining a vulnerability. + * @param vulnerability a vulnerability */ - public void addVulnerability(Vulnerability vulnerability) { + public synchronized void addVulnerability(Vulnerability vulnerability) { this.vulnerabilities.add(vulnerability); } /** - * Get the value of {@link #relatedDependencies}. This field is used to - * collect other dependencies which really represent the same dependency, - * and may be presented as one item in reports. + * Adds a list of vulnerabilities to the dependency. * - * @return the value of relatedDependencies + * @param vulnerabilities a list of vulnerabilities */ - public Set getRelatedDependencies() { - return relatedDependencies; + public synchronized void addVulnerabilities(List vulnerabilities) { + this.vulnerabilities.addAll(vulnerabilities); } /** - * Get the value of projectReferences. + * Removes the given vulnerability from the list. * - * @return the value of projectReferences + * @param v the vulnerability to remove */ - public Set getProjectReferences() { - return projectReferences; + public synchronized void removeVulnerability(Vulnerability v) { + this.vulnerabilities.remove(v); } /** - * Set the value of projectReferences. + * Get the unmodifiable set of {@link #relatedDependencies}. This field is + * used to collect other dependencies which really represent the same + * dependency, and may be presented as one item in reports. * - * @param projectReferences new value of projectReferences + * @return the unmodifiable set of relatedDependencies */ - public void setProjectReferences(Set projectReferences) { - this.projectReferences = projectReferences; + public synchronized Set getRelatedDependencies() { + return Collections.unmodifiableSet(new HashSet<>(relatedDependencies)); + } + + /** + * Get the unmodifiable set of projectReferences. + * + * @return the unmodifiable set of projectReferences + */ + public synchronized Set getProjectReferences() { + return Collections.unmodifiableSet(new HashSet<>(projectReferences)); } /** @@ -590,7 +585,7 @@ public class Dependency extends EvidenceCollection implements Serializable, Comp * * @param projectReference a project reference */ - public void addProjectReference(String projectReference) { + public synchronized void addProjectReference(String projectReference) { this.projectReferences.add(projectReference); } @@ -599,19 +594,10 @@ public class Dependency extends EvidenceCollection implements Serializable, Comp * * @param projectReferences a set of project references */ - public void addAllProjectReferences(Set projectReferences) { + public synchronized void addAllProjectReferences(Set projectReferences) { this.projectReferences.addAll(projectReferences); } - /** - * Set the value of relatedDependencies. - * - * @param relatedDependencies new value of relatedDependencies - */ - public void setRelatedDependencies(Set relatedDependencies) { - this.relatedDependencies = relatedDependencies; - } - /** * Adds a related dependency. The internal collection is normally a * {@link java.util.TreeSet}, which relies on @@ -621,7 +607,7 @@ public class Dependency extends EvidenceCollection implements Serializable, Comp * * @param dependency a reference to the related dependency */ - public void addRelatedDependency(Dependency dependency) { + public synchronized void addRelatedDependency(Dependency dependency) { if (this == dependency) { LOGGER.warn("Attempted to add a circular reference - please post the log file to issue #172 here " + "https://github.com/jeremylong/DependencyCheck/issues/172"); @@ -634,22 +620,22 @@ public class Dependency extends EvidenceCollection implements Serializable, Comp } } + /** + * Removes a related dependency. + * + * @param dependency the dependency to remove + */ + public synchronized void removeRelatedDependencies(Dependency dependency) { + this.relatedDependencies.remove(dependency); + } + /** * Get the value of availableVersions. * * @return the value of availableVersions */ - public List getAvailableVersions() { - return availableVersions; - } - - /** - * Set the value of availableVersions. - * - * @param availableVersions new value of availableVersions - */ - public void setAvailableVersions(List availableVersions) { - this.availableVersions = availableVersions; + public synchronized List getAvailableVersions() { + return Collections.unmodifiableList(new ArrayList<>(availableVersions)); } /** @@ -657,7 +643,7 @@ public class Dependency extends EvidenceCollection implements Serializable, Comp * * @param version the version to add to the list */ - public void addAvailableVersion(String version) { + public synchronized void addAvailableVersion(String version) { this.availableVersions.add(version); } @@ -746,4 +732,13 @@ public class Dependency extends EvidenceCollection implements Serializable, Comp return "Dependency{ fileName='" + fileName + "', actualFilePath='" + actualFilePath + "', filePath='" + filePath + "', packagePath='" + packagePath + "'}"; } + + /** + * Add a list of suppressed vulnerabilities to the collection. + * + * @param vulns the list of suppressed vulnerabilities to add + */ + public synchronized void addSuppressedVulnerabilities(List vulns) { + this.suppressedVulnerabilities.addAll(vulns); + } } diff --git a/dependency-check-core/src/main/java/org/owasp/dependencycheck/dependency/EvidenceCollection.java b/dependency-check-core/src/main/java/org/owasp/dependencycheck/dependency/EvidenceCollection.java index c4f7de2cd..eacb5d40c 100644 --- a/dependency-check-core/src/main/java/org/owasp/dependencycheck/dependency/EvidenceCollection.java +++ b/dependency-check-core/src/main/java/org/owasp/dependencycheck/dependency/EvidenceCollection.java @@ -119,13 +119,13 @@ class EvidenceCollection implements Serializable { switch (type) { case VENDOR: - list = Collections.unmodifiableSet(vendors); + list = Collections.unmodifiableSet(new HashSet<>(vendors)); break; case PRODUCT: - list = Collections.unmodifiableSet(products); + list = Collections.unmodifiableSet(new HashSet<>(products)); break; case VERSION: - list = Collections.unmodifiableSet(versions); + list = Collections.unmodifiableSet(new HashSet<>(versions)); break; default: return null; @@ -254,7 +254,7 @@ class EvidenceCollection implements Serializable { * @return an unmodifiable set of vendor weighting strings */ public synchronized Set getVendorWeightings() { - return Collections.unmodifiableSet(vendorWeightings); + return Collections.unmodifiableSet(new HashSet<>(vendorWeightings)); } /** @@ -265,7 +265,7 @@ class EvidenceCollection implements Serializable { * @return an unmodifiable set of vendor weighting strings */ public synchronized Set getProductWeightings() { - return Collections.unmodifiableSet(productWeightings); + return Collections.unmodifiableSet(new HashSet<>(productWeightings)); } /** @@ -278,11 +278,11 @@ class EvidenceCollection implements Serializable { if (null != type) { switch (type) { case VENDOR: - return Collections.unmodifiableSet(vendors); + return Collections.unmodifiableSet(new HashSet<>(vendors)); case PRODUCT: - return Collections.unmodifiableSet(products); + return Collections.unmodifiableSet(new HashSet<>(products)); case VERSION: - return Collections.unmodifiableSet(versions); + return Collections.unmodifiableSet(new HashSet<>(versions)); default: break; } diff --git a/dependency-check-core/src/main/java/org/owasp/dependencycheck/dependency/Vulnerability.java b/dependency-check-core/src/main/java/org/owasp/dependencycheck/dependency/Vulnerability.java index 394b9d2f5..dee7b5f80 100644 --- a/dependency-check-core/src/main/java/org/owasp/dependencycheck/dependency/Vulnerability.java +++ b/dependency-check-core/src/main/java/org/owasp/dependencycheck/dependency/Vulnerability.java @@ -278,6 +278,8 @@ public class Vulnerability implements Serializable, Comparable { * @param vulnSoftware the vulnerable software */ public void updateVulnerableSoftware(VulnerableSoftware vulnSoftware) { + //this behavior is because the vuln software being updated may have + // a value that is not included in the hash/comparison if (vulnerableSoftware.contains(vulnSoftware)) { vulnerableSoftware.remove(vulnSoftware); } diff --git a/dependency-check-core/src/main/java/org/owasp/dependencycheck/xml/suppression/SuppressionRule.java b/dependency-check-core/src/main/java/org/owasp/dependencycheck/xml/suppression/SuppressionRule.java index 875d33126..b2b9daf19 100644 --- a/dependency-check-core/src/main/java/org/owasp/dependencycheck/xml/suppression/SuppressionRule.java +++ b/dependency-check-core/src/main/java/org/owasp/dependencycheck/xml/suppression/SuppressionRule.java @@ -18,8 +18,10 @@ package org.owasp.dependencycheck.xml.suppression; import java.util.ArrayList; +import java.util.HashSet; import java.util.Iterator; import java.util.List; +import java.util.Set; import javax.annotation.concurrent.NotThreadSafe; import org.owasp.dependencycheck.dependency.Dependency; import org.owasp.dependencycheck.dependency.Identifier; @@ -365,9 +367,8 @@ public class SuppressionRule { } if (this.hasCpe()) { - final Iterator itr = dependency.getIdentifiers().iterator(); - while (itr.hasNext()) { - final Identifier i = itr.next(); + Set removalList = new HashSet<>(); + for (Identifier i : dependency.getIdentifiers()) { for (PropertyType c : this.cpe) { if (identifierMatches("cpe", c, i)) { if (!isBase()) { @@ -376,19 +377,22 @@ public class SuppressionRule { } dependency.addSuppressedIdentifier(i); } - itr.remove(); + removalList.add(i); break; } } } + for (Identifier i : removalList) { + dependency.removeIdentifier(i); + } } if (hasCve() || hasCwe() || hasCvssBelow()) { - final Iterator itr = dependency.getVulnerabilities().iterator(); - while (itr.hasNext()) { + Set removeVulns = new HashSet<>(); + for (Vulnerability v : dependency.getVulnerabilities()) { boolean remove = false; - final Vulnerability v = itr.next(); for (String entry : this.cve) { if (entry.equalsIgnoreCase(v.getName())) { + removeVulns.add(v); remove = true; break; } @@ -400,6 +404,7 @@ public class SuppressionRule { final String toTest = v.getCwe().substring(0, toMatch.length()).toUpperCase(); if (toTest.equals(toMatch)) { remove = true; + removeVulns.add(v); break; } } @@ -409,6 +414,7 @@ public class SuppressionRule { for (float cvss : this.cvssBelow) { if (v.getCvssScore() < cvss) { remove = true; + removeVulns.add(v); break; } } @@ -420,9 +426,11 @@ public class SuppressionRule { } dependency.addSuppressedVulnerability(v); } - itr.remove(); } } + for (Vulnerability v : removeVulns) { + dependency.removeVulnerability(v); + } } } diff --git a/dependency-check-core/src/test/java/org/owasp/dependencycheck/dependency/DependencyTest.java b/dependency-check-core/src/test/java/org/owasp/dependencycheck/dependency/DependencyTest.java index 4b303e7a3..01ed98315 100644 --- a/dependency-check-core/src/test/java/org/owasp/dependencycheck/dependency/DependencyTest.java +++ b/dependency-check-core/src/test/java/org/owasp/dependencycheck/dependency/DependencyTest.java @@ -177,7 +177,7 @@ public class DependencyTest extends BaseTest { public void testSetIdentifiers() { Set identifiers = new HashSet<>(); Dependency instance = new Dependency(); - instance.setIdentifiers(identifiers); + instance.addIdentifiers(identifiers); assertNotNull(instance.getIdentifiers()); } @@ -220,7 +220,7 @@ public class DependencyTest extends BaseTest { MavenArtifact mavenArtifact = new MavenArtifact("group", "artifact", "version", "url"); instance.addAsEvidence("pom", mavenArtifact, Confidence.HIGH); assertTrue(instance.contains(EvidenceType.VENDOR, Confidence.HIGH)); - assertTrue(instance.size()>1); + assertTrue(instance.size() > 1); assertFalse(instance.getIdentifiers().isEmpty()); } @@ -233,7 +233,7 @@ public class DependencyTest extends BaseTest { MavenArtifact mavenArtifact = new MavenArtifact(null, null, null, null); instance.addAsEvidence("pom", mavenArtifact, Confidence.HIGH); assertFalse(instance.getEvidence(EvidenceType.VENDOR).contains(Confidence.HIGH)); - assertTrue(instance.size()==0); + assertTrue(instance.size() == 0); assertTrue(instance.getIdentifiers().isEmpty()); } }