From edd4191d47c97b43e806af7e46271d7caa748b43 Mon Sep 17 00:00:00 2001 From: Jeremy Long Date: Fri, 16 Dec 2016 06:29:42 -0500 Subject: [PATCH 01/11] fix for #517 --- .../resources/dependencycheck-base-suppression.xml | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/dependency-check-core/src/main/resources/dependencycheck-base-suppression.xml b/dependency-check-core/src/main/resources/dependencycheck-base-suppression.xml index 70b1c71e7..61041a9f1 100644 --- a/dependency-check-core/src/main/resources/dependencycheck-base-suppression.xml +++ b/dependency-check-core/src/main/resources/dependencycheck-base-suppression.xml @@ -460,4 +460,18 @@ .* CVE-2007-6059 + + + com\.fasterxml\.jackson\.core:jackson.* + CVE-2016-3720 + + + + com\.fasterxml\.jackson\.dataformat:jackson(?!\-dataformat\-xml).* + CVE-2016-3720 + From 32ebf6c8ed42581aaa572b5a4deda4f2fbf64732 Mon Sep 17 00:00:00 2001 From: Jeremy Long Date: Sun, 18 Dec 2016 11:58:20 -0500 Subject: [PATCH 02/11] added phase to accomodate the fix for issue #630 --- .../org/owasp/dependencycheck/analyzer/AnalysisPhase.java | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/dependency-check-core/src/main/java/org/owasp/dependencycheck/analyzer/AnalysisPhase.java b/dependency-check-core/src/main/java/org/owasp/dependencycheck/analyzer/AnalysisPhase.java index 1716388b1..ab77388dd 100644 --- a/dependency-check-core/src/main/java/org/owasp/dependencycheck/analyzer/AnalysisPhase.java +++ b/dependency-check-core/src/main/java/org/owasp/dependencycheck/analyzer/AnalysisPhase.java @@ -36,6 +36,10 @@ public enum AnalysisPhase { * Information collection phase. */ INFORMATION_COLLECTION, + /** + * Post information collection phase. + */ + POST_INFORMATION_COLLECTION, /** * Pre identifier analysis phase. */ From d85491709025af72a363cdbb60a477c028a37b31 Mon Sep 17 00:00:00 2001 From: Jeremy Long Date: Sun, 18 Dec 2016 11:58:58 -0500 Subject: [PATCH 03/11] changes for issue #630 --- .../analyzer/DependencyBundlingAnalyzer.java | 134 +++--------------- 1 file changed, 23 insertions(+), 111 deletions(-) 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 6e0968e3a..9bb34f403 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 @@ -84,7 +84,7 @@ public class DependencyBundlingAnalyzer extends AbstractAnalyzer { /** * The phase that this analyzer is intended to run in. */ - private static final AnalysisPhase ANALYSIS_PHASE = AnalysisPhase.PRE_FINDING_ANALYSIS; + private static final AnalysisPhase ANALYSIS_PHASE = AnalysisPhase.FINAL; /** * Returns the name of the analyzer. @@ -162,6 +162,7 @@ public class DependencyBundlingAnalyzer extends AbstractAnalyzer { } } else if (cpeIdentifiersMatch(dependency, nextDependency) && hasSameBasePath(dependency, nextDependency) + && vulnCountMatches(dependency, nextDependency) && fileNameMatch(dependency, nextDependency)) { if (isCore(dependency, nextDependency)) { mergeDependencies(dependency, nextDependency, dependenciesToRemove); @@ -169,20 +170,6 @@ public class DependencyBundlingAnalyzer extends AbstractAnalyzer { mergeDependencies(nextDependency, dependency, dependenciesToRemove); break; //since we merged into the next dependency - skip forward to the next in mainIterator } - } else if ((main = getMainGemspecDependency(dependency, nextDependency)) != null) { - if (main == dependency) { - mergeDependencies(dependency, nextDependency, dependenciesToRemove); - } else { - mergeDependencies(nextDependency, dependency, dependenciesToRemove); - break; //since we merged into the next dependency - skip forward to the next in mainIterator - } - } else if ((main = getMainSwiftDependency(dependency, nextDependency)) != null) { - if (main == dependency) { - mergeDependencies(dependency, nextDependency, dependenciesToRemove); - } else { - mergeDependencies(nextDependency, dependency, dependenciesToRemove); - break; //since we merged into the next dependency - skip forward to the next in mainIterator - } } } } @@ -224,7 +211,12 @@ public class DependencyBundlingAnalyzer extends AbstractAnalyzer { * @return a string representing the base path. */ private String getBaseRepoPath(final String path) { - int pos = path.indexOf("repository" + File.separator) + 11; + int pos; + if (path.contains("local-repo")) { + pos = path.indexOf("local-repo" + File.separator) + 11; + } else { + pos = path.indexOf("repository" + File.separator) + 11; + } if (pos < 0) { return path; } @@ -317,6 +309,19 @@ public class DependencyBundlingAnalyzer extends AbstractAnalyzer { return matches; } + /** + * Returns true if the two dependencies have the same vulnerability count. + * + * @param dependency1 a dependency2 to compare + * @param dependency2 a dependency2 to compare + * @return true if the two dependencies have the same vulnerability count + */ + private boolean vulnCountMatches(Dependency dependency1, Dependency dependency2) { + return dependency1.getVulnerabilities() != null && dependency2.getVulnerabilities() != null + && dependency1.getVulnerabilities().size() == dependency2.getVulnerabilities().size(); + + } + /** * Determines if the two dependencies have the same base path. * @@ -341,7 +346,7 @@ public class DependencyBundlingAnalyzer extends AbstractAnalyzer { return true; } - if (left.matches(".*[/\\\\]repository[/\\\\].*") && right.matches(".*[/\\\\]repository[/\\\\].*")) { + if (left.matches(".*[/\\\\](repository|local-repo)[/\\\\].*") && right.matches(".*[/\\\\](repository|local-repo)[/\\\\].*")) { left = getBaseRepoPath(left); right = getBaseRepoPath(right); } @@ -357,96 +362,6 @@ public class DependencyBundlingAnalyzer extends AbstractAnalyzer { return false; } - /** - * Bundling Ruby gems that are identified from different .gemspec files but - * denote the same package path. This happens when Ruby bundler installs an - * application's dependencies by running "bundle install". - * - * @param dependency1 dependency to compare - * @param dependency2 dependency to compare - * @return true if the the dependencies being analyzed appear to be the - * same; otherwise false - */ - private boolean isSameRubyGem(Dependency dependency1, Dependency dependency2) { - if (dependency1 == null || dependency2 == null - || !dependency1.getFileName().endsWith(".gemspec") - || !dependency2.getFileName().endsWith(".gemspec") - || dependency1.getPackagePath() == null - || dependency2.getPackagePath() == null) { - return false; - } - return dependency1.getPackagePath().equalsIgnoreCase(dependency2.getPackagePath()); - } - - /** - * Ruby gems installed by "bundle install" can have zero or more *.gemspec - * files, all of which have the same packagePath and should be grouped. If - * one of these gemspec is from /specifications/*.gemspec, because - * it is a stub with fully resolved gem meta-data created by Ruby bundler, - * this dependency should be the main one. Otherwise, use dependency2 as - * main. - * - * This method returns null if any dependency is not from *.gemspec, or the - * two do not have the same packagePath. In this case, they should not be - * grouped. - * - * @param dependency1 dependency to compare - * @param dependency2 dependency to compare - * @return the main dependency; or null if a gemspec is not included in the - * analysis - */ - private Dependency getMainGemspecDependency(Dependency dependency1, Dependency dependency2) { - if (isSameRubyGem(dependency1, dependency2)) { - final File lFile = dependency1.getActualFile(); - final File left = lFile.getParentFile(); - if (left != null && left.getName().equalsIgnoreCase("specifications")) { - return dependency1; - } - return dependency2; - } - return null; - } - - /** - * Bundling same swift dependencies with the same packagePath but identified - * by different analyzers. - * - * @param dependency1 dependency to test - * @param dependency2 dependency to test - * @return true if the dependencies appear to be the same; - * otherwise false - */ - private boolean isSameSwiftPackage(Dependency dependency1, Dependency dependency2) { - if (dependency1 == null || dependency2 == null - || (!dependency1.getFileName().endsWith(".podspec") - && !dependency1.getFileName().equals("Package.swift")) - || (!dependency2.getFileName().endsWith(".podspec") - && !dependency2.getFileName().equals("Package.swift")) - || dependency1.getPackagePath() == null - || dependency2.getPackagePath() == null) { - return false; - } - return dependency1.getPackagePath().equalsIgnoreCase(dependency2.getPackagePath()); - } - - /** - * Determines which of the swift dependencies should be considered the - * primary. - * - * @param dependency1 the first swift dependency to compare - * @param dependency2 the second swift dependency to compare - * @return the primary swift dependency - */ - private Dependency getMainSwiftDependency(Dependency dependency1, Dependency dependency2) { - if (isSameSwiftPackage(dependency1, dependency2)) { - if (dependency1.getFileName().endsWith(".podspec")) { - return dependency1; - } - return dependency2; - } - return null; - } - /** * This is likely a very broken attempt at determining if the 'left' * dependency is the 'core' library in comparison to the 'right' library. @@ -469,10 +384,6 @@ public class DependencyBundlingAnalyzer extends AbstractAnalyzer { || !rightName.contains("core") && leftName.contains("core") || !rightName.contains("kernel") && leftName.contains("kernel")) { returnVal = true; -// } else if (leftName.matches(".*struts2\\-core.*") && rightName.matches(".*xwork\\-core.*")) { -// returnVal = true; -// } else if (rightName.matches(".*struts2\\-core.*") && leftName.matches(".*xwork\\-core.*")) { -// returnVal = false; } else { /* * considered splitting the names up and comparing the components, @@ -577,4 +488,5 @@ public class DependencyBundlingAnalyzer extends AbstractAnalyzer { private boolean containedInWar(String filePath) { return filePath == null ? false : filePath.matches(".*\\.(ear|war)[\\\\/].*"); } + } From 35ae8fd660c5e77543fd040a8086079a04683af9 Mon Sep 17 00:00:00 2001 From: Jeremy Long Date: Sun, 18 Dec 2016 11:59:30 -0500 Subject: [PATCH 04/11] updated test for #630 --- .../analyzer/DependencyMergingAnalyzer.java | 580 ++++++++++++++++++ 1 file changed, 580 insertions(+) create mode 100644 dependency-check-core/src/main/java/org/owasp/dependencycheck/analyzer/DependencyMergingAnalyzer.java 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 new file mode 100644 index 000000000..6e0968e3a --- /dev/null +++ b/dependency-check-core/src/main/java/org/owasp/dependencycheck/analyzer/DependencyMergingAnalyzer.java @@ -0,0 +1,580 @@ +/* + * 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.analyzer; + +import java.io.File; +import java.util.HashSet; +import java.util.Iterator; +import java.util.ListIterator; +import java.util.Set; +import java.util.regex.Matcher; +import java.util.regex.Pattern; +import org.owasp.dependencycheck.Engine; +import org.owasp.dependencycheck.analyzer.exception.AnalysisException; +import org.owasp.dependencycheck.dependency.Dependency; +import org.owasp.dependencycheck.dependency.Identifier; +import org.owasp.dependencycheck.utils.DependencyVersion; +import org.owasp.dependencycheck.utils.DependencyVersionUtil; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +/** + *

