From 6a807bc002bbb0c869da9a8651935e6dc6abdf2d Mon Sep 17 00:00:00 2001 From: Jeremy Long Date: Wed, 25 May 2016 17:21:46 -0400 Subject: [PATCH] checkstyle/findbugs corrections --- .../org/slf4j/impl/StaticLoggerBinder.java | 2 +- .../analyzer/DependencyBundlingAnalyzer.java | 138 +++++++++------ .../analyzer/FileNameAnalyzer.java | 12 +- .../dependencycheck/analyzer/JarAnalyzer.java | 3 +- .../analyzer/PythonPackageAnalyzer.java | 25 +-- .../analyzer/RubyBundleAuditAnalyzer.java | 16 +- .../analyzer/RubyBundlerAnalyzer.java | 144 +++++++++------- .../analyzer/RubyGemspecAnalyzer.java | 163 +++++++++++------- .../dependency/Dependency.java | 88 ++++++---- .../owasp/dependencycheck/xml/pom/Model.java | 33 ++-- src/main/config/checkstyle-header.txt | 2 +- 11 files changed, 368 insertions(+), 258 deletions(-) diff --git a/dependency-check-ant/src/main/java/org/slf4j/impl/StaticLoggerBinder.java b/dependency-check-ant/src/main/java/org/slf4j/impl/StaticLoggerBinder.java index 92c87d3e1..3c96bb5b4 100644 --- a/dependency-check-ant/src/main/java/org/slf4j/impl/StaticLoggerBinder.java +++ b/dependency-check-ant/src/main/java/org/slf4j/impl/StaticLoggerBinder.java @@ -73,7 +73,7 @@ public class StaticLoggerBinder implements LoggerFactoryBinder { public static String REQUESTED_API_VERSION = "1.7.12"; // final //CSON: VisibilityModifier //CSON: StaticVariableName - + /** * The logger factory class string. */ 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 fd6911e6f..419eacb09 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 @@ -35,11 +35,14 @@ 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.

+ * 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

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

* * @author Jeremy Long */ @@ -92,12 +95,14 @@ public class DependencyBundlingAnalyzer extends AbstractAnalyzer implements Anal // /** - * 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 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. + * @throws AnalysisException is thrown if there is an error reading the JAR + * file. */ @Override public void analyze(Dependency ignore, Engine engine) throws AnalysisException { @@ -138,11 +143,11 @@ public class DependencyBundlingAnalyzer extends AbstractAnalyzer implements Anal mergeDependencies(nextDependency, dependency, dependenciesToRemove); break; //since we merged into the next dependency - skip forward to the next in mainIterator } - } else if ( isSameRubyGem(dependency, nextDependency) ) { - Dependency main = getMainGemspecDependency(dependency, nextDependency); - if (main == dependency) { - mergeDependencies(dependency, nextDependency, dependenciesToRemove); - } else { + } else if (isSameRubyGem(dependency, nextDependency)) { + final Dependency main = getMainGemspecDependency(dependency, nextDependency); + 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 } @@ -160,10 +165,11 @@ public class DependencyBundlingAnalyzer extends AbstractAnalyzer implements Anal * 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 + * @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); @@ -179,7 +185,8 @@ public class DependencyBundlingAnalyzer extends AbstractAnalyzer implements Anal } /** - * Attempts to trim a maven repo to a common base path. This is typically [drive]\[repo_location]\repository\[path1]\[path2]. + * 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. @@ -204,11 +211,13 @@ public class DependencyBundlingAnalyzer extends AbstractAnalyzer implements Anal } /** - * Returns true if the file names (and version if it exists) of the two dependencies are sufficiently similar. + * 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 + * @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 @@ -236,11 +245,13 @@ public class DependencyBundlingAnalyzer extends AbstractAnalyzer implements Anal } /** - * Returns true if the CPE identifiers in the two supplied dependencies are equal. + * 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 + * @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 @@ -310,37 +321,53 @@ public class DependencyBundlingAnalyzer extends AbstractAnalyzer implements Anal } 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 app's dependencies by running "bundle install". + * 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) { + if (dependency1 == null || dependency2 == null + || !dependency1.getFileName().endsWith(".gemspec") + || !dependency2.getFileName().endsWith(".gemspec") + || dependency1.getPackagePath() == null + || dependency2.getPackagePath() == null) { return false; } - if (dependency1.getPackagePath().equalsIgnoreCase(dependency2.getPackagePath())) - return true; + if (dependency1.getPackagePath().equalsIgnoreCase(dependency2.getPackagePath())) { + return true; + } - return false; + return false; } - + /** - * 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. + * 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(); - File left = lFile.getParentFile(); + if (isSameRubyGem(dependency1, dependency2)) { + final File lFile = dependency1.getActualFile(); + final File left = lFile.getParentFile(); if (left != null && left.getName().equalsIgnoreCase("specifications")) { return dependency1; } @@ -350,12 +377,13 @@ public class DependencyBundlingAnalyzer extends AbstractAnalyzer implements Anal } /** - * This is likely a very broken attempt at determining if the 'left' dependency is the 'core' library in comparison to the - * 'right' library. + * 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. + * @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(); @@ -391,11 +419,13 @@ public class DependencyBundlingAnalyzer extends AbstractAnalyzer implements Anal } /** - * Compares the SHA1 hashes of two dependencies to determine if they are equal. + * 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 + * @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) { @@ -405,12 +435,13 @@ public class DependencyBundlingAnalyzer extends AbstractAnalyzer implements Anal } /** - * 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. + * 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 + * @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(); @@ -424,12 +455,13 @@ public class DependencyBundlingAnalyzer extends AbstractAnalyzer implements Anal } /** - * 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. + * 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 + * @return true if the leftPath is the shortest; otherwise + * false */ protected boolean firstPathIsShortest(String left, String right) { final String leftPath = left.replace('\\', '/'); diff --git a/dependency-check-core/src/main/java/org/owasp/dependencycheck/analyzer/FileNameAnalyzer.java b/dependency-check-core/src/main/java/org/owasp/dependencycheck/analyzer/FileNameAnalyzer.java index d8edade74..fcaaeb102 100644 --- a/dependency-check-core/src/main/java/org/owasp/dependencycheck/analyzer/FileNameAnalyzer.java +++ b/dependency-check-core/src/main/java/org/owasp/dependencycheck/analyzer/FileNameAnalyzer.java @@ -70,7 +70,7 @@ public class FileNameAnalyzer extends AbstractAnalyzer implements Analyzer { /** * Python init files */ - private static final NameFileFilter IGNORED_FILES = new NameFileFilter(new String[] { + private static final NameFileFilter IGNORED_FILES = new NameFileFilter(new String[]{ "__init__.py", "__init__.pyc", "__init__.pyo", @@ -81,7 +81,8 @@ public class FileNameAnalyzer extends AbstractAnalyzer implements Analyzer { * * @param dependency the dependency to analyze. * @param engine the engine that is scanning the dependencies - * @throws AnalysisException is thrown if there is an error reading the JAR file. + * @throws AnalysisException is thrown if there is an error reading the JAR + * file. */ @Override public void analyze(Dependency dependency, Engine engine) throws AnalysisException { @@ -107,13 +108,6 @@ public class FileNameAnalyzer extends AbstractAnalyzer implements Analyzer { fileName, Confidence.MEDIUM); } - //add as vendor and product evidence -// if (fileName.contains("-")) { -// dependency.getProductEvidence().addEvidence("file", "name", -// fileName, Confidence.HIGHEST); -// dependency.getVendorEvidence().addEvidence("file", "name", -// fileName, Confidence.HIGHEST); -// } else if (!IGNORED_FILES.accept(f)) { dependency.getProductEvidence().addEvidence("file", "name", fileName, Confidence.HIGH); 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 4ffca8773..0423c5eeb 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 @@ -29,7 +29,6 @@ import java.util.ArrayList; import java.util.Collections; import java.util.Enumeration; import java.util.HashMap; -import java.util.Iterator; import java.util.List; import java.util.Map; import java.util.Map.Entry; @@ -578,7 +577,7 @@ public class JarAnalyzer extends AbstractFileTypeAnalyzer { addMatchingValues(classes, trimmedDescription, dependency.getProductEvidence()); } - String projectURL = pom.getProjectURL(); + final String projectURL = pom.getProjectURL(); if (projectURL != null && !projectURL.trim().isEmpty()) { dependency.getVendorEvidence().addEvidence("pom", "url", projectURL, Confidence.HIGHEST); } diff --git a/dependency-check-core/src/main/java/org/owasp/dependencycheck/analyzer/PythonPackageAnalyzer.java b/dependency-check-core/src/main/java/org/owasp/dependencycheck/analyzer/PythonPackageAnalyzer.java index 527892bce..9f12e0e95 100644 --- a/dependency-check-core/src/main/java/org/owasp/dependencycheck/analyzer/PythonPackageAnalyzer.java +++ b/dependency-check-core/src/main/java/org/owasp/dependencycheck/analyzer/PythonPackageAnalyzer.java @@ -39,7 +39,8 @@ import java.util.regex.Matcher; import java.util.regex.Pattern; /** - * Used to analyze a Python package, and collect information that can be used to determine the associated CPE. + * Used to analyze a Python package, and collect information that can be used to + * determine the associated CPE. * * @author Dale Visser */ @@ -166,7 +167,8 @@ public class PythonPackageAnalyzer extends AbstractFileTypeAnalyzer { * * @param dependency the dependency being analyzed * @param engine the engine being used to perform the scan - * @throws AnalysisException thrown if there is an unrecoverable error analyzing the dependency + * @throws AnalysisException thrown if there is an unrecoverable error + * analyzing the dependency */ @Override protected void analyzeFileType(Dependency dependency, Engine engine) @@ -175,21 +177,20 @@ public class PythonPackageAnalyzer extends AbstractFileTypeAnalyzer { final File parent = file.getParentFile(); final String parentName = parent.getName(); if (INIT_PY_FILTER.accept(file)) { - //by definition, the containing folder of __init__.py is considered the package, even the file is empty: - //"The __init__.py files are required to make Python treat the directories as containing packages" - //see section "6.4 Packages" from https://docs.python.org/2/tutorial/modules.html; + //by definition, the containing folder of __init__.py is considered the package, even the file is empty: + //"The __init__.py files are required to make Python treat the directories as containing packages" + //see section "6.4 Packages" from https://docs.python.org/2/tutorial/modules.html; dependency.setDisplayFileName(parentName + "/__init__.py"); dependency.getProductEvidence().addEvidence(file.getName(), "PackageName", parentName, Confidence.HIGHEST); - + final File[] fileList = parent.listFiles(PY_FILTER); if (fileList != null) { for (final File sourceFile : fileList) { analyzeFileContents(dependency, sourceFile); } } - } - else { + } else { // copy, alter and set in case some other thread is iterating over final List dependencies = new ArrayList( engine.getDependencies()); @@ -199,8 +200,9 @@ public class PythonPackageAnalyzer extends AbstractFileTypeAnalyzer { } /** - * This should gather information from leading docstrings, file comments, and assignments to __version__, __title__, - * __summary__, __uri__, __url__, __home*page__, __author__, and their all caps equivalents. + * This should gather information from leading docstrings, file comments, + * and assignments to __version__, __title__, __summary__, __uri__, __url__, + * __home*page__, __author__, and their all caps equivalents. * * @param dependency the dependency being analyzed * @param file the file name to analyze @@ -291,7 +293,8 @@ public class PythonPackageAnalyzer extends AbstractFileTypeAnalyzer { } /** - * Gather evidence from a Python source file using the given string assignment regex pattern. + * Gather evidence from a Python source file using the given string + * assignment regex pattern. * * @param pattern to scan contents with * @param contents of Python source file diff --git a/dependency-check-core/src/main/java/org/owasp/dependencycheck/analyzer/RubyBundleAuditAnalyzer.java b/dependency-check-core/src/main/java/org/owasp/dependencycheck/analyzer/RubyBundleAuditAnalyzer.java index ea26ce227..003f4ed26 100644 --- a/dependency-check-core/src/main/java/org/owasp/dependencycheck/analyzer/RubyBundleAuditAnalyzer.java +++ b/dependency-check-core/src/main/java/org/owasp/dependencycheck/analyzer/RubyBundleAuditAnalyzer.java @@ -61,8 +61,7 @@ public class RubyBundleAuditAnalyzer extends AbstractFileTypeAnalyzer { */ private static final AnalysisPhase ANALYSIS_PHASE = AnalysisPhase.PRE_INFORMATION_COLLECTION; - private static final FileFilter FILTER - = FileFilterBuilder.newInstance().addFilenames("Gemfile.lock").build(); + private static final FileFilter FILTER = FileFilterBuilder.newInstance().addFilenames("Gemfile.lock").build(); public static final String NAME = "Name: "; public static final String VERSION = "Version: "; public static final String ADVISORY = "Advisory: "; @@ -81,7 +80,9 @@ public class RubyBundleAuditAnalyzer extends AbstractFileTypeAnalyzer { /** * Launch bundle-audit. * + * @param folder directory that contains bundle audit * @return a handle to the process + * @throws AnalysisException thrown when there is an issue launching bundle audit */ private Process launchBundleAudit(File folder) throws AnalysisException { if (!folder.isDirectory()) { @@ -131,7 +132,7 @@ public class RubyBundleAuditAnalyzer extends AbstractFileTypeAnalyzer { throw ae; } - int exitValue = process.waitFor(); + final int exitValue = process.waitFor(); if (0 == exitValue) { LOGGER.warn("Unexpected exit code from bundle-audit process. Disabling {}: {}", ANALYZER_NAME, exitValue); setEnabled(false); @@ -236,7 +237,7 @@ public class RubyBundleAuditAnalyzer extends AbstractFileTypeAnalyzer { try { errReader = new BufferedReader(new InputStreamReader(process.getErrorStream(), "UTF-8")); while (errReader.ready()) { - String error = errReader.readLine(); + final String error = errReader.readLine(); LOGGER.warn(error); } rdr = new BufferedReader(new InputStreamReader(process.getInputStream(), "UTF-8")); @@ -284,7 +285,7 @@ public class RubyBundleAuditAnalyzer extends AbstractFileTypeAnalyzer { dependency = map.get(gem); LOGGER.debug(String.format("bundle-audit (%s): %s", parentName, nextLine)); } else if (nextLine.startsWith(VERSION)) { - vulnerability = createVulnerability(parentName, dependency, vulnerability, gem, nextLine); + vulnerability = createVulnerability(parentName, dependency, gem, nextLine); } else if (nextLine.startsWith(ADVISORY)) { setVulnerabilityName(parentName, dependency, vulnerability, nextLine); } else if (nextLine.startsWith(CRITICALITY)) { @@ -318,7 +319,7 @@ public class RubyBundleAuditAnalyzer extends AbstractFileTypeAnalyzer { private void addReferenceToVulnerability(String parentName, Vulnerability vulnerability, String nextLine) { final String url = nextLine.substring(("URL: ").length()); if (null != vulnerability) { - Reference ref = new Reference(); + final Reference ref = new Reference(); ref.setName(vulnerability.getName()); ref.setSource("bundle-audit"); ref.setUrl(url); @@ -351,7 +352,8 @@ public class RubyBundleAuditAnalyzer extends AbstractFileTypeAnalyzer { LOGGER.debug(String.format("bundle-audit (%s): %s", parentName, nextLine)); } - private Vulnerability createVulnerability(String parentName, Dependency dependency, Vulnerability vulnerability, String gem, String nextLine) { + private Vulnerability createVulnerability(String parentName, Dependency dependency, String gem, String nextLine) { + Vulnerability vulnerability = null; if (null != dependency) { final String version = nextLine.substring(VERSION.length()); dependency.getVersionEvidence().addEvidence( diff --git a/dependency-check-core/src/main/java/org/owasp/dependencycheck/analyzer/RubyBundlerAnalyzer.java b/dependency-check-core/src/main/java/org/owasp/dependencycheck/analyzer/RubyBundlerAnalyzer.java index 05bfddb89..bdaafd443 100644 --- a/dependency-check-core/src/main/java/org/owasp/dependencycheck/analyzer/RubyBundlerAnalyzer.java +++ b/dependency-check-core/src/main/java/org/owasp/dependencycheck/analyzer/RubyBundlerAnalyzer.java @@ -25,20 +25,23 @@ import org.owasp.dependencycheck.analyzer.exception.AnalysisException; import org.owasp.dependencycheck.dependency.Dependency; /** - * This analyzer accepts the fully resolved .gemspec created by the Ruby bundler (http://bundler.io) - * for better evidence results. It also tries to resolve the dependency packagePath - * to where the gem is actually installed. Then during {@link AnalysisPhase.PRE_FINDING_ANALYSIS} - * {@link DependencyBundlingAnalyzer} will merge two .gemspec dependencies together if - * Dependency.getPackagePath() are the same. - * - * Ruby bundler creates new .gemspec files under a folder called "specifications" at deploy time, - * in addition to the original .gemspec files from source. The bundler generated - * .gemspec files always contain fully resolved attributes thus provide more accurate - * evidences, whereas the original .gemspec from source often contain variables for attributes - * that can't be used for evidences. - * - * Note this analyzer share the same {@link Settings.KEYS.ANALYZER_RUBY_GEMSPEC_ENABLED} as - * {@link RubyGemspecAnalyzer}, so it will enabled/disabled with {@link RubyGemspecAnalyzer}. + * This analyzer accepts the fully resolved .gemspec created by the Ruby bundler + * (http://bundler.io) for better evidence results. It also tries to resolve the + * dependency packagePath to where the gem is actually installed. Then during {@link AnalysisPhase.PRE_FINDING_ANALYSIS} + * {@link DependencyBundlingAnalyzer} will merge two .gemspec dependencies + * together if Dependency.getPackagePath() are the same. + * + * Ruby bundler creates new .gemspec files under a folder called + * "specifications" at deploy time, in addition to the original .gemspec files + * from source. The bundler generated .gemspec files always contain fully + * resolved attributes thus provide more accurate evidences, whereas the + * original .gemspec from source often contain variables for attributes that + * can't be used for evidences. + * + * Note this analyzer share the same + * {@link Settings.KEYS.ANALYZER_RUBY_GEMSPEC_ENABLED} as + * {@link RubyGemspecAnalyzer}, so it will enabled/disabled with + * {@link RubyGemspecAnalyzer}. * * @author Bianca Jiang (biancajiang@gmail.com) */ @@ -48,12 +51,16 @@ public class RubyBundlerAnalyzer extends RubyGemspecAnalyzer { * The name of the analyzer. */ private static final String ANALYZER_NAME = "Ruby Bundler Analyzer"; - - //Folder name that contains .gemspec files created by "bundle install" - private static final String SPECIFICATIONS = "specifications"; - - //Folder name that contains the gems by "bundle install" - private static final String GEMS = "gems"; + + /** + * Folder name that contains .gemspec files created by "bundle install" + */ + private static final String SPECIFICATIONS = "specifications"; + + /** + * Folder name that contains the gems by "bundle install" + */ + private static final String GEMS = "gems"; /** * Returns the name of the analyzer. @@ -66,61 +73,66 @@ public class RubyBundlerAnalyzer extends RubyGemspecAnalyzer { } /** - * Only accept *.gemspec files generated by "bundle install --deployment" under "specifications" folder. + * Only accept *.gemspec files generated by "bundle install --deployment" + * under "specifications" folder. + * + * @param pathname the path name to test + * @return true if the analyzer can process the given file; otherwise false */ - @Override + @Override public boolean accept(File pathname) { - + boolean accepted = super.accept(pathname); - if(accepted == true) { - File parentDir = pathname.getParentFile(); - accepted = parentDir != null && parentDir.getName().equals(SPECIFICATIONS); + if (accepted) { + final File parentDir = pathname.getParentFile(); + accepted = parentDir != null && parentDir.getName().equals(SPECIFICATIONS); } - + return accepted; } - - @Override + + @Override protected void analyzeFileType(Dependency dependency, Engine engine) throws AnalysisException { super.analyzeFileType(dependency, engine); - + //find the corresponding gem folder for this .gemspec stub by "bundle install --deployment" - File gemspecFile = dependency.getActualFile(); - String gemFileName = gemspecFile.getName(); + final File gemspecFile = dependency.getActualFile(); + final String gemFileName = gemspecFile.getName(); final String gemName = gemFileName.substring(0, gemFileName.lastIndexOf(".gemspec")); - File specificationsDir = gemspecFile.getParentFile(); - if(specificationsDir != null && specificationsDir.getName().equals(SPECIFICATIONS) && specificationsDir.exists()) { - File parentDir = specificationsDir.getParentFile(); - if(parentDir != null && parentDir.exists()) { - File gemsDir = new File(parentDir, GEMS); - if(gemsDir != null && gemsDir.exists()) { - File[] matchingFiles = gemsDir.listFiles(new FilenameFilter() { - public boolean accept(File dir, String name) { - return name.equals(gemName); - } - }); - - if(matchingFiles.length > 0) { - String gemPath = matchingFiles[0].getAbsolutePath(); - if(dependency.getActualFilePath().equals(dependency.getFilePath())) { - if(gemPath != null) - dependency.setPackagePath(gemPath); - } else { - //.gemspec's actualFilePath and filePath are different when it's from a compressed file - //in which case actualFilePath is the temp directory used by decompression. - //packagePath should use the filePath of the identified gem file in "gems" folder - File gemspecStub = new File(dependency.getFilePath()); - File specDir = gemspecStub.getParentFile(); - if(specDir != null && specDir.getName().equals(SPECIFICATIONS)) { - File gemsDir2 = new File(specDir.getParentFile(), GEMS); - File packageDir = new File(gemsDir2, gemName); - dependency.setPackagePath(packageDir.getAbsolutePath()); - } - } - } - } - } - } - } + final File specificationsDir = gemspecFile.getParentFile(); + if (specificationsDir != null && specificationsDir.getName().equals(SPECIFICATIONS) && specificationsDir.exists()) { + final File parentDir = specificationsDir.getParentFile(); + if (parentDir != null && parentDir.exists()) { + final File gemsDir = new File(parentDir, GEMS); + if (gemsDir.exists()) { + final File[] matchingFiles = gemsDir.listFiles(new FilenameFilter() { + public boolean accept(File dir, String name) { + return name.equals(gemName); + } + }); + + if (matchingFiles != null && matchingFiles.length > 0) { + final String gemPath = matchingFiles[0].getAbsolutePath(); + if (dependency.getActualFilePath().equals(dependency.getFilePath())) { + if (gemPath != null) { + dependency.setPackagePath(gemPath); + } + } else { + //.gemspec's actualFilePath and filePath are different when it's from a compressed file + //in which case actualFilePath is the temp directory used by decompression. + //packagePath should use the filePath of the identified gem file in "gems" folder + final File gemspecStub = new File(dependency.getFilePath()); + final File specDir = gemspecStub.getParentFile(); + if (specDir != null && specDir.getName().equals(SPECIFICATIONS)) { + final File gemsDir2 = new File(specDir.getParentFile(), GEMS); + final File packageDir = new File(gemsDir2, gemName); + dependency.setPackagePath(packageDir.getAbsolutePath()); + } + } + } + } + } + } + } } diff --git a/dependency-check-core/src/main/java/org/owasp/dependencycheck/analyzer/RubyGemspecAnalyzer.java b/dependency-check-core/src/main/java/org/owasp/dependencycheck/analyzer/RubyGemspecAnalyzer.java index 8bad7cab2..2ee96b9db 100644 --- a/dependency-check-core/src/main/java/org/owasp/dependencycheck/analyzer/RubyGemspecAnalyzer.java +++ b/dependency-check-core/src/main/java/org/owasp/dependencycheck/analyzer/RubyGemspecAnalyzer.java @@ -34,15 +34,22 @@ import org.owasp.dependencycheck.dependency.Dependency; import org.owasp.dependencycheck.dependency.EvidenceCollection; import org.owasp.dependencycheck.utils.FileFilterBuilder; import org.owasp.dependencycheck.utils.Settings; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; /** - * Used to analyze Ruby Gem specifications and collect information that can be used to determine the associated CPE. Regular - * expressions are used to parse the well-defined Ruby syntax that forms the specification. + * Used to analyze Ruby Gem specifications and collect information that can be + * used to determine the associated CPE. Regular expressions are used to parse + * the well-defined Ruby syntax that forms the specification. * * @author Dale Visser */ public class RubyGemspecAnalyzer extends AbstractFileTypeAnalyzer { + /** + * The logger. + */ + private static final Logger LOGGER = LoggerFactory.getLogger(RubyGemspecAnalyzer.class); /** * The name of the analyzer. */ @@ -53,13 +60,22 @@ public class RubyGemspecAnalyzer extends AbstractFileTypeAnalyzer { */ private static final AnalysisPhase ANALYSIS_PHASE = AnalysisPhase.INFORMATION_COLLECTION; + /** + * The gemspec file extension. + */ private static final String GEMSPEC = "gemspec"; - private static final FileFilter FILTER - = FileFilterBuilder.newInstance().addExtensions(GEMSPEC).build(); - //TODO: support Rakefile - //= FileFilterBuilder.newInstance().addExtensions(GEMSPEC).addFilenames("Rakefile").build(); + /** + * The file filter containing the list of file extensions that can be + * analyzed. + */ + private static final FileFilter FILTER = FileFilterBuilder.newInstance().addExtensions(GEMSPEC).build(); + //TODO: support Rakefile + //= FileFilterBuilder.newInstance().addExtensions(GEMSPEC).addFilenames("Rakefile").build(); + /** + * The name of the version file. + */ private static final String VERSION_FILE_NAME = "VERSION"; /** @@ -96,7 +112,8 @@ public class RubyGemspecAnalyzer extends AbstractFileTypeAnalyzer { } /** - * Returns the key used in the properties file to reference the analyzer's enabled property. + * Returns the key used in the properties file to reference the analyzer's + * enabled property. * * @return the analyzer's enabled property setting key */ @@ -108,8 +125,7 @@ public class RubyGemspecAnalyzer extends AbstractFileTypeAnalyzer { /** * The capture group #1 is the block variable. */ - private static final Pattern GEMSPEC_BLOCK_INIT - = Pattern.compile("Gem::Specification\\.new\\s+?do\\s+?\\|(.+?)\\|"); + private static final Pattern GEMSPEC_BLOCK_INIT = Pattern.compile("Gem::Specification\\.new\\s+?do\\s+?\\|(.+?)\\|"); @Override protected void analyzeFileType(Dependency dependency, Engine engine) @@ -125,7 +141,7 @@ public class RubyGemspecAnalyzer extends AbstractFileTypeAnalyzer { if (matcher.find()) { contents = contents.substring(matcher.end()); final String blockVariable = matcher.group(1); - + final EvidenceCollection vendor = dependency.getVendorEvidence(); final EvidenceCollection product = dependency.getProductEvidence(); final String name = addStringEvidence(product, contents, blockVariable, "name", "name", Confidence.HIGHEST); @@ -138,71 +154,90 @@ public class RubyGemspecAnalyzer extends AbstractFileTypeAnalyzer { addStringEvidence(vendor, contents, blockVariable, "email", "emails?", Confidence.MEDIUM); addStringEvidence(vendor, contents, blockVariable, "homepage", "homepage", Confidence.HIGHEST); addStringEvidence(vendor, contents, blockVariable, "license", "licen[cs]es?", Confidence.HIGHEST); - - String value = addStringEvidence(dependency.getVersionEvidence(), contents, blockVariable, "version", "version", Confidence.HIGHEST); - if(value.length() < 1) - addEvidenceFromVersionFile(dependency.getActualFile(), dependency.getVersionEvidence()); + + final String value = addStringEvidence(dependency.getVersionEvidence(), contents, + blockVariable, "version", "version", Confidence.HIGHEST); + if (value.length() < 1) { + addEvidenceFromVersionFile(dependency.getActualFile(), dependency.getVersionEvidence()); + } } - + setPackagePath(dependency); } + /** + * Adds the specified evidence to the given evidence collection. + * + * @param evidences the collection to add the evidence to + * @param contents the evidence contents + * @param blockVariable the variable + * @param field the field + * @param fieldPattern the field pattern + * @param confidence the confidence of the evidence + * @return the evidence string value added + */ private String addStringEvidence(EvidenceCollection evidences, String contents, String blockVariable, String field, String fieldPattern, Confidence confidence) { String value = ""; - - //capture array value between [ ] - final Matcher arrayMatcher = Pattern.compile( + + //capture array value between [ ] + final Matcher arrayMatcher = Pattern.compile( String.format("\\s*?%s\\.%s\\s*?=\\s*?\\[(.*?)\\]", blockVariable, fieldPattern), Pattern.CASE_INSENSITIVE).matcher(contents); - if(arrayMatcher.find()) { - String arrayValue = arrayMatcher.group(1); - value = arrayValue.replaceAll("['\"]", "").trim(); //strip quotes - } - //capture single value between quotes - else { - final Matcher matcher = Pattern.compile( - String.format("\\s*?%s\\.%s\\s*?=\\s*?(['\"])(.*?)\\1", blockVariable, fieldPattern), Pattern.CASE_INSENSITIVE).matcher(contents); - if (matcher.find()) { - value = matcher.group(2); - } - } - if(value.length() > 0) - evidences.addEvidence(GEMSPEC, field, value, confidence); - + if (arrayMatcher.find()) { + final String arrayValue = arrayMatcher.group(1); + value = arrayValue.replaceAll("['\"]", "").trim(); //strip quotes + } else { //capture single value between quotes + final Matcher matcher = Pattern.compile( + String.format("\\s*?%s\\.%s\\s*?=\\s*?(['\"])(.*?)\\1", blockVariable, fieldPattern), Pattern.CASE_INSENSITIVE).matcher(contents); + if (matcher.find()) { + value = matcher.group(2); + } + } + if (value.length() > 0) { + evidences.addEvidence(GEMSPEC, field, value, confidence); + } + return value; } - - private String addEvidenceFromVersionFile(File dependencyFile, EvidenceCollection versionEvidences) { - String value = null; - File parentDir = dependencyFile.getParentFile(); - if(parentDir != null) { - File[] matchingFiles = parentDir.listFiles(new FilenameFilter() { - public boolean accept(File dir, String name) { - return name.contains(VERSION_FILE_NAME); - } - }); - - for(int i = 0; i < matchingFiles.length; i++) { - try { - List lines = FileUtils.readLines(matchingFiles[i]); - if(lines.size() == 1) { //TODO other checking? - value = lines.get(0).trim(); - versionEvidences.addEvidence(GEMSPEC, "version", value, Confidence.HIGH); - } - - } catch (IOException e) { - e.printStackTrace(); - } - } - } - - return value; + + /** + * Adds evidence from the version file. + * + * @param dependencyFile the dependency being analyzed + * @param versionEvidences the version evidence + */ + private void addEvidenceFromVersionFile(File dependencyFile, EvidenceCollection versionEvidences) { + final File parentDir = dependencyFile.getParentFile(); + if (parentDir != null) { + final File[] matchingFiles = parentDir.listFiles(new FilenameFilter() { + public boolean accept(File dir, String name) { + return name.contains(VERSION_FILE_NAME); + } + }); + for (File f : matchingFiles) { + try { + final List lines = FileUtils.readLines(f, Charset.defaultCharset()); + if (lines.size() == 1) { //TODO other checking? + final String value = lines.get(0).trim(); + versionEvidences.addEvidence(GEMSPEC, "version", value, Confidence.HIGH); + } + } catch (IOException e) { + LOGGER.debug("Error reading gemspec", e); + } + } + } } - + + /** + * Sets the package path on the dependency. + * + * @param dep the dependency to alter + */ private void setPackagePath(Dependency dep) { - File file = new File(dep.getFilePath()); - String parent = file.getParent(); - if(parent != null) - dep.setPackagePath(parent); + final File file = new File(dep.getFilePath()); + final String parent = file.getParent(); + if (parent != null) { + dep.setPackagePath(parent); + } } } 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 37e6c5bb1..5e0d03eed 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 @@ -36,9 +36,10 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; /** - * A program dependency. This object is one of the core components within DependencyCheck. It is used to collect information about - * the dependency in the form of evidence. The Evidence is then used to determine if there are any known, published, - * vulnerabilities associated with the program dependency. + * A program dependency. This object is one of the core components within + * DependencyCheck. It is used to collect information about the dependency in + * the form of evidence. The Evidence is then used to determine if there are any + * known, published, vulnerabilities associated with the program dependency. * * @author Jeremy Long */ @@ -72,17 +73,31 @@ public class Dependency implements Serializable, Comparable { * The file name of the dependency. */ private String fileName; - + + /** + * The package path. + */ private String packagePath; + + /** + * Returns the package path. + * + * @return the package path + */ public String getPackagePath() { - return packagePath; - } + return packagePath; + } - public void setPackagePath(String packagePath) { - this.packagePath = packagePath; - } + /** + * Sets the package path. + * + * @param packagePath the package path + */ + public void setPackagePath(String packagePath) { + this.packagePath = packagePath; + } - /** + /** * The md5 hash of the dependency. */ private String md5sum; @@ -144,10 +159,12 @@ public class Dependency implements Serializable, Comparable { } /** - * Returns the file name of the dependency with the backslash escaped for use in JavaScript. This is a complete hack as I - * could not get the replace to work in the template itself. + * Returns the file name of the dependency with the backslash escaped for + * use in JavaScript. This is a complete hack as I could not get the replace + * to work in the template itself. * - * @return the file name of the dependency with the backslash escaped for use in JavaScript + * @return the file name of the dependency with the backslash escaped for + * use in JavaScript */ public String getFileNameForJavaScript() { return this.fileName.replace("\\", "\\\\"); @@ -199,9 +216,9 @@ public class Dependency implements Serializable, Comparable { * @param filePath the file path of the dependency */ public void setFilePath(String filePath) { - if(this.packagePath == null || this.packagePath.equals(this.filePath)) - this.packagePath = filePath; - + if (this.packagePath == null || this.packagePath.equals(this.filePath)) { + this.packagePath = filePath; + } this.filePath = filePath; } @@ -220,7 +237,8 @@ public class Dependency implements Serializable, Comparable { } /** - * Returns the file name to display in reports; if no display file name has been set it will default to the actual file name. + * Returns the file name to display in reports; if no display file name has + * been set it will default to the actual file name. * * @return the file name to display */ @@ -235,8 +253,9 @@ public class Dependency implements Serializable, Comparable { *

* Gets the file path of the dependency.

*

- * NOTE: This may not be the actual path of the file on disk. The actual path of the file on disk can be obtained via - * the getActualFilePath().

+ * NOTE: This may not be the actual path of the file on disk. The + * actual path of the file on disk can be obtained via the + * getActualFilePath().

* * @return the file path of the dependency */ @@ -299,7 +318,8 @@ public class Dependency implements Serializable, Comparable { } /** - * Adds an entry to the list of detected Identifiers for the dependency file. + * Adds an entry to the list of detected Identifiers for the dependency + * file. * * @param type the type of identifier (such as CPE) * @param value the value of the identifier @@ -311,7 +331,8 @@ public class Dependency implements Serializable, Comparable { } /** - * Adds an entry to the list of detected Identifiers for the dependency file. + * Adds an entry to the list of detected Identifiers for the dependency + * file. * * @param type the type of identifier (such as CPE) * @param value the value of the identifier @@ -362,7 +383,8 @@ public class Dependency implements Serializable, Comparable { } /** - * Adds an entry to the list of detected Identifiers for the dependency file. + * Adds an entry to the list of detected Identifiers for the dependency + * file. * * @param identifier the identifier to add */ @@ -594,8 +616,9 @@ public class Dependency implements Serializable, Comparable { private Set relatedDependencies = new TreeSet(); /** - * Get the value of {@link #relatedDependencies}. This field is used to collect other dependencies which really represent the - * same dependency, and may be presented as one item in reports. + * Get the value of {@link #relatedDependencies}. This field is used to + * collect other dependencies which really represent the same dependency, + * and may be presented as one item in reports. * * @return the value of relatedDependencies */ @@ -654,9 +677,11 @@ public class Dependency implements Serializable, Comparable { } /** - * 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. 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. * * @param dependency a reference to the related dependency */ @@ -706,7 +731,8 @@ public class Dependency implements Serializable, Comparable { } /** - * Implementation of the Comparable<Dependency> interface. The comparison is solely based on the file path. + * 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 @@ -776,12 +802,14 @@ public class Dependency implements Serializable, Comparable { } /** - * Standard toString() implementation showing the filename, actualFilePath, and filePath. + * Standard toString() implementation showing the filename, actualFilePath, + * and filePath. * * @return the string representation of the file */ @Override public String toString() { - return "Dependency{ fileName='" + fileName + "', actualFilePath='" + actualFilePath + "', filePath='" + filePath + "', packagePath='" + packagePath + "'}"; + return "Dependency{ fileName='" + fileName + "', actualFilePath='" + actualFilePath + + "', filePath='" + filePath + "', packagePath='" + packagePath + "'}"; } } diff --git a/dependency-check-core/src/main/java/org/owasp/dependencycheck/xml/pom/Model.java b/dependency-check-core/src/main/java/org/owasp/dependencycheck/xml/pom/Model.java index 2cecff585..1a78294f7 100644 --- a/dependency-check-core/src/main/java/org/owasp/dependencycheck/xml/pom/Model.java +++ b/dependency-check-core/src/main/java/org/owasp/dependencycheck/xml/pom/Model.java @@ -260,7 +260,7 @@ public class Model { public void addLicense(License license) { licenses.add(license); } - + /** * The project URL. */ @@ -272,17 +272,17 @@ public class Model { * @return the value of projectURL */ public String getProjectURL() { - return projectURL; - } + return projectURL; + } /** * Set the value of projectURL. * - * @param parentVersion new value of projectURL + * @param projectURL new value of projectURL */ - public void setProjectURL(String projectURL) { - this.projectURL = projectURL; - } + public void setProjectURL(String projectURL) { + this.projectURL = projectURL; + } /** * Process the Maven properties file and interpolate all properties. @@ -308,11 +308,14 @@ public class Model { /** *

- * A utility function that will interpolate strings based on values given in the properties file. It will also interpolate the - * strings contained within the properties file so that properties can reference other properties.

+ * A utility function that will interpolate strings based on values given in + * the properties file. It will also interpolate the strings contained + * within the properties file so that properties can reference other + * properties.

*

- * Note: if there is no property found the reference will be removed. In other words, if the interpolated string will - * be replaced with an empty string. + * Note: if there is no property found the reference will be removed. + * In other words, if the interpolated string will be replaced with an empty + * string. *

*

* Example:

@@ -329,7 +332,8 @@ public class Model { * * * @param text the string that contains references to properties. - * @param properties a collection of properties that may be referenced within the text. + * @param properties a collection of properties that may be referenced + * within the text. * @return the interpolated text. */ public static String interpolateString(String text, Properties properties) { @@ -340,8 +344,9 @@ public class Model { return substitutor.replace(text); } - /** - * Utility class that can provide values from a Properties object to a StrSubstitutor. + /** + * Utility class that can provide values from a Properties object to a + * StrSubstitutor. */ private static class PropertyLookup extends StrLookup { diff --git a/src/main/config/checkstyle-header.txt b/src/main/config/checkstyle-header.txt index c2ceba9ef..2e87cd304 100644 --- a/src/main/config/checkstyle-header.txt +++ b/src/main/config/checkstyle-header.txt @@ -13,6 +13,6 @@ ^ \* See the License for the specific language governing permissions and\s*$ ^ \* limitations under the License\.\s*$ ^ \*\s*$ -^ \* Copyright \(c\) 201[0-9] (Jeremy Long|Steve Springett|The OWASP Foundation|Institute for Defense Analyses)\. All Rights Reserved\.\s*$ +^ \* Copyright \(c\) 201[0-9] (Jeremy Long|Steve Springett|Bianca Jiang|The OWASP Foundation|Institute for Defense Analyses)\. All Rights Reserved\.\s*$ ^ \*/\s*$ ^package