coverity suggested corrections

This commit is contained in:
Jeremy Long
2016-08-21 14:40:07 -04:00
parent 4861592d2a
commit cedd93e774
14 changed files with 175 additions and 63 deletions

View File

@@ -158,8 +158,13 @@ public class App {
exitCode = -4; exitCode = -4;
} }
try { try {
runScan(cli.getReportDirectory(), cli.getReportFormat(), cli.getProjectName(), cli.getScanFiles(), String[] scanFiles = cli.getScanFiles();
if (scanFiles != null) {
runScan(cli.getReportDirectory(), cli.getReportFormat(), cli.getProjectName(), scanFiles,
cli.getExcludeList(), cli.getSymLinkDepth()); cli.getExcludeList(), cli.getSymLinkDepth());
} else {
LOGGER.error("No scan files configured");
}
} catch (InvalidScanPathException ex) { } catch (InvalidScanPathException ex) {
LOGGER.error("An invalid scan path was detected; unable to scan '//*' paths"); LOGGER.error("An invalid scan path was detected; unable to scan '//*' paths");
exitCode = -10; exitCode = -10;

View File

@@ -196,6 +196,10 @@ public final class CliParser {
isValid = false; isValid = false;
final String msg = String.format("Invalid '%s' argument: '%s'%nUnable to scan paths that start with '//'.", argumentName, path); final String msg = String.format("Invalid '%s' argument: '%s'%nUnable to scan paths that start with '//'.", argumentName, path);
throw new FileNotFoundException(msg); throw new FileNotFoundException(msg);
} else if ((path.endsWith("/*") && !path.endsWith("**/*")) || (path.endsWith("\\*") && path.endsWith("**\\*"))) {
final String msg = String.format("Possibly incorrect path '%s' from argument '%s' because it ends with a slash star; "
+ "dependency-check uses ant-style paths", path, argumentName);
LOGGER.warn(msg);
} }
} }

View File

