From 9fcf23c8022724b56758a4d7c1e91e36f183dfb6 Mon Sep 17 00:00:00 2001 From: Jeremy Long Date: Thu, 1 Sep 2016 05:46:09 -0400 Subject: [PATCH] coverity, checkstyle, pmd, and findbugs suggested corrections --- .../analyzer/AssemblyAnalyzer.java | 17 +++++----- .../analyzer/CMakeAnalyzer.java | 3 ++ .../analyzer/CocoaPodsAnalyzer.java | 32 +++++++++++++++---- .../analyzer/DependencyBundlingAnalyzer.java | 8 +++++ .../analyzer/FileNameAnalyzer.java | 2 ++ .../analyzer/PythonDistributionAnalyzer.java | 15 +++++---- .../analyzer/RubyBundleAuditAnalyzer.java | 7 +++- .../analyzer/RubyBundlerAnalyzer.java | 2 +- .../analyzer/SwiftPackageManagerAnalyzer.java | 18 ++++++++++- .../exception/ExceptionCollection.java | 2 +- .../suppression/SuppressionErrorHandler.java | 4 +-- .../owasp/dependencycheck/utils/Checksum.java | 10 +++--- .../dependencycheck/utils/Downloader.java | 2 +- src/main/config/checkstyle-suppressions.xml | 5 +-- 14 files changed, 91 insertions(+), 36 deletions(-) diff --git a/dependency-check-core/src/main/java/org/owasp/dependencycheck/analyzer/AssemblyAnalyzer.java b/dependency-check-core/src/main/java/org/owasp/dependencycheck/analyzer/AssemblyAnalyzer.java index 016d2d84b..897c5ad23 100644 --- a/dependency-check-core/src/main/java/org/owasp/dependencycheck/analyzer/AssemblyAnalyzer.java +++ b/dependency-check-core/src/main/java/org/owasp/dependencycheck/analyzer/AssemblyAnalyzer.java @@ -17,7 +17,6 @@ */ package org.owasp.dependencycheck.analyzer; -import java.io.BufferedReader; import java.io.File; import java.io.FileFilter; import java.io.FileOutputStream; @@ -36,9 +35,6 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; import org.w3c.dom.Document; import org.xml.sax.SAXException; -import java.io.InputStreamReader; -import java.nio.file.Path; -import java.nio.file.Paths; import javax.xml.parsers.DocumentBuilder; import javax.xml.parsers.DocumentBuilderFactory; @@ -122,6 +118,10 @@ public class AssemblyAnalyzer extends AbstractFileTypeAnalyzer { } final List args = buildArgumentList(); + if (args == null) { + LOGGER.warn("Assembly Analyzer was unable to execute"); + return; + } args.add(dependency.getActualFilePath()); final ProcessBuilder pb = new ProcessBuilder(args); Document doc = null; @@ -237,7 +237,7 @@ public class AssemblyAnalyzer extends AbstractFileTypeAnalyzer { final List args = buildArgumentList(); //TODO this creaes an "unreported" error - if someone doesn't look // at the command output this could easily be missed (especially in an - // Ant or Mmaven build. + // Ant or Mmaven build. // // We need to create a non-fatal warning error type that will // get added to the report. @@ -249,6 +249,7 @@ public class AssemblyAnalyzer extends AbstractFileTypeAnalyzer { + "'exe' or 'dll' was scanned. The 'mono' executale could not be found on " + "the path; either disable the Assembly Analyzer or configure the path mono."); LOGGER.error("----------------------------------------------------"); + return; } try { final ProcessBuilder pb = new ProcessBuilder(args); @@ -353,10 +354,10 @@ public class AssemblyAnalyzer extends AbstractFileTypeAnalyzer { * false */ private boolean isInPath(String file) { - ProcessBuilder pb = new ProcessBuilder("which", file); + final ProcessBuilder pb = new ProcessBuilder("which", file); try { - Process proc = pb.start(); - int retCode = proc.waitFor(); + final Process proc = pb.start(); + final int retCode = proc.waitFor(); if (retCode == 0) { return true; } diff --git a/dependency-check-core/src/main/java/org/owasp/dependencycheck/analyzer/CMakeAnalyzer.java b/dependency-check-core/src/main/java/org/owasp/dependencycheck/analyzer/CMakeAnalyzer.java index 68a499d31..3220b30a8 100644 --- a/dependency-check-core/src/main/java/org/owasp/dependencycheck/analyzer/CMakeAnalyzer.java +++ b/dependency-check-core/src/main/java/org/owasp/dependencycheck/analyzer/CMakeAnalyzer.java @@ -196,6 +196,9 @@ public class CMakeAnalyzer extends AbstractFileTypeAnalyzer { * @param engine the dependency-check engine * @param contents the version information */ + @edu.umd.cs.findbugs.annotations.SuppressFBWarnings( + value = "DM_DEFAULT_ENCODING", + justification = "Default encoding is only used if UTF-8 is not available") private void analyzeSetVersionCommand(Dependency dependency, Engine engine, String contents) { Dependency currentDep = dependency; diff --git a/dependency-check-core/src/main/java/org/owasp/dependencycheck/analyzer/CocoaPodsAnalyzer.java b/dependency-check-core/src/main/java/org/owasp/dependencycheck/analyzer/CocoaPodsAnalyzer.java index d23ec72ea..1108d5e6a 100644 --- a/dependency-check-core/src/main/java/org/owasp/dependencycheck/analyzer/CocoaPodsAnalyzer.java +++ b/dependency-check-core/src/main/java/org/owasp/dependencycheck/analyzer/CocoaPodsAnalyzer.java @@ -13,7 +13,7 @@ * See the License for the specific language governing permissions and * limitations under the License. * - * © Copyright IBM Corporation 2016. + * Copyright (c) 2016 IBM Corporation. All Rights Reserved. */ package org.owasp.dependencycheck.analyzer; @@ -154,19 +154,32 @@ public class CocoaPodsAnalyzer extends AbstractFileTypeAnalyzer { setPackagePath(dependency); } + /** + * Extracts evidence from the contents and adds it to the given evidence + * collection. + * + * @param evidences the evidence collection to update + * @param contents the text to extract evidence from + * @param blockVariable the block variable within the content to search for + * @param field the name of the field being searched for + * @param fieldPattern the field pattern within the contents to search for + * @param confidence the confidence level of the evidence if found + * @return the string that was added as evidence + */ 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( - String.format("\\s*?%s\\.%s\\s*?=\\s*?\\{\\s*?(.*?)\\s*?\\}", blockVariable, fieldPattern), Pattern.CASE_INSENSITIVE).matcher(contents); + String.format("\\s*?%s\\.%s\\s*?=\\s*?\\{\\s*?(.*?)\\s*?\\}", blockVariable, fieldPattern), + Pattern.CASE_INSENSITIVE).matcher(contents); if (arrayMatcher.find()) { value = arrayMatcher.group(1); - } //capture single value between quotes - else { + } 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); + String.format("\\s*?%s\\.%s\\s*?=\\s*?(['\"])(.*?)\\1", blockVariable, fieldPattern), + Pattern.CASE_INSENSITIVE).matcher(contents); if (matcher.find()) { value = matcher.group(2); } @@ -177,9 +190,14 @@ public class CocoaPodsAnalyzer extends AbstractFileTypeAnalyzer { return value; } + /** + * Sets the package path on the given dependency. + * + * @param dep the dependency to update + */ private void setPackagePath(Dependency dep) { - File file = new File(dep.getFilePath()); - String parent = file.getParent(); + 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/analyzer/DependencyBundlingAnalyzer.java b/dependency-check-core/src/main/java/org/owasp/dependencycheck/analyzer/DependencyBundlingAnalyzer.java index 7febee2e3..946194ab9 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 @@ -411,6 +411,14 @@ public class DependencyBundlingAnalyzer extends AbstractAnalyzer implements Anal return false; } + /** + * 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")) { 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 75f417ebf..2aded7eb1 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,10 +70,12 @@ public class FileNameAnalyzer extends AbstractAnalyzer implements Analyzer { /** * Python init files */ + //CSOFF: WhitespaceAfter private static final NameFileFilter IGNORED_FILES = new NameFileFilter(new String[]{ "__init__.py", "__init__.pyc", "__init__.pyo",}); + //CSON: WhitespaceAfter /** * Collects information about the file name. diff --git a/dependency-check-core/src/main/java/org/owasp/dependencycheck/analyzer/PythonDistributionAnalyzer.java b/dependency-check-core/src/main/java/org/owasp/dependencycheck/analyzer/PythonDistributionAnalyzer.java index 5561f0494..a82b0ebd9 100644 --- a/dependency-check-core/src/main/java/org/owasp/dependencycheck/analyzer/PythonDistributionAnalyzer.java +++ b/dependency-check-core/src/main/java/org/owasp/dependencycheck/analyzer/PythonDistributionAnalyzer.java @@ -178,7 +178,7 @@ public class PythonDistributionAnalyzer extends AbstractFileTypeAnalyzer { protected String getAnalyzerEnabledSettingKey() { return Settings.KEYS.ANALYZER_PYTHON_DISTRIBUTION_ENABLED; } - + @Override protected void analyzeFileType(Dependency dependency, Engine engine) throws AnalysisException { @@ -227,11 +227,14 @@ public class PythonDistributionAnalyzer extends AbstractFileTypeAnalyzer { } catch (ExtractionException ex) { throw new AnalysisException(ex); } - - collectWheelMetadata( - dependency, - getMatchingFile(getMatchingFile(temp, folderFilter), - metadataFilter)); + + File matchingFile = getMatchingFile(temp, folderFilter); + if (matchingFile != null) { + matchingFile = getMatchingFile(matchingFile, metadataFilter); + if (matchingFile != null) { + collectWheelMetadata(dependency, matchingFile); + } + } } /** 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 708144ebc..bca567fa5 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 @@ -280,11 +280,16 @@ public class RubyBundleAuditAnalyzer extends AbstractFileTypeAnalyzer { } final File parentFile = dependency.getActualFile().getParentFile(); final Process process = launchBundleAudit(parentFile); + final int exitValue; try { - process.waitFor(); + exitValue = process.waitFor(); } catch (InterruptedException ie) { throw new AnalysisException("bundle-audit process interrupted", ie); } + if (exitValue != 0) { + final String msg = String.format("Unexpected exit code from bundle-audit process; exit code: %s", exitValue); + throw new AnalysisException(msg); + } BufferedReader rdr = null; BufferedReader errReader = null; try { 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 b89a11d92..df394760f 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 @@ -13,7 +13,7 @@ * See the License for the specific language governing permissions and * limitations under the License. * - * © Copyright IBM Corporation 2016. + * Copyright (c) 2016 IBM Corporation. All Rights Reserved. */ package org.owasp.dependencycheck.analyzer; diff --git a/dependency-check-core/src/main/java/org/owasp/dependencycheck/analyzer/SwiftPackageManagerAnalyzer.java b/dependency-check-core/src/main/java/org/owasp/dependencycheck/analyzer/SwiftPackageManagerAnalyzer.java index 5e7f363f5..d0a6bb0b9 100644 --- a/dependency-check-core/src/main/java/org/owasp/dependencycheck/analyzer/SwiftPackageManagerAnalyzer.java +++ b/dependency-check-core/src/main/java/org/owasp/dependencycheck/analyzer/SwiftPackageManagerAnalyzer.java @@ -13,7 +13,7 @@ * See the License for the specific language governing permissions and * limitations under the License. * - * © Copyright IBM Corporation 2016. + * Copyright (c) 2016 IBM Corporation. All Rights Reserved. */ package org.owasp.dependencycheck.analyzer; @@ -146,6 +146,17 @@ public class SwiftPackageManagerAnalyzer extends AbstractFileTypeAnalyzer { setPackagePath(dependency); } + /** + * Extracts evidence from the package description and adds it to the given + * evidence collection. + * + * @param evidences the evidence collection to update + * @param packageDescription the text to extract evidence from + * @param field the name of the field being searched for + * @param fieldPattern the field pattern within the contents to search for + * @param confidence the confidence level of the evidence if found + * @return the string that was added as evidence + */ private String addStringEvidence(EvidenceCollection evidences, String packageDescription, String field, String fieldPattern, Confidence confidence) { String value = ""; @@ -166,6 +177,11 @@ public class SwiftPackageManagerAnalyzer extends AbstractFileTypeAnalyzer { return value; } + /** + * Sets the package path on the given dependency. + * + * @param dep the dependency to update + */ private void setPackagePath(Dependency dep) { final File file = new File(dep.getFilePath()); final String parent = file.getParent(); diff --git a/dependency-check-core/src/main/java/org/owasp/dependencycheck/exception/ExceptionCollection.java b/dependency-check-core/src/main/java/org/owasp/dependencycheck/exception/ExceptionCollection.java index 94cae929d..1932dc79a 100644 --- a/dependency-check-core/src/main/java/org/owasp/dependencycheck/exception/ExceptionCollection.java +++ b/dependency-check-core/src/main/java/org/owasp/dependencycheck/exception/ExceptionCollection.java @@ -212,7 +212,7 @@ public class ExceptionCollection extends Exception { */ @Override public String getMessage() { - StringBuilder sb = new StringBuilder(); + final StringBuilder sb = new StringBuilder(); final String msg = super.getMessage(); if (msg == null || msg.isEmpty()) { sb.append("One or more exceptions occured during analysis:"); diff --git a/dependency-check-core/src/main/java/org/owasp/dependencycheck/xml/suppression/SuppressionErrorHandler.java b/dependency-check-core/src/main/java/org/owasp/dependencycheck/xml/suppression/SuppressionErrorHandler.java index e36bc5365..07d4b661e 100644 --- a/dependency-check-core/src/main/java/org/owasp/dependencycheck/xml/suppression/SuppressionErrorHandler.java +++ b/dependency-check-core/src/main/java/org/owasp/dependencycheck/xml/suppression/SuppressionErrorHandler.java @@ -17,8 +17,6 @@ */ package org.owasp.dependencycheck.xml.suppression; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; import org.xml.sax.ErrorHandler; import org.xml.sax.SAXException; import org.xml.sax.SAXParseException; @@ -33,7 +31,7 @@ public class SuppressionErrorHandler implements ErrorHandler { /** * The logger. */ - private static final Logger LOGGER = LoggerFactory.getLogger(SuppressionErrorHandler.class); + //private static final Logger LOGGER = LoggerFactory.getLogger(SuppressionErrorHandler.class); /** * Builds a prettier exception message. diff --git a/dependency-check-utils/src/main/java/org/owasp/dependencycheck/utils/Checksum.java b/dependency-check-utils/src/main/java/org/owasp/dependencycheck/utils/Checksum.java index 62c0bf4ad..28842818d 100644 --- a/dependency-check-utils/src/main/java/org/owasp/dependencycheck/utils/Checksum.java +++ b/dependency-check-utils/src/main/java/org/owasp/dependencycheck/utils/Checksum.java @@ -58,11 +58,11 @@ public final class Checksum { * @throws NoSuchAlgorithmException when an algorithm is specified that does not exist */ public static byte[] getChecksum(String algorithm, File file) throws NoSuchAlgorithmException, IOException { - MessageDigest digest = MessageDigest.getInstance(algorithm); + final MessageDigest digest = MessageDigest.getInstance(algorithm); FileInputStream fis = null; try { fis = new FileInputStream(file); - FileChannel ch = fis.getChannel(); + final FileChannel ch = fis.getChannel(); long remainingToRead = file.length(); long start = 0; while (remainingToRead > 0) { @@ -74,7 +74,7 @@ public final class Checksum { amountToRead = remainingToRead; remainingToRead = 0; } - MappedByteBuffer byteBuffer = ch.map(FileChannel.MapMode.READ_ONLY, start, amountToRead); + final MappedByteBuffer byteBuffer = ch.map(FileChannel.MapMode.READ_ONLY, start, amountToRead); digest.update(byteBuffer); start += amountToRead; } @@ -99,7 +99,7 @@ public final class Checksum { * @throws NoSuchAlgorithmException when the MD5 algorithm is not available */ public static String getMD5Checksum(File file) throws IOException, NoSuchAlgorithmException { - byte[] b = getChecksum("MD5", file); + final byte[] b = getChecksum("MD5", file); return getHex(b); } @@ -112,7 +112,7 @@ public final class Checksum { * @throws NoSuchAlgorithmException when the SHA1 algorithm is not available */ public static String getSHA1Checksum(File file) throws IOException, NoSuchAlgorithmException { - byte[] b = getChecksum("SHA1", file); + final byte[] b = getChecksum("SHA1", file); return getHex(b); } /** diff --git a/dependency-check-utils/src/main/java/org/owasp/dependencycheck/utils/Downloader.java b/dependency-check-utils/src/main/java/org/owasp/dependencycheck/utils/Downloader.java index 88d12b9e2..5965ef1a6 100644 --- a/dependency-check-utils/src/main/java/org/owasp/dependencycheck/utils/Downloader.java +++ b/dependency-check-utils/src/main/java/org/owasp/dependencycheck/utils/Downloader.java @@ -304,7 +304,7 @@ public final class Downloader { Throwable cause = ex; while (cause != null) { if (cause instanceof java.net.UnknownHostException) { - final String msg = String.format("Unable to resolve domain '%s'", cause.getMessage()); + final String msg = format("Unable to resolve domain '%s'", cause.getMessage()); LOGGER.error(msg); throw new DownloadFailedException(msg); } diff --git a/src/main/config/checkstyle-suppressions.xml b/src/main/config/checkstyle-suppressions.xml index 27c63c4b8..56ec1750f 100644 --- a/src/main/config/checkstyle-suppressions.xml +++ b/src/main/config/checkstyle-suppressions.xml @@ -7,8 +7,9 @@ - + - + + \ No newline at end of file