Merge pull request #3343 from jeremylong/ordering

Ensure Ordered Output in Reports

cli/src/main/java/org/owasp/dependencycheck/App.java

@@ -46,6 +46,7 @@
 import ch.qos.logback.classic.spi.ILoggingEvent;
 import ch.qos.logback.classic.Level;
 import ch.qos.logback.classic.LoggerContext;
+import java.util.TreeSet;
 import org.owasp.dependencycheck.utils.SeverityUtil;
 
 /**
@@ -316,7 +317,7 @@ private int determineReturnCode(Engine engine, float cvssFailScore) {
      * @return returns the set of identified files
      */
     private Set<File> scanAntStylePaths(List<String> antStylePaths, int symLinkDepth, String[] excludes) {
-        final Set<File> paths = new HashSet<>();
+        final Set<File> paths = new TreeSet<>();
         for (String file : antStylePaths) {
             LOGGER.debug("Scanning {}", file);
             final DirectoryScanner scanner = new DirectoryScanner();

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

@@ -80,6 +80,7 @@
 import static org.owasp.dependencycheck.analyzer.AnalysisPhase.PRE_FINDING_ANALYSIS;
 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;
 
 /**
  * Scans files, directories, etc. for Dependencies. Analyzers are loaded and
@@ -543,8 +544,9 @@ protected synchronized Dependency scanFile(@NotNull final File file, @Nullable f
                 if (sha1 != null) {
                     for (Dependency existing : dependencies) {
                         if (sha1.equals(existing.getSha1sum())) {
-                            if (existing.getFileName().contains(": ") || dependency.getFileName().contains(": ")) {
-                                //TODO this won't be quite right 100% of the time. Its possible that the ": " would get added later
+                            if (existing.getDisplayFileName().contains(": ") 
+                                    || dependency.getDisplayFileName().contains(": ") 
+                                    || dependency.getActualFilePath().contains("dctemp")) {
                                 continue;
                             }
                             found = true;
@@ -553,9 +555,18 @@ protected synchronized Dependency scanFile(@NotNull final File file, @Nullable f
                             }
                             if (existing.getActualFilePath() != null && dependency.getActualFilePath() != null
                                     && !existing.getActualFilePath().equals(dependency.getActualFilePath())) {
-                                existing.addRelatedDependency(dependency);
-                            } else {
-                                dependency = existing;
+
+                                if (DependencyBundlingAnalyzer.firstPathIsShortest(existing.getFilePath(), dependency.getFilePath())) {
+                                    DependencyBundlingAnalyzer.mergeDependencies(existing, dependency, null);
+                                    return null;
+                                } else {
+                                    //Merging dependency<-existing could be complicated. Instead analyze them seperately
+                                    //and possibly merge them at the end.
+                                    found = false;
+                                }
+
+                            } else { //somehow we scanned the same file twice?
+                                return null;
                             }
                             break;
                         }
@@ -1165,6 +1176,7 @@ public synchronized void writeReports(String applicationName, @Nullable final St
             throw new UnsupportedOperationException("Cannot generate report in evidence collection mode.");
         }
         final DatabaseProperties prop = database.getDatabaseProperties();
+
         final ReportGenerator r = new ReportGenerator(applicationName, groupId, artifactId, version,
                 dependencies, getAnalyzers(), prop, settings, exceptions);
         try {

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

@@ -89,7 +89,7 @@ protected synchronized void analyzeDependency(Dependency ignore, Engine engine)
             if (dependencies.length < 2) {
                 return;
             }
-            Arrays.sort(dependencies, (l, r) -> (l.getDisplayFileName() + l.getActualFilePath()).compareTo(r.getDisplayFileName() + r.getActualFilePath()));
+            Arrays.sort(dependencies, Dependency.NameComparator);
             for (int x = 0; x < dependencies.length - 1; x++) {
                 final Dependency dependency = dependencies[x];
                 if (!dependenciesToRemove.contains(dependency)) {

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

@@ -118,7 +118,7 @@ public boolean accept(File pathname) {
      * @throws AnalysisException thrown if the canonical path cannot be obtained
      * from the given file
      */
-    protected boolean shouldProcess(File pathname) throws AnalysisException {
+    public static boolean shouldProcess(File pathname) throws AnalysisException {
         try {
             // Do not scan the node_modules (or bower_components) directory
             final String canonicalPath = pathname.getCanonicalPath();

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

@@ -47,8 +47,10 @@
 import org.apache.commons.compress.utils.IOUtils;
 
 import org.owasp.dependencycheck.Engine;
+import static org.owasp.dependencycheck.analyzer.AbstractNpmAnalyzer.shouldProcess;
 import org.owasp.dependencycheck.analyzer.exception.AnalysisException;
 import org.owasp.dependencycheck.analyzer.exception.ArchiveExtractionException;
+import org.owasp.dependencycheck.analyzer.exception.UnexpectedAnalysisException;
 import org.owasp.dependencycheck.dependency.Dependency;
 import org.owasp.dependencycheck.exception.InitializationException;
 import org.owasp.dependencycheck.utils.FileFilterBuilder;
@@ -226,6 +228,30 @@ public void closeAnalyzer() throws Exception {
         }
     }
 
+    /**
+     * Determines if the file can be analyzed by the analyzer. If the npm
+     * analyzer are enabled the archive analyzer will skip the node_modules and
+     * bower_modules directories.
+     *
+     * @param pathname the path to the file
+     * @return true if the file can be analyzed by the given analyzer; otherwise
+     * false
+     */
+    @Override
+    public boolean accept(File pathname) {
+        boolean accept = super.accept(pathname);
+        boolean npmEnabled = getSettings().getBoolean(Settings.KEYS.ANALYZER_NODE_AUDIT_ENABLED, false);
+        boolean yarnEnabled = getSettings().getBoolean(Settings.KEYS.ANALYZER_YARN_AUDIT_ENABLED, false);
+        if (accept && (npmEnabled || yarnEnabled)) {
+            try {
+                accept = shouldProcess(pathname);
+            } catch (AnalysisException ex) {
+                throw new UnexpectedAnalysisException(ex.getMessage(), ex.getCause());
+            }
+        }
+        return accept;
+    }
+
     /**
      * Analyzes a given dependency. If the dependency is an archive, such as a
      * WAR or EAR, the contents are extracted, scanned, and added to the list of
@@ -469,10 +495,10 @@ private void extractFiles(File archive, File destination, Engine engine) throws
     }
 
     /**
-     * Checks if the file being scanned is a JAR or WAR that begins with '#!/bin' which
-     * indicates it is a fully executable jar. If a fully executable JAR is
-     * identified the input stream will be advanced to the start of the actual
-     * JAR file ( skipping the script).
+     * Checks if the file being scanned is a JAR or WAR that begins with
+     * '#!/bin' which indicates it is a fully executable jar. If a fully
+     * executable JAR is identified the input stream will be advanced to the
+     * start of the actual JAR file ( skipping the script).
      *
      * @see
      * <a href="http://docs.spring.io/spring-boot/docs/1.3.0.BUILD-SNAPSHOT/reference/htmlsingle/#deployment-install">Installing

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

@@ -476,8 +476,10 @@ protected boolean isWebJar(Dependency dependency, Dependency nextDependency) {
 
     /**
      * Attempts to convert a given JavaScript identifier to a web jar CPE.
+     *
      * @param id a JavaScript CPE
-     * @return a Maven CPE for a web jar if conversion is possible; otherwise the original CPE is returned
+     * @return a Maven CPE for a web jar if conversion is possible; otherwise
+     * the original CPE is returned
      */
     private String identifierToWebJarForCompairson(Identifier id) {
         if (id instanceof PurlIdentifier) {
@@ -529,8 +531,8 @@ protected boolean isShadedJar(Dependency dependency, Dependency nextDependency)
      * @return <code>true</code> if the leftPath is the shortest; otherwise
      * <code>false</code>
      */
-    protected boolean firstPathIsShortest(String left, String right) {
-        if (left.contains("dctemp")) {
+    public static boolean firstPathIsShortest(String left, String right) {
+        if (left.contains("dctemp") && !right.contains("dctemp")) {
             return false;
         }
         final String leftPath = left.replace('\\', '/');
@@ -552,7 +554,7 @@ protected boolean firstPathIsShortest(String left, String right) {
      * @param c the character to count
      * @return the number of times the character is present in the string
      */
-    private int countChar(String string, char c) {
+    private static int countChar(String string, char c) {
         int count = 0;
         final int max = string.length();
         for (int i = 0; i < max; i++) {

core/src/main/java/org/owasp/dependencycheck/dependency/Dependency.java

@@ -33,10 +33,12 @@
 import java.security.NoSuchAlgorithmException;
 import java.util.ArrayList;
 import java.util.Collections;
+import java.util.Comparator;
 import java.util.HashSet;
 import java.util.List;
 import java.util.Optional;
 import java.util.Set;
+import java.util.SortedSet;
 import java.util.TreeSet;
 import org.apache.commons.lang3.StringUtils;
 
@@ -99,7 +101,7 @@ public class Dependency extends EvidenceCollection implements Serializable {
     /**
      * A collection of related dependencies.
      */
-    private final Set<Dependency> relatedDependencies = new HashSet<>();
+    private final SortedSet<Dependency> relatedDependencies = new TreeSet<>(Dependency.NameComparator);
     /**
      * A list of projects that reference this dependency.
      */
@@ -741,7 +743,7 @@ public synchronized void removeVulnerability(Vulnerability v) {
      * @return the unmodifiable set of relatedDependencies
      */
     public synchronized Set<Dependency> getRelatedDependencies() {
-        return Collections.unmodifiableSet(new HashSet<>(relatedDependencies));
+        return Collections.unmodifiableSet(new TreeSet<>(relatedDependencies));
     }
 
     /**
@@ -942,6 +944,14 @@ public void setEcosystem(String ecosystem) {
         this.ecosystem = ecosystem;
     }
 
+    /**
+     * Simple sorting by display file name and actual file path.
+     */
+    public static Comparator<Dependency> NameComparator
+            = (Dependency d1, Dependency d2)
+            -> (d1.getDisplayFileName() + d1.getFilePath())
+                    .compareTo(d2.getDisplayFileName() + d2.getFilePath());
+
     /**
      * A hashing function shortcut.
      */

core/src/main/java/org/owasp/dependencycheck/dependency/EvidenceCollection.java

@@ -19,8 +19,8 @@
 
 import java.io.Serializable;
 import java.util.Collections;
-import java.util.HashSet;
 import java.util.Set;
+import java.util.TreeSet;
 import javax.annotation.concurrent.ThreadSafe;
 import org.apache.commons.lang3.builder.EqualsBuilder;
 import org.apache.commons.lang3.builder.HashCodeBuilder;
@@ -41,23 +41,23 @@ class EvidenceCollection implements Serializable {
     /**
      * A collection of vendor evidence.
      */
-    private final Set<Evidence> vendors = new HashSet<>();
+    private final Set<Evidence> vendors = new TreeSet<>();
     /**
      * A collection of strings used to adjust Lucene's vendor term weighting.
      */
-    private final Set<String> vendorWeightings = new HashSet<>();
+    private final Set<String> vendorWeightings = new TreeSet<>();
     /**
      * A collection of product evidence.
      */
-    private final Set<Evidence> products = new HashSet<>();
+    private final Set<Evidence> products = new TreeSet<>();
     /**
      * A collection of strings used to adjust Lucene's product term weighting.
      */
-    private final Set<String> productWeightings = new HashSet<>();
+    private final Set<String> productWeightings = new TreeSet<>();
     /**
      * A collection of version evidence.
      */
-    private final Set<Evidence> versions = new HashSet<>();
+    private final Set<Evidence> versions = new TreeSet<>();
 
     /**
      * Used to iterate over highest confidence evidence contained in the
@@ -113,13 +113,13 @@ public synchronized Iterable<Evidence> getIterator(EvidenceType type, Confidence
 
             switch (type) {
                 case VENDOR:
-                    list = Collections.unmodifiableSet(new HashSet<>(vendors));
+                    list = Collections.unmodifiableSet(new TreeSet<>(vendors));
                     break;
                 case PRODUCT:
-                    list = Collections.unmodifiableSet(new HashSet<>(products));
+                    list = Collections.unmodifiableSet(new TreeSet<>(products));
                     break;
                 case VERSION:
-                    list = Collections.unmodifiableSet(new HashSet<>(versions));
+                    list = Collections.unmodifiableSet(new TreeSet<>(versions));
                     break;
                 default:
                     return null;
@@ -248,7 +248,7 @@ public synchronized void addProductWeighting(String str) {
      * @return an unmodifiable set of vendor weighting strings
      */
     public synchronized Set<String> getVendorWeightings() {
-        return Collections.unmodifiableSet(new HashSet<>(vendorWeightings));
+        return Collections.unmodifiableSet(new TreeSet<>(vendorWeightings));
     }
 
     /**
@@ -259,7 +259,7 @@ public synchronized Set<String> getVendorWeightings() {
      * @return an unmodifiable set of vendor weighting strings
      */
     public synchronized Set<String> getProductWeightings() {
-        return Collections.unmodifiableSet(new HashSet<>(productWeightings));
+        return Collections.unmodifiableSet(new TreeSet<>(productWeightings));
     }
 
     /**
@@ -272,11 +272,11 @@ public synchronized Set<Evidence> getEvidence(EvidenceType type) {
         if (null != type) {
             switch (type) {
                 case VENDOR:
-                    return Collections.unmodifiableSet(new HashSet<>(vendors));
+                    return Collections.unmodifiableSet(new TreeSet<>(vendors));
                 case PRODUCT:
-                    return Collections.unmodifiableSet(new HashSet<>(products));
+                    return Collections.unmodifiableSet(new TreeSet<>(products));
                 case VERSION:
-                    return Collections.unmodifiableSet(new HashSet<>(versions));
+                    return Collections.unmodifiableSet(new TreeSet<>(versions));
                 default:
                     break;
             }
@@ -290,7 +290,7 @@ public synchronized Set<Evidence> getEvidence(EvidenceType type) {
      * @return the unmodifiable set of evidence
      */
     public synchronized Set<Evidence> getEvidence() {
-        final Set<Evidence> e = new HashSet<>(vendors);
+        final Set<Evidence> e = new TreeSet<>(vendors);
         e.addAll(products);
         e.addAll(versions);
         return Collections.unmodifiableSet(e);

core/src/main/java/org/owasp/dependencycheck/reporting/ReportGenerator.java

@@ -249,9 +249,7 @@ private VelocityContext createContext(String applicationName, List<Dependency> d
 
         final VelocityContext ctxt = new VelocityContext();
         ctxt.put("applicationName", applicationName);
-        Collections.sort(dependencies, (d1, d2) -> {
-            return d1.getDisplayFileName().compareTo(d2.getDisplayFileName());
-        });
+        Collections.sort(dependencies, Dependency.NameComparator);
         ctxt.put("dependencies", dependencies);
         ctxt.put("analyzers", analyzers);
         ctxt.put("properties", properties);

core/src/main/resources/templates/htmlReport.vsl

@@ -983,7 +983,7 @@ Getting Help: <a href="https://github.com/jeremylong/DependencyCheck/issues" tar
                             #elseif ($vuln.getVulnerableSoftware().size()!=0)
                                 <p>Vulnerable Software &amp; Versions ($vuln.getSource().name()):
                                     <ul>
-                                    #foreach($vs in $vuln.getVulnerableSoftware())
+                                    #foreach($vs in $vuln.getVulnerableSoftware(true))
                                         <li class="vs$vsctr">$enc.html($vs.toString())</li>
                                     #end
                                     </ul>
@@ -1151,7 +1151,7 @@ Getting Help: <a href="https://github.com/jeremylong/DependencyCheck/issues" tar
                                     #else
                                         <p>Vulnerable Software &amp; Versions ($vuln.getSource().name()):
                                             <ul>
-                                            #foreach($vs in $vuln.getVulnerableSoftware())
+                                            #foreach($vs in $vuln.getVulnerableSoftware(true))
                                                 <li class="vs$vsctr">$enc.html($vs.toString())</li>
                                             #end
                                             </ul>

core/src/main/resources/templates/jsonReport.vsl

@@ -237,7 +237,7 @@
                     }#end
                 ],
                 "vulnerableSoftware": [
-                    #foreach($vs in $vuln.getVulnerableSoftware())
+                    #foreach($vs in $vuln.getVulnerableSoftware(true))
                         #if($foreach.count > 1),#end {
                         "software": { 
                             "id":"$enc.json($vs.toCpe23FS())"
@@ -314,7 +314,7 @@
                     }#end
                 ],
                 "vulnerableSoftware": [
-                    #foreach($vs in $vuln.getVulnerableSoftware())
+                    #foreach($vs in $vuln.getVulnerableSoftware(true))
                         #if($foreach.count > 1),#end {
                         "software": { 
                             "id":"$enc.json($vs.toCpe23FS())"

core/src/main/resources/templates/xmlReport.vsl

@@ -257,7 +257,7 @@ Copyright (c) 2018 Jeremy Long. All Rights Reserved.
 #end
                     </references>
                     <vulnerableSoftware>
-#foreach($vs in $vuln.getVulnerableSoftware())
+#foreach($vs in $vuln.getVulnerableSoftware(true))
                         <software#if($vs == $vuln.matchedVulnerableSoftware) vulnerabilityIdMatched="true"#end
                             #if($vs.versionStartIncluding) versionStartIncluding="$enc.xml($vs.versionStartIncluding)"#end
                             #if($vs.versionStartExcluding) versionStartExcluding="$enc.xml($vs.versionStartExcluding)"#end
@@ -330,14 +330,14 @@ Copyright (c) 2018 Jeremy Long. All Rights Reserved.
 #end
                     </references>
                     <vulnerableSoftware>
-#foreach($vs in $vuln.getVulnerableSoftware())
+#foreach($vs in $vuln.getVulnerableSoftware(true))
                         <software#if($vs == $vuln.matchedVulnerableSoftware) matched="true"#end
                             #if($vs.versionStartIncluding) versionStartIncluding="$enc.xml($vs.versionStartIncluding)"#end
                             #if($vs.versionStartExcluding) versionStartExcluding="$enc.xml($vs.versionStartExcluding)"#end
                             #if($vs.versionEndIncluding) versionEndIncluding="$enc.xml($vs.versionEndIncluding)"#end
                             #if($vs.versionEndExcluding) versionEndExcluding="$enc.xml($vs.versionEndExcluding)"#end
                             #if(!$vs.vulnerable) vulnerable="$vs.vulnerable"#end
-                        >$enc.xml($vs.toCpe23FS())</software>                            
+                        >$enc.xml($vs.toCpe23FS())</software>
 #end
                     </vulnerableSoftware>
                 </suppressedVulnerability>

core/src/test/java/org/owasp/dependencycheck/EngineTest.java

@@ -53,7 +53,6 @@ public void testScanFile() throws DatabaseException {
             Dependency secondDwr = instance.scanFile(file);
 
             assertEquals(2, instance.getDependencies().length);
-            assertEquals(dwr, secondDwr);
         }
     }
 }