From 6d2a6bbd3d07af89bbf2fe978fe7324f9c92fa82 Mon Sep 17 00:00:00 2001 From: Hans Aikema Date: Wed, 12 Jul 2017 21:40:31 +0200 Subject: [PATCH 1/5] Fix issue #799 - Initialize exCol to prevent NPE --- .../owasp/dependencycheck/maven/BaseDependencyCheckMojo.java | 3 +++ 1 file changed, 3 insertions(+) diff --git a/dependency-check-maven/src/main/java/org/owasp/dependencycheck/maven/BaseDependencyCheckMojo.java b/dependency-check-maven/src/main/java/org/owasp/dependencycheck/maven/BaseDependencyCheckMojo.java index e910be398..bf3b945a1 100644 --- a/dependency-check-maven/src/main/java/org/owasp/dependencycheck/maven/BaseDependencyCheckMojo.java +++ b/dependency-check-maven/src/main/java/org/owasp/dependencycheck/maven/BaseDependencyCheckMojo.java @@ -691,6 +691,9 @@ public abstract class BaseDependencyCheckMojo extends AbstractMojo implements Ma } if (!isResolved) { getLog().error("Unable to resolve system scoped dependency: " + dependencyNode.toNodeString()); + if (exCol == null) { + exCol = new ExceptionCollection(); + } exCol.addException(new DependencyNotFoundException("Unable to resolve system scoped dependency: " + dependencyNode.toNodeString())); } From fccac8cb858b7e1d790813f7c00c506cebe13668 Mon Sep 17 00:00:00 2001 From: Stefan Neuhaus Date: Thu, 13 Jul 2017 21:16:46 +0200 Subject: [PATCH 2/5] =?UTF-8?q?Actual=20fix:=20the=20database=20product=20?= =?UTF-8?q?was=20reported=20as=20=E2=80=9CPostgreSQL=E2=80=9D=20by=20the?= =?UTF-8?q?=20driver.=20As=20the=20custom=20=E2=80=9CLocale=E2=80=9D=20use?= =?UTF-8?q?d=20in=20the=20ResourceBundle=20is=20case-sensitive,=20the=20mi?= =?UTF-8?q?xed-case=20properties=20file=20fails=20to=20be=20resolved=20(at?= =?UTF-8?q?=20least=20on=20case-sensitive=20file=20systems)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../main/java/org/owasp/dependencycheck/data/nvdcve/CveDB.java | 2 +- ...postgreSQL.properties => dbStatements_postgresql.properties} | 0 2 files changed, 1 insertion(+), 1 deletion(-) rename dependency-check-core/src/main/resources/data/{dbStatements_postgreSQL.properties => dbStatements_postgresql.properties} (100%) diff --git a/dependency-check-core/src/main/java/org/owasp/dependencycheck/data/nvdcve/CveDB.java b/dependency-check-core/src/main/java/org/owasp/dependencycheck/data/nvdcve/CveDB.java index 8d1c63415..d0e4c09e3 100644 --- a/dependency-check-core/src/main/java/org/owasp/dependencycheck/data/nvdcve/CveDB.java +++ b/dependency-check-core/src/main/java/org/owasp/dependencycheck/data/nvdcve/CveDB.java @@ -231,7 +231,7 @@ public final class CveDB implements AutoCloseable { */ private static String determineDatabaseProductName(Connection conn) { try { - final String databaseProductName = conn.getMetaData().getDatabaseProductName(); + final String databaseProductName = conn.getMetaData().getDatabaseProductName().toLowerCase(); LOGGER.debug("Database product: {}", databaseProductName); return databaseProductName; } catch (SQLException se) { diff --git a/dependency-check-core/src/main/resources/data/dbStatements_postgreSQL.properties b/dependency-check-core/src/main/resources/data/dbStatements_postgresql.properties similarity index 100% rename from dependency-check-core/src/main/resources/data/dbStatements_postgreSQL.properties rename to dependency-check-core/src/main/resources/data/dbStatements_postgresql.properties From d7d5e0c757621558a723cec351e39a37b50fc11b Mon Sep 17 00:00:00 2001 From: Stefan Neuhaus Date: Thu, 13 Jul 2017 21:18:27 +0200 Subject: [PATCH 3/5] Cleanup: Ease debugging connection problems: add cause to thrown exceptions --- .../data/nvdcve/ConnectionFactory.java | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/dependency-check-core/src/main/java/org/owasp/dependencycheck/data/nvdcve/ConnectionFactory.java b/dependency-check-core/src/main/java/org/owasp/dependencycheck/data/nvdcve/ConnectionFactory.java index c79e21b2f..829fdad27 100644 --- a/dependency-check-core/src/main/java/org/owasp/dependencycheck/data/nvdcve/ConnectionFactory.java +++ b/dependency-check-core/src/main/java/org/owasp/dependencycheck/data/nvdcve/ConnectionFactory.java @@ -117,7 +117,7 @@ public final class ConnectionFactory { } } catch (DriverLoadException ex) { LOGGER.debug("Unable to load database driver", ex); - throw new DatabaseException("Unable to load database driver"); + throw new DatabaseException("Unable to load database driver", ex); } } userName = Settings.getString(Settings.KEYS.DB_USER, "dcuser"); @@ -130,7 +130,7 @@ public final class ConnectionFactory { } catch (IOException ex) { LOGGER.debug( "Unable to retrieve the database connection string", ex); - throw new DatabaseException("Unable to retrieve the database connection string"); + throw new DatabaseException("Unable to retrieve the database connection string", ex); } boolean shouldCreateSchema = false; try { @@ -140,7 +140,7 @@ public final class ConnectionFactory { } } catch (IOException ioex) { LOGGER.debug("Unable to verify database exists", ioex); - throw new DatabaseException("Unable to verify database exists"); + throw new DatabaseException("Unable to verify database exists", ioex); } LOGGER.debug("Loading database connection"); LOGGER.debug("Connection String: {}", connectionString); @@ -157,11 +157,11 @@ public final class ConnectionFactory { LOGGER.debug("Unable to start the database in server mode; reverting to single user mode"); } catch (SQLException sqlex) { LOGGER.debug("Unable to connect to the database", ex); - throw new DatabaseException("Unable to connect to the database"); + throw new DatabaseException("Unable to connect to the database", ex); } } else { LOGGER.debug("Unable to connect to the database", ex); - throw new DatabaseException("Unable to connect to the database"); + throw new DatabaseException("Unable to connect to the database", ex); } } @@ -170,7 +170,7 @@ public final class ConnectionFactory { createTables(conn); } catch (DatabaseException dex) { LOGGER.debug("", dex); - throw new DatabaseException("Unable to create the database structure"); + throw new DatabaseException("Unable to create the database structure", dex); } } try { @@ -228,7 +228,7 @@ public final class ConnectionFactory { conn = DriverManager.getConnection(connectionString, userName, password); } catch (SQLException ex) { LOGGER.debug("", ex); - throw new DatabaseException("Unable to connect to the database"); + throw new DatabaseException("Unable to connect to the database", ex); } return conn; } @@ -317,7 +317,7 @@ public final class ConnectionFactory { try { databaseProductName = conn.getMetaData().getDatabaseProductName(); } catch (SQLException ex) { - throw new DatabaseException("Unable to get the database product name"); + throw new DatabaseException("Unable to get the database product name", ex); } if ("h2".equalsIgnoreCase(databaseProductName)) { LOGGER.debug("Updating database structure"); @@ -412,7 +412,7 @@ public final class ConnectionFactory { } } catch (SQLException ex) { LOGGER.debug("", ex); - throw new DatabaseException("Unable to check the database schema version"); + throw new DatabaseException("Unable to check the database schema version", ex); } finally { DBUtils.closeResultSet(rs); DBUtils.closeStatement(ps); From 239c5f2e463cc63cac3f77bfc7bd093af2ea3b37 Mon Sep 17 00:00:00 2001 From: Stefan Neuhaus Date: Thu, 13 Jul 2017 21:21:03 +0200 Subject: [PATCH 4/5] Prevent NPE in case the CveDB.getInstance() failed. This NPE masked the actual cause thereby hampering issue analysis --- .../org/owasp/dependencycheck/data/update/NvdCveUpdater.java | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/dependency-check-core/src/main/java/org/owasp/dependencycheck/data/update/NvdCveUpdater.java b/dependency-check-core/src/main/java/org/owasp/dependencycheck/data/update/NvdCveUpdater.java index ee26e8dbd..3ab3c5c56 100644 --- a/dependency-check-core/src/main/java/org/owasp/dependencycheck/data/update/NvdCveUpdater.java +++ b/dependency-check-core/src/main/java/org/owasp/dependencycheck/data/update/NvdCveUpdater.java @@ -172,7 +172,9 @@ public class NvdCveUpdater implements CachedWebDataSource { throw new UpdateException("Database Exception", ex); } finally { shutdownExecutorServices(); - cveDb.close(); + if(cveDb != null) { + cveDb.close(); + } if (lock != null) { try { lock.release(); From cbb10a1b1cd6a451d84633db7681a6a0bae731b1 Mon Sep 17 00:00:00 2001 From: Stefan Neuhaus Date: Thu, 13 Jul 2017 21:22:15 +0200 Subject: [PATCH 5/5] In case of missing resources for prepared statements detect and clearly indicate this issue. --- .../dependencycheck/data/nvdcve/CveDB.java | 39 ++++++++++++------- 1 file changed, 24 insertions(+), 15 deletions(-) diff --git a/dependency-check-core/src/main/java/org/owasp/dependencycheck/data/nvdcve/CveDB.java b/dependency-check-core/src/main/java/org/owasp/dependencycheck/data/nvdcve/CveDB.java index d0e4c09e3..d743c522f 100644 --- a/dependency-check-core/src/main/java/org/owasp/dependencycheck/data/nvdcve/CveDB.java +++ b/dependency-check-core/src/main/java/org/owasp/dependencycheck/data/nvdcve/CveDB.java @@ -258,14 +258,19 @@ public final class CveDB implements AutoCloseable { * database connection */ private synchronized void open() throws DatabaseException { - if (!instance.isOpen()) { - instance.connection = ConnectionFactory.getConnection(); - final String databaseProductName = determineDatabaseProductName(instance.connection); - instance.statementBundle = databaseProductName != null - ? ResourceBundle.getBundle("data/dbStatements", new Locale(databaseProductName)) - : ResourceBundle.getBundle("data/dbStatements"); - instance.prepareStatements(); - instance.databaseProperties = new DatabaseProperties(instance); + try { + if (!instance.isOpen()) { + instance.connection = ConnectionFactory.getConnection(); + final String databaseProductName = determineDatabaseProductName(instance.connection); + instance.statementBundle = databaseProductName != null + ? ResourceBundle.getBundle("data/dbStatements", new Locale(databaseProductName)) + : ResourceBundle.getBundle("data/dbStatements"); + instance.prepareStatements(); + instance.databaseProperties = new DatabaseProperties(instance); + } + } catch(DatabaseException e) { + releaseResources(); + throw e; } } @@ -290,14 +295,18 @@ public final class CveDB implements AutoCloseable { LOGGER.error("There was an exception attempting to close the CveDB, see the log for more details."); LOGGER.debug("", ex); } - instance.statementBundle = null; - instance.preparedStatements.clear(); - instance.databaseProperties = null; - instance.connection = null; + releaseResources(); } } } + private synchronized void releaseResources() { + instance.statementBundle = null; + instance.preparedStatements.clear(); + instance.databaseProperties = null; + instance.connection = null; + } + /** * Returns whether the database connection is open or closed. * @@ -315,15 +324,15 @@ public final class CveDB implements AutoCloseable { */ private void prepareStatements() throws DatabaseException { for (PreparedStatementCveDb key : values()) { - final String statementString = statementBundle.getString(key.name()); final PreparedStatement preparedStatement; try { + final String statementString = statementBundle.getString(key.name()); if (key == INSERT_VULNERABILITY || key == INSERT_CPE) { preparedStatement = connection.prepareStatement(statementString, new String[]{"id"}); } else { preparedStatement = connection.prepareStatement(statementString); } - } catch (SQLException exception) { + } catch (SQLException | MissingResourceException exception) { throw new DatabaseException(exception); } preparedStatements.put(key, preparedStatement); @@ -492,7 +501,7 @@ public final class CveDB implements AutoCloseable { mergeProperty.setString(1, key); mergeProperty.setString(2, value); mergeProperty.executeUpdate(); - } catch (MissingResourceException mre) { + } catch (SQLException e) { // No Merge statement, so doing an Update/Insert... final PreparedStatement updateProperty = getPreparedStatement(UPDATE_PROPERTY); updateProperty.setString(1, value);