+ * This analyzer ensures dependencies that should be grouped together, to remove + * excess noise from the report, are grouped. An example would be Spring, Spring + * Beans, Spring MVC, etc. If they are all for the same version and have the + * same relative path then these should be grouped into a single dependency + * under the core/main library.

+ *

+ * Note, this grouping only works on dependencies with identified CVE + * entries

+ * + * @author Jeremy Long + */ +public class DependencyBundlingAnalyzer extends AbstractAnalyzer { + + /** + * The Logger. + */ + private static final Logger LOGGER = LoggerFactory.getLogger(DependencyBundlingAnalyzer.class); + + // + /** + * A pattern for obtaining the first part of a filename. + */ + private static final Pattern STARTING_TEXT_PATTERN = Pattern.compile("^[a-zA-Z0-9]*"); + + /** + * a flag indicating if this analyzer has run. This analyzer only runs once. + */ + private boolean analyzed = false; + + /** + * Returns a flag indicating if this analyzer has run. This analyzer only + * runs once. Note this is currently only used in the unit tests. + * + * @return a flag indicating if this analyzer has run. This analyzer only + * runs once + */ + protected boolean getAnalyzed() { + return analyzed; + } + + // + // + /** + * The name of the analyzer. + */ + private static final String ANALYZER_NAME = "Dependency Bundling Analyzer"; + /** + * The phase that this analyzer is intended to run in. + */ + private static final AnalysisPhase ANALYSIS_PHASE = AnalysisPhase.PRE_FINDING_ANALYSIS; + + /** + * Returns the name of the analyzer. + * + * @return the name of the analyzer. + */ + @Override + public String getName() { + return ANALYZER_NAME; + } + + /** + * Returns the phase that the analyzer is intended to run in. + * + * @return the phase that the analyzer is intended to run in. + */ + @Override + public AnalysisPhase getAnalysisPhase() { + return ANALYSIS_PHASE; + } + // + + /** + * Does not support parallel processing as it only runs once and then + * operates on all dependencies. + * + * @return whether or not parallel processing is enabled + * @see #analyze(Dependency, Engine) + */ + @Override + public boolean supportsParallelProcessing() { + return false; + } + + /** + * Analyzes a set of dependencies. If they have been found to have the same + * base path and the same set of identifiers they are likely related. The + * related dependencies are bundled into a single reportable item. + * + * @param ignore this analyzer ignores the dependency being analyzed + * @param engine the engine that is scanning the dependencies + * @throws AnalysisException is thrown if there is an error reading the JAR + * file. + */ + @Override + public void analyze(Dependency ignore, Engine engine) throws AnalysisException { + if (!analyzed) { + analyzed = true; + final Set dependenciesToRemove = new HashSet(); + final ListIterator mainIterator = engine.getDependencies().listIterator(); + //for (Dependency nextDependency : engine.getDependencies()) { + while (mainIterator.hasNext()) { + final Dependency dependency = mainIterator.next(); + if (mainIterator.hasNext() && !dependenciesToRemove.contains(dependency)) { + final ListIterator subIterator = engine.getDependencies().listIterator(mainIterator.nextIndex()); + while (subIterator.hasNext()) { + final Dependency nextDependency = subIterator.next(); + Dependency main = null; + if (hashesMatch(dependency, nextDependency) && !containedInWar(dependency.getFilePath()) + && !containedInWar(nextDependency.getFilePath())) { + if (firstPathIsShortest(dependency.getFilePath(), nextDependency.getFilePath())) { + mergeDependencies(dependency, nextDependency, dependenciesToRemove); + } else { + mergeDependencies(nextDependency, dependency, dependenciesToRemove); + break; //since we merged into the next dependency - skip forward to the next in mainIterator + } + } else if (isShadedJar(dependency, nextDependency)) { + if (dependency.getFileName().toLowerCase().endsWith("pom.xml")) { + mergeDependencies(nextDependency, dependency, dependenciesToRemove); + nextDependency.getRelatedDependencies().remove(dependency); + break; + } else { + mergeDependencies(dependency, nextDependency, dependenciesToRemove); + dependency.getRelatedDependencies().remove(nextDependency); + } + } else if (cpeIdentifiersMatch(dependency, nextDependency) + && hasSameBasePath(dependency, nextDependency) + && fileNameMatch(dependency, nextDependency)) { + if (isCore(dependency, nextDependency)) { + mergeDependencies(dependency, nextDependency, dependenciesToRemove); + } else { + mergeDependencies(nextDependency, dependency, dependenciesToRemove); + break; //since we merged into the next dependency - skip forward to the next in mainIterator + } + } else if ((main = getMainGemspecDependency(dependency, nextDependency)) != null) { + if (main == dependency) { + mergeDependencies(dependency, nextDependency, dependenciesToRemove); + } else { + mergeDependencies(nextDependency, dependency, dependenciesToRemove); + break; //since we merged into the next dependency - skip forward to the next in mainIterator + } + } else if ((main = getMainSwiftDependency(dependency, nextDependency)) != null) { + if (main == dependency) { + mergeDependencies(dependency, nextDependency, dependenciesToRemove); + } else { + mergeDependencies(nextDependency, dependency, dependenciesToRemove); + break; //since we merged into the next dependency - skip forward to the next in mainIterator + } + } + } + } + } + //removing dependencies here as ensuring correctness and avoiding ConcurrentUpdateExceptions + // was difficult because of the inner iterator. + engine.getDependencies().removeAll(dependenciesToRemove); + } + } + + /** + * Adds the relatedDependency to the dependency's related dependencies. + * + * @param dependency the main dependency + * @param relatedDependency a collection of dependencies to be removed from + * the main analysis loop, this is the source of dependencies to remove + * @param dependenciesToRemove a collection of dependencies that will be + * removed from the main analysis loop, this function adds to this + * collection + */ + 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(); + } + if (dependency.getSha1sum().equals(relatedDependency.getSha1sum())) { + dependency.addAllProjectReferences(relatedDependency.getProjectReferences()); + } + dependenciesToRemove.add(relatedDependency); + } + + /** + * Attempts to trim a maven repo to a common base path. This is typically + * [drive]\[repo_location]\repository\[path1]\[path2]. + * + * @param path the path to trim + * @return a string representing the base path. + */ + private String getBaseRepoPath(final String path) { + int pos = path.indexOf("repository" + File.separator) + 11; + if (pos < 0) { + return path; + } + int tmp = path.indexOf(File.separator, pos); + if (tmp <= 0) { + return path; + } + if (tmp > 0) { + pos = tmp + 1; + } + tmp = path.indexOf(File.separator, pos); + if (tmp > 0) { + pos = tmp + 1; + } + return path.substring(0, pos); + } + + /** + * Returns true if the file names (and version if it exists) of the two + * dependencies are sufficiently similar. + * + * @param dependency1 a dependency2 to compare + * @param dependency2 a dependency2 to compare + * @return true if the identifiers in the two supplied dependencies are + * equal + */ + private boolean fileNameMatch(Dependency dependency1, Dependency dependency2) { + if (dependency1 == null || dependency1.getFileName() == null + || dependency2 == null || dependency2.getFileName() == null) { + return false; + } + final String fileName1 = dependency1.getActualFile().getName(); + final String fileName2 = dependency2.getActualFile().getName(); + + //version check + final DependencyVersion version1 = DependencyVersionUtil.parseVersion(fileName1); + final DependencyVersion version2 = DependencyVersionUtil.parseVersion(fileName2); + if (version1 != null && version2 != null && !version1.equals(version2)) { + return false; + } + + //filename check + final Matcher match1 = STARTING_TEXT_PATTERN.matcher(fileName1); + final Matcher match2 = STARTING_TEXT_PATTERN.matcher(fileName2); + if (match1.find() && match2.find()) { + return match1.group().equals(match2.group()); + } + + return false; + } + + /** + * Returns true if the CPE identifiers in the two supplied dependencies are + * equal. + * + * @param dependency1 a dependency2 to compare + * @param dependency2 a dependency2 to compare + * @return true if the identifiers in the two supplied dependencies are + * equal + */ + private boolean cpeIdentifiersMatch(Dependency dependency1, Dependency dependency2) { + if (dependency1 == null || dependency1.getIdentifiers() == null + || dependency2 == null || dependency2.getIdentifiers() == null) { + return false; + } + boolean matches = false; + int cpeCount1 = 0; + int cpeCount2 = 0; + for (Identifier i : dependency1.getIdentifiers()) { + if ("cpe".equals(i.getType())) { + cpeCount1 += 1; + } + } + for (Identifier i : dependency2.getIdentifiers()) { + if ("cpe".equals(i.getType())) { + cpeCount2 += 1; + } + } + if (cpeCount1 > 0 && cpeCount1 == cpeCount2) { + for (Identifier i : dependency1.getIdentifiers()) { + if ("cpe".equals(i.getType())) { + matches |= dependency2.getIdentifiers().contains(i); + if (!matches) { + break; + } + } + } + } + LOGGER.debug("IdentifiersMatch={} ({}, {})", matches, dependency1.getFileName(), dependency2.getFileName()); + return matches; + } + + /** + * Determines if the two dependencies have the same base path. + * + * @param dependency1 a Dependency object + * @param dependency2 a Dependency object + * @return true if the base paths of the dependencies are identical + */ + private boolean hasSameBasePath(Dependency dependency1, Dependency dependency2) { + if (dependency1 == null || dependency2 == null) { + return false; + } + final File lFile = new File(dependency1.getFilePath()); + String left = lFile.getParent(); + final File rFile = new File(dependency2.getFilePath()); + String right = rFile.getParent(); + if (left == null) { + return right == null; + } else if (right == null) { + return false; + } + if (left.equalsIgnoreCase(right)) { + return true; + } + + if (left.matches(".*[/\\\\]repository[/\\\\].*") && right.matches(".*[/\\\\]repository[/\\\\].*")) { + left = getBaseRepoPath(left); + right = getBaseRepoPath(right); + } + if (left.equalsIgnoreCase(right)) { + return true; + } + //new code + for (Dependency child : dependency2.getRelatedDependencies()) { + if (hasSameBasePath(dependency1, child)) { + return true; + } + } + return false; + } + + /** + * Bundling Ruby gems that are identified from different .gemspec files but + * denote the same package path. This happens when Ruby bundler installs an + * application's dependencies by running "bundle install". + * + * @param dependency1 dependency to compare + * @param dependency2 dependency to compare + * @return true if the the dependencies being analyzed appear to be the + * same; otherwise false + */ + private boolean isSameRubyGem(Dependency dependency1, Dependency dependency2) { + if (dependency1 == null || dependency2 == null + || !dependency1.getFileName().endsWith(".gemspec") + || !dependency2.getFileName().endsWith(".gemspec") + || dependency1.getPackagePath() == null + || dependency2.getPackagePath() == null) { + return false; + } + return dependency1.getPackagePath().equalsIgnoreCase(dependency2.getPackagePath()); + } + + /** + * Ruby gems installed by "bundle install" can have zero or more *.gemspec + * files, all of which have the same packagePath and should be grouped. If + * one of these gemspec is from /specifications/*.gemspec, because + * it is a stub with fully resolved gem meta-data created by Ruby bundler, + * this dependency should be the main one. Otherwise, use dependency2 as + * main. + * + * This method returns null if any dependency is not from *.gemspec, or the + * two do not have the same packagePath. In this case, they should not be + * grouped. + * + * @param dependency1 dependency to compare + * @param dependency2 dependency to compare + * @return the main dependency; or null if a gemspec is not included in the + * analysis + */ + private Dependency getMainGemspecDependency(Dependency dependency1, Dependency dependency2) { + if (isSameRubyGem(dependency1, dependency2)) { + final File lFile = dependency1.getActualFile(); + final File left = lFile.getParentFile(); + if (left != null && left.getName().equalsIgnoreCase("specifications")) { + return dependency1; + } + return dependency2; + } + return null; + } + + /** + * Bundling same swift dependencies with the same packagePath but identified + * by different analyzers. + * + * @param dependency1 dependency to test + * @param dependency2 dependency to test + * @return true if the dependencies appear to be the same; + * otherwise false + */ + private boolean isSameSwiftPackage(Dependency dependency1, Dependency dependency2) { + if (dependency1 == null || dependency2 == null + || (!dependency1.getFileName().endsWith(".podspec") + && !dependency1.getFileName().equals("Package.swift")) + || (!dependency2.getFileName().endsWith(".podspec") + && !dependency2.getFileName().equals("Package.swift")) + || dependency1.getPackagePath() == null + || dependency2.getPackagePath() == null) { + return false; + } + return dependency1.getPackagePath().equalsIgnoreCase(dependency2.getPackagePath()); + } + + /** + * Determines which of the swift dependencies should be considered the + * primary. + * + * @param dependency1 the first swift dependency to compare + * @param dependency2 the second swift dependency to compare + * @return the primary swift dependency + */ + private Dependency getMainSwiftDependency(Dependency dependency1, Dependency dependency2) { + if (isSameSwiftPackage(dependency1, dependency2)) { + if (dependency1.getFileName().endsWith(".podspec")) { + return dependency1; + } + return dependency2; + } + return null; + } + + /** + * This is likely a very broken attempt at determining if the 'left' + * dependency is the 'core' library in comparison to the 'right' library. + * + * @param left the dependency to test + * @param right the dependency to test against + * @return a boolean indicating whether or not the left dependency should be + * considered the "core" version. + */ + boolean isCore(Dependency left, Dependency right) { + final String leftName = left.getFileName().toLowerCase(); + final String rightName = right.getFileName().toLowerCase(); + + final boolean returnVal; + if (!rightName.matches(".*\\.(tar|tgz|gz|zip|ear|war).+") && leftName.matches(".*\\.(tar|tgz|gz|zip|ear|war).+") + || rightName.contains("core") && !leftName.contains("core") + || rightName.contains("kernel") && !leftName.contains("kernel")) { + returnVal = false; + } else if (rightName.matches(".*\\.(tar|tgz|gz|zip|ear|war).+") && !leftName.matches(".*\\.(tar|tgz|gz|zip|ear|war).+") + || !rightName.contains("core") && leftName.contains("core") + || !rightName.contains("kernel") && leftName.contains("kernel")) { + returnVal = true; +// } else if (leftName.matches(".*struts2\\-core.*") && rightName.matches(".*xwork\\-core.*")) { +// returnVal = true; +// } else if (rightName.matches(".*struts2\\-core.*") && leftName.matches(".*xwork\\-core.*")) { +// returnVal = false; + } else { + /* + * considered splitting the names up and comparing the components, + * but decided that the file name length should be sufficient as the + * "core" component, if this follows a normal naming protocol should + * be shorter: + * axis2-saaj-1.4.1.jar + * axis2-1.4.1.jar <----- + * axis2-kernel-1.4.1.jar + */ + returnVal = leftName.length() <= rightName.length(); + } + LOGGER.debug("IsCore={} ({}, {})", returnVal, left.getFileName(), right.getFileName()); + return returnVal; + } + + /** + * Compares the SHA1 hashes of two dependencies to determine if they are + * equal. + * + * @param dependency1 a dependency object to compare + * @param dependency2 a dependency object to compare + * @return true if the sha1 hashes of the two dependencies match; otherwise + * false + */ + private boolean hashesMatch(Dependency dependency1, Dependency dependency2) { + if (dependency1 == null || dependency2 == null || dependency1.getSha1sum() == null || dependency2.getSha1sum() == null) { + return false; + } + return dependency1.getSha1sum().equals(dependency2.getSha1sum()); + } + + /** + * Determines if the jar is shaded and the created pom.xml identified the + * same CPE as the jar - if so, the pom.xml dependency should be removed. + * + * @param dependency a dependency to check + * @param nextDependency another dependency to check + * @return true if on of the dependencies is a pom.xml and the identifiers + * between the two collections match; otherwise false + */ + private boolean isShadedJar(Dependency dependency, Dependency nextDependency) { + final String mainName = dependency.getFileName().toLowerCase(); + final String nextName = nextDependency.getFileName().toLowerCase(); + if (mainName.endsWith(".jar") && nextName.endsWith("pom.xml")) { + return dependency.getIdentifiers().containsAll(nextDependency.getIdentifiers()); + } else if (nextName.endsWith(".jar") && mainName.endsWith("pom.xml")) { + return nextDependency.getIdentifiers().containsAll(dependency.getIdentifiers()); + } + return false; + } + + /** + * Determines which path is shortest; if path lengths are equal then we use + * compareTo of the string method to determine if the first path is smaller. + * + * @param left the first path to compare + * @param right the second path to compare + * @return true if the leftPath is the shortest; otherwise + * false + */ + protected boolean firstPathIsShortest(String left, String right) { + if (left.contains("dctemp")) { + return false; + } + final String leftPath = left.replace('\\', '/'); + final String rightPath = right.replace('\\', '/'); + + final int leftCount = countChar(leftPath, '/'); + final int rightCount = countChar(rightPath, '/'); + if (leftCount == rightCount) { + return leftPath.compareTo(rightPath) <= 0; + } else { + return leftCount < rightCount; + } + } + + /** + * Counts the number of times the character is present in the string. + * + * @param string the string to count the characters in + * @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) { + int count = 0; + final int max = string.length(); + for (int i = 0; i < max; i++) { + if (c == string.charAt(i)) { + count++; + } + } + return count; + } + + /** + * Checks if the given file path is contained within a war or ear file. + * + * @param filePath the file path to check + * @return true if the path contains '.war\' or '.ear\'. + */ + private boolean containedInWar(String filePath) { + return filePath == null ? false : filePath.matches(".*\\.(ear|war)[\\\\/].*"); + } +} From 91dbb39f1822a3fe5bac84e95c3e1ab2f3875608 Mon Sep 17 00:00:00 2001 From: Jeremy Long Date: Sun, 18 Dec 2016 11:59:59 -0500 Subject: [PATCH 05/11] updated test for #630 --- .../analyzer/DependencyBundlingAnalyzerTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dependency-check-core/src/test/java/org/owasp/dependencycheck/analyzer/DependencyBundlingAnalyzerTest.java b/dependency-check-core/src/test/java/org/owasp/dependencycheck/analyzer/DependencyBundlingAnalyzerTest.java index 7b3f81383..c4ba3357b 100644 --- a/dependency-check-core/src/test/java/org/owasp/dependencycheck/analyzer/DependencyBundlingAnalyzerTest.java +++ b/dependency-check-core/src/test/java/org/owasp/dependencycheck/analyzer/DependencyBundlingAnalyzerTest.java @@ -53,7 +53,7 @@ public class DependencyBundlingAnalyzerTest extends BaseTest { @Test public void testGetAnalysisPhase() { DependencyBundlingAnalyzer instance = new DependencyBundlingAnalyzer(); - AnalysisPhase expResult = AnalysisPhase.PRE_FINDING_ANALYSIS; + AnalysisPhase expResult = AnalysisPhase.FINAL; AnalysisPhase result = instance.getAnalysisPhase(); assertEquals(expResult, result); } From d91b4c3151d36ba23141e08431b7346022d0fe6e Mon Sep 17 00:00:00 2001 From: Jeremy Long Date: Sun, 18 Dec 2016 12:12:10 -0500 Subject: [PATCH 06/11] updated test case for performance of build --- .../618-aggregator-purge/invoker.properties | 5 ++-- .../it/618-aggregator-purge/prebuild.groovy | 28 ------------------ .../618-aggregator-purge/save-nvd-cve.groovy | 29 ------------------- 3 files changed, 2 insertions(+), 60 deletions(-) delete mode 100644 dependency-check-maven/src/it/618-aggregator-purge/prebuild.groovy delete mode 100644 dependency-check-maven/src/it/618-aggregator-purge/save-nvd-cve.groovy diff --git a/dependency-check-maven/src/it/618-aggregator-purge/invoker.properties b/dependency-check-maven/src/it/618-aggregator-purge/invoker.properties index b93b3959f..adb5bc444 100644 --- a/dependency-check-maven/src/it/618-aggregator-purge/invoker.properties +++ b/dependency-check-maven/src/it/618-aggregator-purge/invoker.properties @@ -16,6 +16,5 @@ # Copyright (c) 2014 Jeremy Long. All Rights Reserved. # -invoker.goals.1 = ${project.groupId}:${project.artifactId}:${project.version}:update-only -invoker.postBuildHookScript.1 = save-nvd-cve.groovy -invoker.goals.2 = ${project.groupId}:${project.artifactId}:${project.version}:purge +invoker.goals.1 = ${project.groupId}:${project.artifactId}:${project.version}:update-only -DdataDirectory=./data -Dcve.startyear=2016 +invoker.goals.2 = ${project.groupId}:${project.artifactId}:${project.version}:purge -DdataDirectory=./data diff --git a/dependency-check-maven/src/it/618-aggregator-purge/prebuild.groovy b/dependency-check-maven/src/it/618-aggregator-purge/prebuild.groovy deleted file mode 100644 index c1e9eda11..000000000 --- a/dependency-check-maven/src/it/618-aggregator-purge/prebuild.groovy +++ /dev/null @@ -1,28 +0,0 @@ -/* - * This file is part of dependency-check-maven. - * - * 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) 2014 Jeremy Long. All Rights Reserved. - */ - -import org.apache.commons.io.FileUtils; - -// Load NVD-CVE if not exist and had been saved in a previous IT -File datasDwl = new File("target/local-repo/org/owasp/dependency-check-data/3.0", "dc.h2.db"); -File datasSave = new File("target/nvd-cve-backup", "dc.h2.db"); - -if (!datasDwl.exists() && datasSave.exists()){ - System.out.println("Load NVD-CVE from backup"); - FileUtils.copyFile(datasSave, datasDwl); -} diff --git a/dependency-check-maven/src/it/618-aggregator-purge/save-nvd-cve.groovy b/dependency-check-maven/src/it/618-aggregator-purge/save-nvd-cve.groovy deleted file mode 100644 index 7ed8b07c4..000000000 --- a/dependency-check-maven/src/it/618-aggregator-purge/save-nvd-cve.groovy +++ /dev/null @@ -1,29 +0,0 @@ -/* - * This file is part of dependency-check-maven. - * - * 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) 2014 Jeremy Long. All Rights Reserved. - */ - -import java.nio.charset.Charset; -import org.apache.commons.io.FileUtils; -import org.apache.commons.lang.StringUtils; - -// Save NVD-CVE for next IT (if not already done) -File datasDwl = new File("target/local-repo/org/owasp/dependency-check-data/3.0", "dc.h2.db"); -File datasSave = new File("target/nvd-cve-backup", "dc.h2.db"); -if (datasDwl.exists() && !datasSave.exists()){ - System.out.println("Save NVD-CVE into backup"); - FileUtils.copyFile(datasDwl, datasSave); -} From bb927b447e3253e8a21a20c5d8ddef3e6bba79a7 Mon Sep 17 00:00:00 2001 From: Jeremy Long Date: Sun, 18 Dec 2016 12:12:57 -0500 Subject: [PATCH 07/11] updated so that the old suppression files could be processed --- .../main/java/org/owasp/dependencycheck/utils/XmlUtils.java | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/dependency-check-utils/src/main/java/org/owasp/dependencycheck/utils/XmlUtils.java b/dependency-check-utils/src/main/java/org/owasp/dependencycheck/utils/XmlUtils.java index d948b700f..734fcbb27 100644 --- a/dependency-check-utils/src/main/java/org/owasp/dependencycheck/utils/XmlUtils.java +++ b/dependency-check-utils/src/main/java/org/owasp/dependencycheck/utils/XmlUtils.java @@ -77,7 +77,9 @@ public final class XmlUtils { factory.setValidating(true); factory.setFeature("http://apache.org/xml/features/disallow-doctype-decl", true); factory.setFeature("http://xml.org/sax/features/external-general-entities", false); - factory.setFeature(XMLConstants.FEATURE_SECURE_PROCESSING, true); + //setting the following unfortunately breaks reading the old suppression files (version 1). + //factory.setFeature(XMLConstants.FEATURE_SECURE_PROCESSING, true); + final SAXParser saxParser = factory.newSAXParser(); saxParser.setProperty(JAXP_SCHEMA_LANGUAGE, W3C_XML_SCHEMA); saxParser.setProperty(JAXP_SCHEMA_SOURCE, schemaStream); From bf258146dace26ace852e049cf2d5f4f4ba3c4bb Mon Sep 17 00:00:00 2001 From: Jeremy Long Date: Sun, 18 Dec 2016 12:14:35 -0500 Subject: [PATCH 08/11] added test case for issue #629 and #517 --- .../629-jackson-datafromat/invoker.properties | 19 ++++++++ .../src/it/629-jackson-datafromat/pom.xml | 47 +++++++++++++++++++ .../629-jackson-datafromat/postbuild.groovy | 42 +++++++++++++++++ .../it/629-jackson-datafromat/prebuild.groovy | 28 +++++++++++ 4 files changed, 136 insertions(+) create mode 100644 dependency-check-maven/src/it/629-jackson-datafromat/invoker.properties create mode 100644 dependency-check-maven/src/it/629-jackson-datafromat/pom.xml create mode 100644 dependency-check-maven/src/it/629-jackson-datafromat/postbuild.groovy create mode 100644 dependency-check-maven/src/it/629-jackson-datafromat/prebuild.groovy diff --git a/dependency-check-maven/src/it/629-jackson-datafromat/invoker.properties b/dependency-check-maven/src/it/629-jackson-datafromat/invoker.properties new file mode 100644 index 000000000..3fc1810a0 --- /dev/null +++ b/dependency-check-maven/src/it/629-jackson-datafromat/invoker.properties @@ -0,0 +1,19 @@ +# +# This file is part of dependency-check-maven. +# +# 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) 2014 Jeremy Long. All Rights Reserved. +# + +invoker.goals = install ${project.groupId}:${project.artifactId}:${project.version}:check -e -Dformat=ALL diff --git a/dependency-check-maven/src/it/629-jackson-datafromat/pom.xml b/dependency-check-maven/src/it/629-jackson-datafromat/pom.xml new file mode 100644 index 000000000..f1e6cd154 --- /dev/null +++ b/dependency-check-maven/src/it/629-jackson-datafromat/pom.xml @@ -0,0 +1,47 @@ + + + + 4.0.0 + org.owasp.test + test-dataformat-jackson + 1.0.0-SNAPSHOT + jar + + + com.fasterxml.jackson.core + jackson-databind + 2.4.5 + + + com.fasterxml.jackson.core + jackson-annotations + 2.4.5 + + + com.fasterxml.jackson.dataformat + jackson-dataformat-cbor + 2.4.5 + + + com.fasterxml.jackson.dataformat + jackson-dataformat-xml + 2.4.5 + + + \ No newline at end of file diff --git a/dependency-check-maven/src/it/629-jackson-datafromat/postbuild.groovy b/dependency-check-maven/src/it/629-jackson-datafromat/postbuild.groovy new file mode 100644 index 000000000..17401a332 --- /dev/null +++ b/dependency-check-maven/src/it/629-jackson-datafromat/postbuild.groovy @@ -0,0 +1,42 @@ +/* + * This file is part of dependency-check-maven. + * + * 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) 2014 Jeremy Long. All Rights Reserved. + */ + +import org.apache.commons.io.FileUtils; +import org.apache.commons.lang.StringUtils; +import java.nio.charset.Charset; + +// Save NVD-CVE for next IT (if not already done) +File datasDwl = new File("target/local-repo/org/owasp/dependency-check-data/3.0", "dc.h2.db"); +File datasSave = new File("target/nvd-cve-backup", "dc.h2.db"); +if (datasDwl.exists() && !datasSave.exists()){ + System.out.println("Save NVD-CVE into backup"); + FileUtils.copyFile(datasDwl, datasSave); +} + + + + +// Check to see if jackson-dataformat-xml-2.4.5.jar was identified. +//TODO change this to xpath and check for CVE-2016-3720 +String log = FileUtils.readFileToString(new File(basedir, "target/dependency-check-report.xml"), Charset.defaultCharset().name()); +int count = StringUtils.countMatches(log, "jackson-dataformat-xml-2.4.5.jar"); +if (count == 0){ + System.out.println(String.format("The update should be unique, it is %s", count)); + return false; + //throw new Exception(String.format("The update should be unique, it is %s", count)); +} diff --git a/dependency-check-maven/src/it/629-jackson-datafromat/prebuild.groovy b/dependency-check-maven/src/it/629-jackson-datafromat/prebuild.groovy new file mode 100644 index 000000000..c1e9eda11 --- /dev/null +++ b/dependency-check-maven/src/it/629-jackson-datafromat/prebuild.groovy @@ -0,0 +1,28 @@ +/* + * This file is part of dependency-check-maven. + * + * 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) 2014 Jeremy Long. All Rights Reserved. + */ + +import org.apache.commons.io.FileUtils; + +// Load NVD-CVE if not exist and had been saved in a previous IT +File datasDwl = new File("target/local-repo/org/owasp/dependency-check-data/3.0", "dc.h2.db"); +File datasSave = new File("target/nvd-cve-backup", "dc.h2.db"); + +if (!datasDwl.exists() && datasSave.exists()){ + System.out.println("Load NVD-CVE from backup"); + FileUtils.copyFile(datasSave, datasDwl); +} From 1dbc183567f173769e05f113a59523f713433363 Mon Sep 17 00:00:00 2001 From: Jeremy Long Date: Thu, 22 Dec 2016 06:52:47 -0500 Subject: [PATCH 09/11] added check for failure --- .../it/618-aggregator-purge/postbuild.groovy | 29 +++++++++++++++++++ 1 file changed, 29 insertions(+) create mode 100644 dependency-check-maven/src/it/618-aggregator-purge/postbuild.groovy diff --git a/dependency-check-maven/src/it/618-aggregator-purge/postbuild.groovy b/dependency-check-maven/src/it/618-aggregator-purge/postbuild.groovy new file mode 100644 index 000000000..77ed8d9d6 --- /dev/null +++ b/dependency-check-maven/src/it/618-aggregator-purge/postbuild.groovy @@ -0,0 +1,29 @@ +/* + * This file is part of dependency-check-maven. + * + * 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) 2016 Jeremy Long. All Rights Reserved. + */ + +import java.nio.charset.Charset; +import org.apache.commons.io.FileUtils; +import org.apache.commons.lang.StringUtils; + + +// Analyse number of "Checking for updates" +String log = FileUtils.readFileToString(new File(basedir, "build.log"), Charset.defaultCharset().name()); +if (!StringUtils.contains(log, "Database file purged; local copy of the NVD has been removed")) { + System.out.println("The database was not purged."); + return false; +} From c33257d266b310c5d800e842c50e89595483bb65 Mon Sep 17 00:00:00 2001 From: Jeremy Long Date: Thu, 22 Dec 2016 06:53:35 -0500 Subject: [PATCH 10/11] addded synchronization - as this analyzer should only run synchronized --- .../dependencycheck/analyzer/DependencyBundlingAnalyzer.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 9bb34f403..8f8415dca 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 @@ -130,7 +130,7 @@ public class DependencyBundlingAnalyzer extends AbstractAnalyzer { * file. */ @Override - public void analyze(Dependency ignore, Engine engine) throws AnalysisException { + public synchronized void analyze(Dependency ignore, Engine engine) throws AnalysisException { if (!analyzed) { analyzed = true; final Set dependenciesToRemove = new HashSet(); From 60e661d3a4e8ffb0385859acdbe64cf75d0aae59 Mon Sep 17 00:00:00 2001 From: Jeremy Long Date: Thu, 22 Dec 2016 06:55:26 -0500 Subject: [PATCH 11/11] updated per issue #630 --- .../analyzer/DependencyMergingAnalyzer.java | 348 +----------------- ...rg.owasp.dependencycheck.analyzer.Analyzer | 1 + 2 files changed, 20 insertions(+), 329 deletions(-) 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 6e0968e3a..3ffbaeced 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 @@ -22,43 +22,26 @@ import java.util.HashSet; import java.util.Iterator; import java.util.ListIterator; import java.util.Set; -import java.util.regex.Matcher; -import java.util.regex.Pattern; import org.owasp.dependencycheck.Engine; import org.owasp.dependencycheck.analyzer.exception.AnalysisException; import org.owasp.dependencycheck.dependency.Dependency; -import org.owasp.dependencycheck.dependency.Identifier; -import org.owasp.dependencycheck.utils.DependencyVersion; -import org.owasp.dependencycheck.utils.DependencyVersionUtil; import org.slf4j.Logger; import org.slf4j.LoggerFactory; /** *

- * This analyzer ensures dependencies that should be grouped together, to remove - * excess noise from the report, are grouped. An example would be Spring, Spring - * Beans, Spring MVC, etc. If they are all for the same version and have the - * same relative path then these should be grouped into a single dependency - * under the core/main library.

- *

- * Note, this grouping only works on dependencies with identified CVE - * entries

+ * This analyzer will merge dependencies, created from different source, into a + * single dependency.

* * @author Jeremy Long */ -public class DependencyBundlingAnalyzer extends AbstractAnalyzer { - - /** - * The Logger. - */ - private static final Logger LOGGER = LoggerFactory.getLogger(DependencyBundlingAnalyzer.class); +public class DependencyMergingAnalyzer extends AbstractAnalyzer { // /** - * A pattern for obtaining the first part of a filename. + * The Logger. */ - private static final Pattern STARTING_TEXT_PATTERN = Pattern.compile("^[a-zA-Z0-9]*"); - + private static final Logger LOGGER = LoggerFactory.getLogger(DependencyMergingAnalyzer.class); /** * a flag indicating if this analyzer has run. This analyzer only runs once. */ @@ -80,11 +63,11 @@ public class DependencyBundlingAnalyzer extends AbstractAnalyzer { /** * The name of the analyzer. */ - private static final String ANALYZER_NAME = "Dependency Bundling Analyzer"; + private static final String ANALYZER_NAME = "Dependency Merging Analyzer"; /** * The phase that this analyzer is intended to run in. */ - private static final AnalysisPhase ANALYSIS_PHASE = AnalysisPhase.PRE_FINDING_ANALYSIS; + private static final AnalysisPhase ANALYSIS_PHASE = AnalysisPhase.POST_INFORMATION_COLLECTION; /** * Returns the name of the analyzer. @@ -105,7 +88,6 @@ public class DependencyBundlingAnalyzer extends AbstractAnalyzer { public AnalysisPhase getAnalysisPhase() { return ANALYSIS_PHASE; } - // /** * Does not support parallel processing as it only runs once and then @@ -118,11 +100,13 @@ public class DependencyBundlingAnalyzer extends AbstractAnalyzer { public boolean supportsParallelProcessing() { return false; } + // /** - * Analyzes a set of dependencies. If they have been found to have the same - * base path and the same set of identifiers they are likely related. The - * related dependencies are bundled into a single reportable item. + * Analyzes a set of dependencies. If they have been found to be the same + * dependency created by more multiple FileTypeAnalyzers (i.e. a gemspec + * dependency and a dependency from the Bundle Audit Analyzer. The + * dependencies are then merged into a single reportable item. * * @param ignore this analyzer ignores the dependency being analyzed * @param engine the engine that is scanning the dependencies @@ -130,7 +114,7 @@ public class DependencyBundlingAnalyzer extends AbstractAnalyzer { * file. */ @Override - public void analyze(Dependency ignore, Engine engine) throws AnalysisException { + public synchronized void analyze(Dependency ignore, Engine engine) throws AnalysisException { if (!analyzed) { analyzed = true; final Set dependenciesToRemove = new HashSet(); @@ -143,33 +127,7 @@ public class DependencyBundlingAnalyzer extends AbstractAnalyzer { while (subIterator.hasNext()) { final Dependency nextDependency = subIterator.next(); Dependency main = null; - if (hashesMatch(dependency, nextDependency) && !containedInWar(dependency.getFilePath()) - && !containedInWar(nextDependency.getFilePath())) { - if (firstPathIsShortest(dependency.getFilePath(), nextDependency.getFilePath())) { - mergeDependencies(dependency, nextDependency, dependenciesToRemove); - } else { - mergeDependencies(nextDependency, dependency, dependenciesToRemove); - break; //since we merged into the next dependency - skip forward to the next in mainIterator - } - } else if (isShadedJar(dependency, nextDependency)) { - if (dependency.getFileName().toLowerCase().endsWith("pom.xml")) { - mergeDependencies(nextDependency, dependency, dependenciesToRemove); - nextDependency.getRelatedDependencies().remove(dependency); - break; - } else { - mergeDependencies(dependency, nextDependency, dependenciesToRemove); - dependency.getRelatedDependencies().remove(nextDependency); - } - } else if (cpeIdentifiersMatch(dependency, nextDependency) - && hasSameBasePath(dependency, nextDependency) - && fileNameMatch(dependency, nextDependency)) { - if (isCore(dependency, nextDependency)) { - mergeDependencies(dependency, nextDependency, dependenciesToRemove); - } else { - mergeDependencies(nextDependency, dependency, dependenciesToRemove); - break; //since we merged into the next dependency - skip forward to the next in mainIterator - } - } else if ((main = getMainGemspecDependency(dependency, nextDependency)) != null) { + if ((main = getMainGemspecDependency(dependency, nextDependency)) != null) { if (main == dependency) { mergeDependencies(dependency, nextDependency, dependenciesToRemove); } else { @@ -205,6 +163,10 @@ public class DependencyBundlingAnalyzer extends AbstractAnalyzer { */ private void mergeDependencies(final Dependency dependency, final Dependency relatedDependency, final Set dependenciesToRemove) { dependency.addRelatedDependency(relatedDependency); + dependency.getVendorEvidence().getEvidence().addAll(relatedDependency.getVendorEvidence().getEvidence()); + dependency.getProductEvidence().getEvidence().addAll(relatedDependency.getProductEvidence().getEvidence()); + dependency.getVersionEvidence().getEvidence().addAll(relatedDependency.getVersionEvidence().getEvidence()); + final Iterator i = relatedDependency.getRelatedDependencies().iterator(); while (i.hasNext()) { dependency.addRelatedDependency(i.next()); @@ -216,147 +178,6 @@ public class DependencyBundlingAnalyzer extends AbstractAnalyzer { dependenciesToRemove.add(relatedDependency); } - /** - * Attempts to trim a maven repo to a common base path. This is typically - * [drive]\[repo_location]\repository\[path1]\[path2]. - * - * @param path the path to trim - * @return a string representing the base path. - */ - private String getBaseRepoPath(final String path) { - int pos = path.indexOf("repository" + File.separator) + 11; - if (pos < 0) { - return path; - } - int tmp = path.indexOf(File.separator, pos); - if (tmp <= 0) { - return path; - } - if (tmp > 0) { - pos = tmp + 1; - } - tmp = path.indexOf(File.separator, pos); - if (tmp > 0) { - pos = tmp + 1; - } - return path.substring(0, pos); - } - - /** - * Returns true if the file names (and version if it exists) of the two - * dependencies are sufficiently similar. - * - * @param dependency1 a dependency2 to compare - * @param dependency2 a dependency2 to compare - * @return true if the identifiers in the two supplied dependencies are - * equal - */ - private boolean fileNameMatch(Dependency dependency1, Dependency dependency2) { - if (dependency1 == null || dependency1.getFileName() == null - || dependency2 == null || dependency2.getFileName() == null) { - return false; - } - final String fileName1 = dependency1.getActualFile().getName(); - final String fileName2 = dependency2.getActualFile().getName(); - - //version check - final DependencyVersion version1 = DependencyVersionUtil.parseVersion(fileName1); - final DependencyVersion version2 = DependencyVersionUtil.parseVersion(fileName2); - if (version1 != null && version2 != null && !version1.equals(version2)) { - return false; - } - - //filename check - final Matcher match1 = STARTING_TEXT_PATTERN.matcher(fileName1); - final Matcher match2 = STARTING_TEXT_PATTERN.matcher(fileName2); - if (match1.find() && match2.find()) { - return match1.group().equals(match2.group()); - } - - return false; - } - - /** - * Returns true if the CPE identifiers in the two supplied dependencies are - * equal. - * - * @param dependency1 a dependency2 to compare - * @param dependency2 a dependency2 to compare - * @return true if the identifiers in the two supplied dependencies are - * equal - */ - private boolean cpeIdentifiersMatch(Dependency dependency1, Dependency dependency2) { - if (dependency1 == null || dependency1.getIdentifiers() == null - || dependency2 == null || dependency2.getIdentifiers() == null) { - return false; - } - boolean matches = false; - int cpeCount1 = 0; - int cpeCount2 = 0; - for (Identifier i : dependency1.getIdentifiers()) { - if ("cpe".equals(i.getType())) { - cpeCount1 += 1; - } - } - for (Identifier i : dependency2.getIdentifiers()) { - if ("cpe".equals(i.getType())) { - cpeCount2 += 1; - } - } - if (cpeCount1 > 0 && cpeCount1 == cpeCount2) { - for (Identifier i : dependency1.getIdentifiers()) { - if ("cpe".equals(i.getType())) { - matches |= dependency2.getIdentifiers().contains(i); - if (!matches) { - break; - } - } - } - } - LOGGER.debug("IdentifiersMatch={} ({}, {})", matches, dependency1.getFileName(), dependency2.getFileName()); - return matches; - } - - /** - * Determines if the two dependencies have the same base path. - * - * @param dependency1 a Dependency object - * @param dependency2 a Dependency object - * @return true if the base paths of the dependencies are identical - */ - private boolean hasSameBasePath(Dependency dependency1, Dependency dependency2) { - if (dependency1 == null || dependency2 == null) { - return false; - } - final File lFile = new File(dependency1.getFilePath()); - String left = lFile.getParent(); - final File rFile = new File(dependency2.getFilePath()); - String right = rFile.getParent(); - if (left == null) { - return right == null; - } else if (right == null) { - return false; - } - if (left.equalsIgnoreCase(right)) { - return true; - } - - if (left.matches(".*[/\\\\]repository[/\\\\].*") && right.matches(".*[/\\\\]repository[/\\\\].*")) { - left = getBaseRepoPath(left); - right = getBaseRepoPath(right); - } - if (left.equalsIgnoreCase(right)) { - return true; - } - //new code - for (Dependency child : dependency2.getRelatedDependencies()) { - if (hasSameBasePath(dependency1, child)) { - return true; - } - } - return false; - } - /** * Bundling Ruby gems that are identified from different .gemspec files but * denote the same package path. This happens when Ruby bundler installs an @@ -409,7 +230,7 @@ public class DependencyBundlingAnalyzer extends AbstractAnalyzer { /** * Bundling same swift dependencies with the same packagePath but identified - * by different analyzers. + * by different file type analyzers. * * @param dependency1 dependency to test * @param dependency2 dependency to test @@ -446,135 +267,4 @@ public class DependencyBundlingAnalyzer extends AbstractAnalyzer { } return null; } - - /** - * This is likely a very broken attempt at determining if the 'left' - * dependency is the 'core' library in comparison to the 'right' library. - * - * @param left the dependency to test - * @param right the dependency to test against - * @return a boolean indicating whether or not the left dependency should be - * considered the "core" version. - */ - boolean isCore(Dependency left, Dependency right) { - final String leftName = left.getFileName().toLowerCase(); - final String rightName = right.getFileName().toLowerCase(); - - final boolean returnVal; - if (!rightName.matches(".*\\.(tar|tgz|gz|zip|ear|war).+") && leftName.matches(".*\\.(tar|tgz|gz|zip|ear|war).+") - || rightName.contains("core") && !leftName.contains("core") - || rightName.contains("kernel") && !leftName.contains("kernel")) { - returnVal = false; - } else if (rightName.matches(".*\\.(tar|tgz|gz|zip|ear|war).+") && !leftName.matches(".*\\.(tar|tgz|gz|zip|ear|war).+") - || !rightName.contains("core") && leftName.contains("core") - || !rightName.contains("kernel") && leftName.contains("kernel")) { - returnVal = true; -// } else if (leftName.matches(".*struts2\\-core.*") && rightName.matches(".*xwork\\-core.*")) { -// returnVal = true; -// } else if (rightName.matches(".*struts2\\-core.*") && leftName.matches(".*xwork\\-core.*")) { -// returnVal = false; - } else { - /* - * considered splitting the names up and comparing the components, - * but decided that the file name length should be sufficient as the - * "core" component, if this follows a normal naming protocol should - * be shorter: - * axis2-saaj-1.4.1.jar - * axis2-1.4.1.jar <----- - * axis2-kernel-1.4.1.jar - */ - returnVal = leftName.length() <= rightName.length(); - } - LOGGER.debug("IsCore={} ({}, {})", returnVal, left.getFileName(), right.getFileName()); - return returnVal; - } - - /** - * Compares the SHA1 hashes of two dependencies to determine if they are - * equal. - * - * @param dependency1 a dependency object to compare - * @param dependency2 a dependency object to compare - * @return true if the sha1 hashes of the two dependencies match; otherwise - * false - */ - private boolean hashesMatch(Dependency dependency1, Dependency dependency2) { - if (dependency1 == null || dependency2 == null || dependency1.getSha1sum() == null || dependency2.getSha1sum() == null) { - return false; - } - return dependency1.getSha1sum().equals(dependency2.getSha1sum()); - } - - /** - * Determines if the jar is shaded and the created pom.xml identified the - * same CPE as the jar - if so, the pom.xml dependency should be removed. - * - * @param dependency a dependency to check - * @param nextDependency another dependency to check - * @return true if on of the dependencies is a pom.xml and the identifiers - * between the two collections match; otherwise false - */ - private boolean isShadedJar(Dependency dependency, Dependency nextDependency) { - final String mainName = dependency.getFileName().toLowerCase(); - final String nextName = nextDependency.getFileName().toLowerCase(); - if (mainName.endsWith(".jar") && nextName.endsWith("pom.xml")) { - return dependency.getIdentifiers().containsAll(nextDependency.getIdentifiers()); - } else if (nextName.endsWith(".jar") && mainName.endsWith("pom.xml")) { - return nextDependency.getIdentifiers().containsAll(dependency.getIdentifiers()); - } - return false; - } - - /** - * Determines which path is shortest; if path lengths are equal then we use - * compareTo of the string method to determine if the first path is smaller. - * - * @param left the first path to compare - * @param right the second path to compare - * @return true if the leftPath is the shortest; otherwise - * false - */ - protected boolean firstPathIsShortest(String left, String right) { - if (left.contains("dctemp")) { - return false; - } - final String leftPath = left.replace('\\', '/'); - final String rightPath = right.replace('\\', '/'); - - final int leftCount = countChar(leftPath, '/'); - final int rightCount = countChar(rightPath, '/'); - if (leftCount == rightCount) { - return leftPath.compareTo(rightPath) <= 0; - } else { - return leftCount < rightCount; - } - } - - /** - * Counts the number of times the character is present in the string. - * - * @param string the string to count the characters in - * @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) { - int count = 0; - final int max = string.length(); - for (int i = 0; i < max; i++) { - if (c == string.charAt(i)) { - count++; - } - } - return count; - } - - /** - * Checks if the given file path is contained within a war or ear file. - * - * @param filePath the file path to check - * @return true if the path contains '.war\' or '.ear\'. - */ - private boolean containedInWar(String filePath) { - return filePath == null ? false : filePath.matches(".*\\.(ear|war)[\\\\/].*"); - } } diff --git a/dependency-check-core/src/main/resources/META-INF/services/org.owasp.dependencycheck.analyzer.Analyzer b/dependency-check-core/src/main/resources/META-INF/services/org.owasp.dependencycheck.analyzer.Analyzer index 674d1d0f7..41d1e9ce1 100644 --- a/dependency-check-core/src/main/resources/META-INF/services/org.owasp.dependencycheck.analyzer.Analyzer +++ b/dependency-check-core/src/main/resources/META-INF/services/org.owasp.dependencycheck.analyzer.Analyzer @@ -6,6 +6,7 @@ org.owasp.dependencycheck.analyzer.CPEAnalyzer org.owasp.dependencycheck.analyzer.FalsePositiveAnalyzer org.owasp.dependencycheck.analyzer.CpeSuppressionAnalyzer org.owasp.dependencycheck.analyzer.DependencyBundlingAnalyzer +org.owasp.dependencycheck.analyzer.DependencyMergingAnalyzer org.owasp.dependencycheck.analyzer.NvdCveAnalyzer org.owasp.dependencycheck.analyzer.VulnerabilitySuppressionAnalyzer org.owasp.dependencycheck.analyzer.CentralAnalyzer