diff --git a/dependency-check-ant/src/main/java/org/owasp/dependencycheck/taskdefs/Check.java b/dependency-check-ant/src/main/java/org/owasp/dependencycheck/taskdefs/Check.java index 7c6bfc5a6..ec88800b6 100644 --- a/dependency-check-ant/src/main/java/org/owasp/dependencycheck/taskdefs/Check.java +++ b/dependency-check-ant/src/main/java/org/owasp/dependencycheck/taskdefs/Check.java @@ -1128,7 +1128,7 @@ public class Check extends Update { for (Dependency d : dependencies) { boolean firstEntry = true; final StringBuilder ids = new StringBuilder(); - for (Vulnerability v : d.getVulnerabilities()) { + for (Vulnerability v : d.getVulnerabilities(true)) { if (firstEntry) { firstEntry = false; } else { diff --git a/dependency-check-core/src/main/java/org/owasp/dependencycheck/Engine.java b/dependency-check-core/src/main/java/org/owasp/dependencycheck/Engine.java index 09545a051..41219c2aa 100644 --- a/dependency-check-core/src/main/java/org/owasp/dependencycheck/Engine.java +++ b/dependency-check-core/src/main/java/org/owasp/dependencycheck/Engine.java @@ -309,8 +309,8 @@ public class Engine implements FileFilter, AutoCloseable { */ public synchronized void sortDependencies() { //TODO - is this actually necassary???? - Collections.sort(dependencies); - dependenciesExternalView = null; +// Collections.sort(dependencies); +// dependenciesExternalView = null; } /** diff --git a/dependency-check-core/src/main/java/org/owasp/dependencycheck/agent/DependencyCheckScanAgent.java b/dependency-check-core/src/main/java/org/owasp/dependencycheck/agent/DependencyCheckScanAgent.java index fcbe5e3ce..a0e9e6fe3 100644 --- a/dependency-check-core/src/main/java/org/owasp/dependencycheck/agent/DependencyCheckScanAgent.java +++ b/dependency-check-core/src/main/java/org/owasp/dependencycheck/agent/DependencyCheckScanAgent.java @@ -1053,7 +1053,7 @@ public class DependencyCheckScanAgent { for (Dependency d : dependencies) { boolean firstEntry = true; final StringBuilder ids = new StringBuilder(); - for (Vulnerability v : d.getVulnerabilities()) { + for (Vulnerability v : d.getVulnerabilities(true)) { if (firstEntry) { firstEntry = false; } else { diff --git a/dependency-check-core/src/main/java/org/owasp/dependencycheck/analyzer/JarAnalyzer.java b/dependency-check-core/src/main/java/org/owasp/dependencycheck/analyzer/JarAnalyzer.java index 5a191efbc..1c3dca5c4 100644 --- a/dependency-check-core/src/main/java/org/owasp/dependencycheck/analyzer/JarAnalyzer.java +++ b/dependency-check-core/src/main/java/org/owasp/dependencycheck/analyzer/JarAnalyzer.java @@ -297,8 +297,7 @@ public class JarAnalyzer extends AbstractFileTypeAnalyzer { */ private boolean hasDependencyWithFilename(final Dependency[] dependencies, final String fileName) { for (final Dependency dependency : dependencies) { - if (Paths.get(dependency.getActualFilePath()).getFileName().toString().toLowerCase() - .equals(fileName.toLowerCase())) { + if (Paths.get(dependency.getActualFilePath()).getFileName().toString().equalsIgnoreCase(fileName)) { return true; } } 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 ee5bb1037..27b554d4b 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 @@ -26,7 +26,6 @@ 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.ThreadSafe; @@ -46,7 +45,7 @@ import org.slf4j.LoggerFactory; * @author Jeremy Long */ @ThreadSafe -public class Dependency extends EvidenceCollection implements Serializable, Comparable { +public class Dependency extends EvidenceCollection implements Serializable { /** * The serial version UID for serialization. @@ -95,7 +94,7 @@ public class Dependency extends EvidenceCollection implements Serializable, Comp /** * A set of vulnerabilities that have been suppressed. */ - private final SortedSet suppressedVulnerabilities = new TreeSet<>(new VulnerabilityComparator()); + private final Set suppressedVulnerabilities = new HashSet<>(); /** * The description of the JAR file. */ @@ -107,11 +106,11 @@ public class Dependency extends EvidenceCollection implements Serializable, Comp /** * A list of vulnerabilities for this dependency. */ - private final SortedSet vulnerabilities = new TreeSet<>(new VulnerabilityComparator()); + private final Set vulnerabilities = new HashSet<>(); /** * A collection of related dependencies. */ - private final Set relatedDependencies = new TreeSet<>(); + private final Set relatedDependencies = new HashSet<>(); /** * A list of projects that reference this dependency. */ @@ -457,12 +456,53 @@ public class Dependency extends EvidenceCollection implements Serializable, Comp } /** - * Get an unmodifiable sorted set of suppressedVulnerabilities. + * Get the unmodifiable sorted set of vulnerabilities. + * + * @return the unmodifiable sorted set of vulnerabilities + */ + public synchronized Set getVulnerabilities() { + return getVulnerabilities(false); + } + + /** + * Get the unmodifiable list of vulnerabilities; optionally sorted. + * + * @param sorted if true the list will be sorted + * @return the unmodifiable list set of vulnerabilities + */ + public synchronized Set getVulnerabilities(boolean sorted) { + Set r; + if (sorted) { + r = new TreeSet<>(vulnerabilities); + } else { + r = vulnerabilities; + } + return Collections.unmodifiableSet(r); + } + + /** + * Get an unmodifiable set of suppressedVulnerabilities. * * @return the unmodifiable sorted set of suppressedVulnerabilities */ - public synchronized SortedSet getSuppressedVulnerabilities() { - return Collections.unmodifiableSortedSet(new TreeSet<>(suppressedVulnerabilities)); + public synchronized Set getSuppressedVulnerabilities() { + return getSuppressedVulnerabilities(false); + } + + /** + * Get an unmodifiable, optionally sorted. set of suppressedVulnerabilities. + * + * @param sorted whether or not the set is sorted + * @return the unmodifiable sorted set of suppressedVulnerabilities + */ + public synchronized Set getSuppressedVulnerabilities(boolean sorted) { + Set r; + if (sorted) { + r = new TreeSet<>(suppressedVulnerabilities); + } else { + r = suppressedVulnerabilities; + } + return Collections.unmodifiableSet(r); } /** @@ -525,16 +565,7 @@ public class Dependency extends EvidenceCollection implements Serializable, Comp } /** - * Get the unmodifiable sorted set of vulnerabilities. - * - * @return the unmodifiable sorted set of vulnerabilities - */ - public synchronized SortedSet getVulnerabilities() { - return Collections.unmodifiableSortedSet(new TreeSet<>(vulnerabilities)); - } - - /** - * Determines the sha1 and md5 sum for the given file. + * Determines the SHA1 and MD5 sum for the given file. * * @param file the file to create checksums for */ @@ -624,11 +655,7 @@ public class Dependency extends EvidenceCollection implements Serializable, Comp } /** - * Adds a related dependency. The internal collection is normally a - * {@link java.util.TreeSet}, which relies on - * {@link #compareTo(Dependency)}. A consequence of this is that if you - * attempt to add a dependency with the same file path (modulo character - * case) as one that is already in the collection, it won't get added. + * Adds a related dependency. * * @param dependency a reference to the related dependency */ @@ -682,18 +709,6 @@ public class Dependency extends EvidenceCollection implements Serializable, Comp return isVirtual; } - /** - * Implementation of the Comparable<Dependency> interface. The - * comparison is solely based on the file path. - * - * @param o a dependency to compare - * @return an integer representing the natural ordering - */ - @Override - public int compareTo(Dependency o) { - return this.getFilePath().compareToIgnoreCase(o.getFilePath()); - } - /** * Implementation of the equals method. * @@ -720,6 +735,8 @@ public class Dependency extends EvidenceCollection implements Serializable, Comp .append(this.vulnerabilities, other.vulnerabilities) .append(this.projectReferences, other.projectReferences) .append(this.availableVersions, other.availableVersions) + .append(this.version, other.version) + .append(this.ecosystem, other.ecosystem) .isEquals(); } @@ -735,6 +752,7 @@ public class Dependency extends EvidenceCollection implements Serializable, Comp .append(actualFilePath) .append(filePath) .append(fileName) + .append(packagePath) .append(md5sum) .append(sha1sum) .append(identifiers) @@ -743,6 +761,8 @@ public class Dependency extends EvidenceCollection implements Serializable, Comp .append(vulnerabilities) .append(projectReferences) .append(availableVersions) + .append(version) + .append(ecosystem) .toHashCode(); } diff --git a/dependency-check-core/src/main/java/org/owasp/dependencycheck/dependency/Evidence.java b/dependency-check-core/src/main/java/org/owasp/dependencycheck/dependency/Evidence.java index aacda1e6a..912335f84 100644 --- a/dependency-check-core/src/main/java/org/owasp/dependencycheck/dependency/Evidence.java +++ b/dependency-check-core/src/main/java/org/owasp/dependencycheck/dependency/Evidence.java @@ -209,7 +209,7 @@ public class Evidence implements Serializable, Comparable { @Override public int compareTo(Evidence o) { if (o == null) { - return 1; + throw new IllegalArgumentException("Unable to compare null evidence"); } if (StringUtils.equalsIgnoreCase(source, o.source)) { if (StringUtils.equalsIgnoreCase(name, o.name)) { diff --git a/dependency-check-core/src/main/java/org/owasp/dependencycheck/dependency/Identifier.java b/dependency-check-core/src/main/java/org/owasp/dependencycheck/dependency/Identifier.java index 35c50d168..5cfa66e6d 100644 --- a/dependency-check-core/src/main/java/org/owasp/dependencycheck/dependency/Identifier.java +++ b/dependency-check-core/src/main/java/org/owasp/dependencycheck/dependency/Identifier.java @@ -19,6 +19,9 @@ package org.owasp.dependencycheck.dependency; import java.io.Serializable; import javax.annotation.concurrent.ThreadSafe; +import org.apache.commons.lang3.builder.CompareToBuilder; +import org.apache.commons.lang3.builder.EqualsBuilder; +import org.apache.commons.lang3.builder.HashCodeBuilder; /** * In identifier such as a CPE or dependency coordinates (i.e. GAV). @@ -42,7 +45,7 @@ public class Identifier implements Serializable, Comparable { */ private String value; /** - * The url for the identifier. + * The URL for the identifier. */ private String url; /** @@ -186,7 +189,7 @@ public class Identifier implements Serializable, Comparable { * * @param type the identifier type. * @param value the identifier value. - * @param url the identifier url. + * @param url the identifier URL. */ public Identifier(String type, String value, String url) { this.type = type; @@ -199,7 +202,7 @@ public class Identifier implements Serializable, Comparable { * * @param type the identifier type. * @param value the identifier value. - * @param url the identifier url. + * @param url the identifier URL. * @param description the description of the identifier. */ public Identifier(String type, String value, String url, String description) { @@ -207,27 +210,36 @@ public class Identifier implements Serializable, Comparable { this.description = description; } + /** + * Basic implementation of equals. This only compares the type and value of + * the identifier. + * @param obj the identifier to compare + * @return true if the objects are equal + */ @Override public boolean equals(Object obj) { - if (obj == null) { - return false; - } - if (getClass() != obj.getClass()) { + if (obj == null || getClass() != obj.getClass()) { return false; } final Identifier other = (Identifier) obj; - if ((this.value == null) ? (other.value != null) : !this.value.equals(other.value)) { - return false; - } - return !((this.type == null) ? (other.type != null) : !this.type.equals(other.type)); + + return new EqualsBuilder() + .append(this.type, other.type) + .append(this.value, other.value) + .isEquals(); } + /** + * Basic implementation of hasCode. Note, this only takes into consideration + * the type and value of the identifier. + * @return the hash code + */ @Override public int hashCode() { - int hash = 5; - hash = 53 * hash + (this.value != null ? this.value.hashCode() : 0); - hash = 53 * hash + (this.type != null ? this.type.hashCode() : 0); - return hash; + return new HashCodeBuilder(5, 49) + .append(type) + .append(value) + .toHashCode(); } /** @@ -241,7 +253,7 @@ public class Identifier implements Serializable, Comparable { } /** - * Implementation of the comparator interface. This compares the value of + * Implementation of the comparator interface. This compares the type and value of * the identifier only. * * @param o the object being compared @@ -250,8 +262,11 @@ public class Identifier implements Serializable, Comparable { @Override public int compareTo(Identifier o) { if (o == null) { - return -1; + throw new IllegalArgumentException("Unable to compare a null identifier"); } - return this.value.compareTo(o.value); + return new CompareToBuilder() + .append(this.type, o.type) + .append(this.value, o.value) + .toComparison(); } } diff --git a/dependency-check-core/src/main/java/org/owasp/dependencycheck/dependency/VulnerabilityComparator.java b/dependency-check-core/src/main/java/org/owasp/dependencycheck/dependency/VulnerabilityComparator.java deleted file mode 100644 index 5432559a0..000000000 --- a/dependency-check-core/src/main/java/org/owasp/dependencycheck/dependency/VulnerabilityComparator.java +++ /dev/null @@ -1,48 +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) 2012 Jeremy Long. All Rights Reserved. - */ -package org.owasp.dependencycheck.dependency; - -import java.io.Serializable; -import java.util.Comparator; -import javax.annotation.concurrent.ThreadSafe; - -/** - * Comparator for Vulnerability objects. - * - * @author Jeremy Long - */ -@ThreadSafe -public class VulnerabilityComparator implements Comparator, Serializable { - - /** - * The serial version UID. - */ - private static final long serialVersionUID = 1L; - - /** - * Implements the comparison of vulnerabilities. - * - * @param o1 a vulnerability - * @param o2 a second vulnerability - * @return the comparison - */ - @Override - public int compare(Vulnerability o1, Vulnerability o2) { - return o2.getName().compareTo(o1.getName()); - } -} diff --git a/dependency-check-core/src/main/resources/templates/csvReport.vsl b/dependency-check-core/src/main/resources/templates/csvReport.vsl index 536b4b370..816384cee 100644 --- a/dependency-check-core/src/main/resources/templates/csvReport.vsl +++ b/dependency-check-core/src/main/resources/templates/csvReport.vsl @@ -20,7 +20,7 @@ Copyright (c) 2017 Jeremy Long. All Rights Reserved. "Project","ScanDate","DependencyName","DependencyPath","Description","License","Md5","Sha1","Identifiers","CPE","CVE","CWE","Vulnerability","Source","Severity","CVSSv2","GAV","CPE Confidence","Evidence Count" #macro(writeSev $score)#if($score<4.0)"Low"#elseif($score>=7.0)"High"#else"Medium"#end#end #foreach($dependency in $dependencies)#if($dependency.getVulnerabilities().size()>0) -#foreach($vuln in $dependency.getVulnerabilities()) +#foreach($vuln in $dependency.getVulnerabilities(true)) $enc.csv($applicationName),$enc.csv($scanDate),$enc.csv($dependency.DisplayFileName),#if($dependency.FilePath)$enc.csv($dependency.FilePath)#end,#if($dependency.description)$enc.csv($dependency.description)#end,#if($dependency.license)$enc.csv($dependency.license)#end,#if($dependency.Md5sum)$enc.csv($dependency.Md5sum)#end,#if($dependency.Sha1sum)$enc.csv($dependency.Sha1sum)#end,#if($dependency.identifiers)$enc.csvIdentifiers($dependency.identifiers)#end,#if($dependency.identifiers)$enc.csvCpe($dependency.identifiers)#end,#if($vuln.name)$enc.csv($vuln.name)#end,#if($dependency.cwe)$enc.csv($vuln.cwe)#end,#if($vuln.description)$enc.csv($vuln.description)#end,#if($vuln.getSource().name())$enc.csv($vuln.getSource().name())#end,#writeSev($vuln.cvssScore),$vuln.cvssScore,#if($dependency.identifiers)$enc.csvGav($dependency.identifiers)#end,#if($dependency.identifiers)$enc.csvCpeConfidence($dependency.identifiers)#end,$dependency.size() #end #end diff --git a/dependency-check-core/src/main/resources/templates/htmlReport.vsl b/dependency-check-core/src/main/resources/templates/htmlReport.vsl index ceb7922d9..08170c044 100644 --- a/dependency-check-core/src/main/resources/templates/htmlReport.vsl +++ b/dependency-check-core/src/main/resources/templates/htmlReport.vsl @@ -843,7 +843,7 @@ Getting Help:

$enc.html($vuln.name)  

@@ -1015,7 +1015,7 @@ Getting Help:

$enc.html($vuln.name)  suppressed

diff --git a/dependency-check-core/src/main/resources/templates/jsonReport.vsl b/dependency-check-core/src/main/resources/templates/jsonReport.vsl index e0e72d76d..decfeaa83 100644 --- a/dependency-check-core/src/main/resources/templates/jsonReport.vsl +++ b/dependency-check-core/src/main/resources/templates/jsonReport.vsl @@ -132,7 +132,7 @@ #if($dependency.getVulnerabilities().size()>0) ,"vulnerabilities": [ - #foreach($vuln in $dependency.getVulnerabilities())#if($foreach.count > 1),#end { + #foreach($vuln in $dependency.getVulnerabilities(true))#if($foreach.count > 1),#end { "source": "$enc.json($vuln.getSource().name())", "name": "$enc.json($vuln.name)", "cvssScore": "$vuln.cvssScore", @@ -170,7 +170,7 @@ #if($dependency.getSuppressedVulnerabilities().size()>0 || $dependency.getSuppressedVulnerabilities().size()>0) ,"suppressedVulnerabilities": [ - #foreach($vuln in $dependency.getSuppressedVulnerabilities())#if($foreach.count > 1),#end { + #foreach($vuln in $dependency.getSuppressedVulnerabilities(true))#if($foreach.count > 1),#end { "source": "$enc.json($vuln.getSource().name())", "name": "$enc.json($vuln.name)", "cvssScore": "$vuln.cvssScore", diff --git a/dependency-check-core/src/main/resources/templates/vulnReport.vsl b/dependency-check-core/src/main/resources/templates/vulnReport.vsl index f3b23e41c..de84dec1b 100644 --- a/dependency-check-core/src/main/resources/templates/vulnReport.vsl +++ b/dependency-check-core/src/main/resources/templates/vulnReport.vsl @@ -208,7 +208,7 @@ have been reported. Additionally, the HTML report provides many features not fou #foreach($dependency in $dependencies) #if($dependency.getVulnerabilities().size()>0) - #foreach($vuln in $dependency.getVulnerabilities()) + #foreach($vuln in $dependency.getVulnerabilities(true)) #if($vuln.getSource().name().equals("NVD")) diff --git a/dependency-check-core/src/main/resources/templates/xmlReport.vsl b/dependency-check-core/src/main/resources/templates/xmlReport.vsl index e7068b110..86581ff37 100644 --- a/dependency-check-core/src/main/resources/templates/xmlReport.vsl +++ b/dependency-check-core/src/main/resources/templates/xmlReport.vsl @@ -140,7 +140,7 @@ Copyright (c) 2012 Jeremy Long. All Rights Reserved. #end #if($dependency.getVulnerabilities().size()>0 || $dependency.getSuppressedVulnerabilities().size()>0) -#foreach($vuln in $dependency.getVulnerabilities()) +#foreach($vuln in $dependency.getVulnerabilities(true)) $enc.xml($vuln.name) $vuln.cvssScore @@ -180,7 +180,7 @@ Copyright (c) 2012 Jeremy Long. All Rights Reserved. #end -#foreach($vuln in $dependency.getSuppressedVulnerabilities()) +#foreach($vuln in $dependency.getSuppressedVulnerabilities(true)) $enc.xml($vuln.name) $vuln.cvssScore diff --git a/dependency-check-core/src/test/java/org/owasp/dependencycheck/analyzer/RubyBundleAuditAnalyzerIT.java b/dependency-check-core/src/test/java/org/owasp/dependencycheck/analyzer/RubyBundleAuditAnalyzerIT.java index 1f44f88f7..8170435a0 100644 --- a/dependency-check-core/src/test/java/org/owasp/dependencycheck/analyzer/RubyBundleAuditAnalyzerIT.java +++ b/dependency-check-core/src/test/java/org/owasp/dependencycheck/analyzer/RubyBundleAuditAnalyzerIT.java @@ -157,7 +157,7 @@ public class RubyBundleAuditAnalyzerIT extends BaseDBTestCase { "ruby/vulnerable/gems/sinatra/Gemfile.lock")); analyzer.analyze(result, engine); Dependency dependency = engine.getDependencies()[0]; - Vulnerability vulnerability = dependency.getVulnerabilities().first(); + Vulnerability vulnerability = dependency.getVulnerabilities(true).iterator().next(); assertEquals(vulnerability.getCvssScore(), 5.0f, 0.0); } catch (InitializationException | DatabaseException | AnalysisException | UpdateException e) { diff --git a/dependency-check-utils/src/main/java/org/owasp/dependencycheck/utils/Checksum.java b/dependency-check-utils/src/main/java/org/owasp/dependencycheck/utils/Checksum.java index 25200c077..23c710408 100644 --- a/dependency-check-utils/src/main/java/org/owasp/dependencycheck/utils/Checksum.java +++ b/dependency-check-utils/src/main/java/org/owasp/dependencycheck/utils/Checksum.java @@ -194,7 +194,7 @@ public final class Checksum { return MessageDigest.getInstance(algorithm); } catch (NoSuchAlgorithmException e) { LOGGER.error(e.getMessage()); - final String msg = String.format("Failed to obtain the {} message digest.", algorithm); + final String msg = String.format("Failed to obtain the %s message digest.", algorithm); throw new IllegalStateException(msg, e); } } diff --git a/pom.xml b/pom.xml index 96c1af16b..53c3b847c 100644 --- a/pom.xml +++ b/pom.xml @@ -132,6 +132,7 @@ Copyright (c) 2012 - Jeremy Long 3.0 2.17 3.6 +