@@ -130,8 +130,10 @@ public abstract class AbstractSuppressionAnalyzer extends AbstractAnalyzer {
} }
} else { } else {
file = new File(suppressionFilePath); file = new File(suppressionFilePath);
InputStream suppressionsFromClasspath = null;
if (!file.exists()) { if (!file.exists()) {
final InputStream suppressionsFromClasspath = this.getClass().getClassLoader().getResourceAsStream(suppressionFilePath); try {
suppressionsFromClasspath = this.getClass().getClassLoader().getResourceAsStream(suppressionFilePath);
if (suppressionsFromClasspath != null) { if (suppressionsFromClasspath != null) {
deleteTempFile = true; deleteTempFile = true;
file = FileUtils.getTempFile("suppression", "xml"); file = FileUtils.getTempFile("suppression", "xml");
@@ -141,6 +143,15 @@ public abstract class AbstractSuppressionAnalyzer extends AbstractAnalyzer {
throwSuppressionParseException("Unable to locate suppressions file in classpath", ex); throwSuppressionParseException("Unable to locate suppressions file in classpath", ex);
} }
} }
} finally {
if (suppressionsFromClasspath != null) {
try {
suppressionsFromClasspath.close();
} catch (IOException ex) {
LOGGER.debug("Failed to close stream", ex);
}
}
}
} }
} }
if (file != null) { if (file != null) {

View File

@@ -357,6 +357,10 @@ public class ArchiveAnalyzer extends AbstractFileTypeAnalyzer {
*/ */
private void extractFiles(File archive, File destination, Engine engine) throws AnalysisException { private void extractFiles(File archive, File destination, Engine engine) throws AnalysisException {
if (archive != null && destination != null) { if (archive != null && destination != null) {
final String archiveExt = FileUtils.getFileExtension(archive.getName()).toLowerCase();
if (archiveExt == null) {
return;
}
FileInputStream fis; FileInputStream fis;
try { try {
fis = new FileInputStream(archive); fis = new FileInputStream(archive);
@@ -364,7 +368,6 @@ public class ArchiveAnalyzer extends AbstractFileTypeAnalyzer {
LOGGER.debug("", ex); LOGGER.debug("", ex);
throw new AnalysisException("Archive file was not found.", ex); throw new AnalysisException("Archive file was not found.", ex);
} }
final String archiveExt = FileUtils.getFileExtension(archive.getName()).toLowerCase();
try { try {
if (ZIPPABLES.contains(archiveExt)) { if (ZIPPABLES.contains(archiveExt)) {
final BufferedInputStream in = new BufferedInputStream(fis); final BufferedInputStream in = new BufferedInputStream(fis);
@@ -414,8 +417,9 @@ public class ArchiveAnalyzer extends AbstractFileTypeAnalyzer {
if ("jar".equals(archiveExt) && in.markSupported()) { if ("jar".equals(archiveExt) && in.markSupported()) {
in.mark(7); in.mark(7);
final byte[] b = new byte[7]; final byte[] b = new byte[7];
in.read(b); final int read = in.read(b);
if (b[0] == '#' if (read == 7
&& b[0] == '#'
&& b[1] == '!' && b[1] == '!'
&& b[2] == '/' && b[2] == '/'
&& b[3] == 'b' && b[3] == 'b'

View File

@@ -311,7 +311,9 @@ public class HintAnalyzer extends AbstractAnalyzer implements Analyzer {
} else { } else {
file = new File(filePath); file = new File(filePath);
if (!file.exists()) { if (!file.exists()) {
final InputStream fromClasspath = this.getClass().getClassLoader().getResourceAsStream(filePath); InputStream fromClasspath = null;
try {
fromClasspath = this.getClass().getClassLoader().getResourceAsStream(filePath);
if (fromClasspath != null) { if (fromClasspath != null) {
deleteTempFile = true; deleteTempFile = true;
file = FileUtils.getTempFile("hint", "xml"); file = FileUtils.getTempFile("hint", "xml");
@@ -321,6 +323,11 @@ public class HintAnalyzer extends AbstractAnalyzer implements Analyzer {
throw new HintParseException("Unable to locate suppressions file in classpath", ex); throw new HintParseException("Unable to locate suppressions file in classpath", ex);
} }
} }
} finally {
if (fromClasspath != null) {
fromClasspath.close();
}
}
} }
} }

View File

@@ -487,7 +487,7 @@ public class JarAnalyzer extends AbstractFileTypeAnalyzer {
} }
final String originalGroupID = groupid; final String originalGroupID = groupid;
if (groupid.startsWith("org.") || groupid.startsWith("com.")) { if (groupid != null && (groupid.startsWith("org.") || groupid.startsWith("com."))) {
groupid = groupid.substring(4); groupid = groupid.substring(4);
} }
@@ -496,7 +496,7 @@ public class JarAnalyzer extends AbstractFileTypeAnalyzer {
} }
final String originalArtifactID = artifactid; final String originalArtifactID = artifactid;
if (artifactid.startsWith("org.") || artifactid.startsWith("com.")) { if (artifactid != null && (artifactid.startsWith("org.") || artifactid.startsWith("com."))) {
artifactid = artifactid.substring(4); artifactid = artifactid.substring(4);
} }

View File

@@ -24,9 +24,9 @@ import java.io.FileInputStream;
import java.io.FileNotFoundException; import java.io.FileNotFoundException;
import java.io.FilenameFilter; import java.io.FilenameFilter;
import java.io.IOException; import java.io.IOException;
import java.io.InputStream;
import org.apache.commons.io.filefilter.NameFileFilter; import org.apache.commons.io.filefilter.NameFileFilter;
import org.apache.commons.io.filefilter.SuffixFileFilter; import org.apache.commons.io.filefilter.SuffixFileFilter;
import org.apache.commons.io.input.AutoCloseInputStream;
import org.apache.commons.lang3.StringUtils; import org.apache.commons.lang3.StringUtils;
import org.owasp.dependencycheck.Engine; import org.owasp.dependencycheck.Engine;
import org.owasp.dependencycheck.analyzer.exception.AnalysisException; import org.owasp.dependencycheck.analyzer.exception.AnalysisException;
@@ -354,13 +354,22 @@ public class PythonDistributionAnalyzer extends AbstractFileTypeAnalyzer {
if (null == manifest) { if (null == manifest) {
LOGGER.debug("Manifest file not found."); LOGGER.debug("Manifest file not found.");
} else { } else {
InputStream in = null;
try { try {
result.load(new AutoCloseInputStream(new BufferedInputStream( in = new BufferedInputStream(new FileInputStream(manifest));
new FileInputStream(manifest)))); result.load(in);
} catch (MessagingException e) { } catch (MessagingException e) {
LOGGER.warn(e.getMessage(), e); LOGGER.warn(e.getMessage(), e);
} catch (FileNotFoundException e) { } catch (FileNotFoundException e) {
LOGGER.warn(e.getMessage(), e); LOGGER.warn(e.getMessage(), e);
} finally {
if (in != null) {
try {
in.close();
} catch (IOException ex) {
LOGGER.debug("failed to close input stream", ex);
}
}
} }
} }
return result; return result;

View File

@@ -217,6 +217,9 @@ public class RubyGemspecAnalyzer extends AbstractFileTypeAnalyzer {
return name.contains(VERSION_FILE_NAME); return name.contains(VERSION_FILE_NAME);
} }
}); });
if (matchingFiles == null) {
return;
}
for (File f : matchingFiles) { for (File f : matchingFiles) {
try { try {
final List<String> lines = FileUtils.readLines(f, Charset.defaultCharset()); final List<String> lines = FileUtils.readLines(f, Charset.defaultCharset());

View File

@@ -36,8 +36,10 @@ import org.slf4j.Logger;
import org.slf4j.LoggerFactory; import org.slf4j.LoggerFactory;
/** /**
* Loads the configured database driver and returns the database connection. If the embedded H2 database is used obtaining a * Loads the configured database driver and returns the database connection. If
* connection will ensure the database file exists and that the appropriate table structure has been created. * the embedded H2 database is used obtaining a connection will ensure the
* database file exists and that the appropriate table structure has been
* created.
* *
* @author Jeremy Long * @author Jeremy Long
*/ */
@@ -87,10 +89,11 @@ public final class ConnectionFactory {
} }
/** /**
* Initializes the connection factory. Ensuring that the appropriate drivers are loaded and that a connection can be made * Initializes the connection factory. Ensuring that the appropriate drivers
* successfully. * are loaded and that a connection can be made successfully.
* *
* @throws DatabaseException thrown if we are unable to connect to the database * @throws DatabaseException thrown if we are unable to connect to the
* database
*/ */
public static synchronized void initialize() throws DatabaseException { public static synchronized void initialize() throws DatabaseException {
//this only needs to be called once. //this only needs to be called once.
@@ -188,9 +191,10 @@ public final class ConnectionFactory {
} }
/** /**
* Cleans up resources and unloads any registered database drivers. This needs to be called to ensure the driver is * Cleans up resources and unloads any registered database drivers. This
* unregistered prior to the finalize method being called as during shutdown the class loader used to load the driver may be * needs to be called to ensure the driver is unregistered prior to the
* unloaded prior to the driver being de-registered. * finalize method being called as during shutdown the class loader used to
* load the driver may be unloaded prior to the driver being de-registered.
*/ */
public static synchronized void cleanup() { public static synchronized void cleanup() {
if (driver != null) { if (driver != null) {
@@ -210,10 +214,12 @@ public final class ConnectionFactory {
} }
/** /**
* Constructs a new database connection object per the database configuration. * Constructs a new database connection object per the database
* configuration.
* *
* @return a database connection object * @return a database connection object
* @throws DatabaseException thrown if there is an exception loading the database connection * @throws DatabaseException thrown if there is an exception loading the
* database connection
*/ */
public static Connection getConnection() throws DatabaseException { public static Connection getConnection() throws DatabaseException {
initialize(); initialize();
@@ -228,10 +234,12 @@ public final class ConnectionFactory {
} }
/** /**
* Determines if the H2 database file exists. If it does not exist then the data structure will need to be created. * Determines if the H2 database file exists. If it does not exist then the
* data structure will need to be created.
* *
* @return true if the H2 database file does not exist; otherwise false * @return true if the H2 database file does not exist; otherwise false
* @throws IOException thrown if the data directory does not exist and cannot be created * @throws IOException thrown if the data directory does not exist and
* cannot be created
*/ */
private static boolean h2DataFileExists() throws IOException { private static boolean h2DataFileExists() throws IOException {
final File dir = Settings.getDataDirectory(); final File dir = Settings.getDataDirectory();
@@ -241,7 +249,8 @@ public final class ConnectionFactory {
} }
/** /**
* Creates the database structure (tables and indexes) to store the CVE data. * Creates the database structure (tables and indexes) to store the CVE
* data.
* *
* @param conn the database connection * @param conn the database connection
* @throws DatabaseException thrown if there is a Database Exception * @throws DatabaseException thrown if there is a Database Exception
@@ -271,14 +280,17 @@ public final class ConnectionFactory {
} }
/** /**
* Updates the database schema by loading the upgrade script for the version specified. The intended use is that if the * Updates the database schema by loading the upgrade script for the version
* current schema version is 2.9 then we would call updateSchema(conn, "2.9"). This would load the upgrade_2.9.sql file and * specified. The intended use is that if the current schema version is 2.9
* execute it against the database. The upgrade script must update the 'version' in the properties table. * then we would call updateSchema(conn, "2.9"). This would load the
* upgrade_2.9.sql file and execute it against the database. The upgrade
* script must update the 'version' in the properties table.
* *
* @param conn the database connection object * @param conn the database connection object
* @param appExpectedVersion the schema version that the application expects * @param appExpectedVersion the schema version that the application expects
* @param currentDbVersion the current schema version of the database * @param currentDbVersion the current schema version of the database
* @throws DatabaseException thrown if there is an exception upgrading the database schema * @throws DatabaseException thrown if there is an exception upgrading the
* database schema
*/ */
private static void updateSchema(Connection conn, DependencyVersion appExpectedVersion, DependencyVersion currentDbVersion) private static void updateSchema(Connection conn, DependencyVersion appExpectedVersion, DependencyVersion currentDbVersion)
throws DatabaseException { throws DatabaseException {
@@ -340,15 +352,18 @@ public final class ConnectionFactory {
} }
/** /**
* Counter to ensure that calls to ensureSchemaVersion does not end up in an endless loop. * Counter to ensure that calls to ensureSchemaVersion does not end up in an
* endless loop.
*/ */
private static int callDepth = 0; private static int callDepth = 0;
/** /**
* Uses the provided connection to check the specified schema version within the database. * Uses the provided connection to check the specified schema version within
* the database.
* *
* @param conn the database connection object * @param conn the database connection object
* @throws DatabaseException thrown if the schema version is not compatible with this version of dependency-check * @throws DatabaseException thrown if the schema version is not compatible
* with this version of dependency-check
*/ */
private static void ensureSchemaVersion(Connection conn) throws DatabaseException { private static void ensureSchemaVersion(Connection conn) throws DatabaseException {
ResultSet rs = null; ResultSet rs = null;
@@ -359,7 +374,13 @@ public final class ConnectionFactory {
rs = ps.executeQuery(); rs = ps.executeQuery();
if (rs.next()) { if (rs.next()) {
final DependencyVersion appDbVersion = DependencyVersionUtil.parseVersion(DB_SCHEMA_VERSION); final DependencyVersion appDbVersion = DependencyVersionUtil.parseVersion(DB_SCHEMA_VERSION);
if (appDbVersion == null) {
throw new DatabaseException("Invalid application database schema");
}
final DependencyVersion db = DependencyVersionUtil.parseVersion(rs.getString(1)); final DependencyVersion db = DependencyVersionUtil.parseVersion(rs.getString(1));
if (db == null) {
throw new DatabaseException("Invalid database schema");
}
if (appDbVersion.compareTo(db) > 0) { if (appDbVersion.compareTo(db) > 0) {
LOGGER.debug("Current Schema: {}", DB_SCHEMA_VERSION); LOGGER.debug("Current Schema: {}", DB_SCHEMA_VERSION);
LOGGER.debug("DB Schema: {}", rs.getString(1)); LOGGER.debug("DB Schema: {}", rs.getString(1));

View File

@@ -104,8 +104,9 @@ public class HintParser {
* @throws SAXException thrown if the XML cannot be parsed * @throws SAXException thrown if the XML cannot be parsed
*/ */
public Hints parseHints(InputStream inputStream) throws HintParseException, SAXException { public Hints parseHints(InputStream inputStream) throws HintParseException, SAXException {
InputStream schemaStream = null;
try { try {
final InputStream schemaStream = this.getClass().getClassLoader().getResourceAsStream(HINT_SCHEMA); schemaStream = this.getClass().getClassLoader().getResourceAsStream(HINT_SCHEMA);
final HintHandler handler = new HintHandler(); final HintHandler handler = new HintHandler();
final SAXParserFactory factory = SAXParserFactory.newInstance(); final SAXParserFactory factory = SAXParserFactory.newInstance();
factory.setNamespaceAware(true); factory.setNamespaceAware(true);
@@ -141,6 +142,14 @@ public class HintParser {
} catch (IOException ex) { } catch (IOException ex) {
LOGGER.debug("", ex); LOGGER.debug("", ex);
throw new HintParseException(ex); throw new HintParseException(ex);
} finally {
if (schemaStream != null) {
try {
schemaStream.close();
} catch (IOException ex) {
LOGGER.debug("Error closing hint file stream", ex);
}
}
} }
} }
} }

View File

@@ -48,13 +48,17 @@ public final class PomUtils {
* *
* @param file the pom.xml file * @param file the pom.xml file
* @return returns a * @return returns a
* @throws AnalysisException is thrown if there is an exception extracting or parsing the POM {@link Model} object * @throws AnalysisException is thrown if there is an exception extracting
* or parsing the POM {@link Model} object
*/ */
public static Model readPom(File file) throws AnalysisException { public static Model readPom(File file) throws AnalysisException {
Model model = null;
try { try {
final PomParser parser = new PomParser(); final PomParser parser = new PomParser();
model = parser.parse(file); final Model model = parser.parse(file);
if (model == null) {
throw new AnalysisException(String.format("Unable to parse pom '%s'", file.getPath()));
}
return model;
} catch (PomParseException ex) { } catch (PomParseException ex) {
LOGGER.warn("Unable to parse pom '{}'", file.getPath()); LOGGER.warn("Unable to parse pom '{}'", file.getPath());
LOGGER.debug("", ex); LOGGER.debug("", ex);
@@ -68,7 +72,6 @@ public final class PomUtils {
LOGGER.debug("", ex); LOGGER.debug("", ex);
throw new AnalysisException(ex); throw new AnalysisException(ex);
} }
return model;
} }
/** /**
@@ -77,7 +80,8 @@ public final class PomUtils {
* @param path the path to the pom.xml file within the jar file * @param path the path to the pom.xml file within the jar file
* @param jar the jar file to extract the pom from * @param jar the jar file to extract the pom from
* @return returns a * @return returns a
* @throws AnalysisException is thrown if there is an exception extracting or parsing the POM {@link Model} object * @throws AnalysisException is thrown if there is an exception extracting
* or parsing the POM {@link Model} object
*/ */
public static Model readPom(String path, JarFile jar) throws AnalysisException { public static Model readPom(String path, JarFile jar) throws AnalysisException {
final ZipEntry entry = jar.getEntry(path); final ZipEntry entry = jar.getEntry(path);
@@ -105,11 +109,13 @@ public final class PomUtils {
} }
/** /**
* Reads in the pom file and adds elements as evidence to the given dependency. * Reads in the pom file and adds elements as evidence to the given
* dependency.
* *
* @param dependency the dependency being analyzed * @param dependency the dependency being analyzed
* @param pomFile the pom file to read * @param pomFile the pom file to read
* @throws AnalysisException is thrown if there is an exception parsing the pom * @throws AnalysisException is thrown if there is an exception parsing the
* pom
*/ */
public static void analyzePOM(Dependency dependency, File pomFile) throws AnalysisException { public static void analyzePOM(Dependency dependency, File pomFile) throws AnalysisException {
final Model pom = PomUtils.readPom(pomFile); final Model pom = PomUtils.readPom(pomFile);

View File

@@ -121,8 +121,9 @@ public class SuppressionParser {
* @throws SAXException thrown if the XML cannot be parsed * @throws SAXException thrown if the XML cannot be parsed
*/ */
public List<SuppressionRule> parseSuppressionRules(InputStream inputStream) throws SuppressionParseException, SAXException { public List<SuppressionRule> parseSuppressionRules(InputStream inputStream) throws SuppressionParseException, SAXException {
InputStream schemaStream = null;
try { try {
final InputStream schemaStream = this.getClass().getClassLoader().getResourceAsStream(SUPPRESSION_SCHEMA); schemaStream = this.getClass().getClassLoader().getResourceAsStream(SUPPRESSION_SCHEMA);
final SuppressionHandler handler = new SuppressionHandler(); final SuppressionHandler handler = new SuppressionHandler();
final SAXParserFactory factory = SAXParserFactory.newInstance(); final SAXParserFactory factory = SAXParserFactory.newInstance();
factory.setNamespaceAware(true); factory.setNamespaceAware(true);
@@ -157,6 +158,14 @@ public class SuppressionParser {
} catch (IOException ex) { } catch (IOException ex) {
LOGGER.debug("", ex); LOGGER.debug("", ex);
throw new SuppressionParseException(ex); throw new SuppressionParseException(ex);
} finally {
if (schemaStream != null) {
try {
schemaStream.close();
} catch (IOException ex) {
LOGGER.debug("Error closing suppression file stream", ex);
}
}
} }
} }
@@ -169,8 +178,9 @@ public class SuppressionParser {
* @throws SuppressionParseException if the XML cannot be parsed * @throws SuppressionParseException if the XML cannot be parsed
*/ */
private List<SuppressionRule> parseOldSuppressionRules(InputStream inputStream) throws SuppressionParseException { private List<SuppressionRule> parseOldSuppressionRules(InputStream inputStream) throws SuppressionParseException {
InputStream schemaStream = null;
try { try {
final InputStream schemaStream = this.getClass().getClassLoader().getResourceAsStream(OLD_SUPPRESSION_SCHEMA); schemaStream = this.getClass().getClassLoader().getResourceAsStream(OLD_SUPPRESSION_SCHEMA);
final SuppressionHandler handler = new SuppressionHandler(); final SuppressionHandler handler = new SuppressionHandler();
final SAXParserFactory factory = SAXParserFactory.newInstance(); final SAXParserFactory factory = SAXParserFactory.newInstance();
factory.setNamespaceAware(true); factory.setNamespaceAware(true);
@@ -200,6 +210,14 @@ public class SuppressionParser {
} catch (IOException ex) { } catch (IOException ex) {
LOGGER.debug("", ex); LOGGER.debug("", ex);
throw new SuppressionParseException(ex); throw new SuppressionParseException(ex);
} finally {
if (schemaStream != null) {
try {
schemaStream.close();
} catch (IOException ex) {
LOGGER.debug("Error closing old suppression file stream", ex);
}
}
} }
} }
} }

View File

@@ -17,7 +17,10 @@
*/ */
package org.owasp.dependencycheck.maven; package org.owasp.dependencycheck.maven;
import java.io.IOException;
import java.io.InputStream; import java.io.InputStream;
import java.util.logging.Level;
import java.util.logging.Logger;
import org.junit.AfterClass; import org.junit.AfterClass;
import org.junit.BeforeClass; import org.junit.BeforeClass;
import org.owasp.dependencycheck.utils.Settings; import org.owasp.dependencycheck.utils.Settings;
@@ -36,8 +39,20 @@ public class BaseTest {
@BeforeClass @BeforeClass
public static void setUpClass() throws Exception { public static void setUpClass() throws Exception {
Settings.initialize(); Settings.initialize();
InputStream mojoProperties = BaseTest.class.getClassLoader().getResourceAsStream(BaseTest.PROPERTIES_FILE); InputStream mojoProperties = null;
try {
mojoProperties = BaseTest.class.getClassLoader().getResourceAsStream(BaseTest.PROPERTIES_FILE);
Settings.mergeProperties(mojoProperties); Settings.mergeProperties(mojoProperties);
} finally {
if (mojoProperties != null) {
try {
mojoProperties.close();
} catch (IOException ex) {
Logger.getLogger(BaseTest.class.getName()).log(Level.SEVERE, null, ex);
}
}
}
} }
@AfterClass @AfterClass

View File

@@ -300,7 +300,7 @@ public final class Downloader {
* @throws DownloadFailedException a wrapper exception that contains the * @throws DownloadFailedException a wrapper exception that contains the
* original exception as the cause * original exception as the cause
*/ */
protected static void checkForCommonExceptionTypes(IOException ex) throws DownloadFailedException { protected static synchronized void checkForCommonExceptionTypes(IOException ex) throws DownloadFailedException {
Throwable cause = ex; Throwable cause = ex;
while (cause != null) { while (cause != null) {
if (cause instanceof java.net.UnknownHostException) { if (cause instanceof java.net.UnknownHostException) {