From bcd6634d8af1303de08d00e46f07c5bb5b131672 Mon Sep 17 00:00:00 2001 From: Jeremy Long Date: Sun, 4 Sep 2016 18:41:58 -0400 Subject: [PATCH] fixed NPE issues --- .../data/cpe/CpeMemoryIndex.java | 10 +- .../data/update/NvdCveUpdater.java | 161 +++++++++--------- .../dependencycheck/dependency/Reference.java | 19 +-- .../analyzer/CPEAnalyzerIntegrationTest.java | 5 + .../update/NvdCveUpdaterIntegrationTest.java | 11 +- 5 files changed, 99 insertions(+), 107 deletions(-) diff --git a/dependency-check-core/src/main/java/org/owasp/dependencycheck/data/cpe/CpeMemoryIndex.java b/dependency-check-core/src/main/java/org/owasp/dependencycheck/data/cpe/CpeMemoryIndex.java index 5caed2e4f..a61543053 100644 --- a/dependency-check-core/src/main/java/org/owasp/dependencycheck/data/cpe/CpeMemoryIndex.java +++ b/dependency-check-core/src/main/java/org/owasp/dependencycheck/data/cpe/CpeMemoryIndex.java @@ -209,10 +209,12 @@ public final class CpeMemoryIndex { final Set> data = cve.getVendorProductList(); for (Pair pair : data) { - v.setStringValue(pair.getLeft()); - p.setStringValue(pair.getRight()); - indexWriter.addDocument(doc); - resetFieldAnalyzer(); + if (pair.getLeft() != null && pair.getRight() != null) { + v.setStringValue(pair.getLeft()); + p.setStringValue(pair.getRight()); + indexWriter.addDocument(doc); + resetFieldAnalyzer(); + } } } catch (DatabaseException ex) { LOGGER.debug("", ex); 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 a0d871015..b28209af0 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 @@ -77,10 +77,10 @@ public class NvdCveUpdater extends BaseUpdater implements CachedWebDataSource { } if (autoUpdate && checkUpdate()) { final UpdateableNvdCve updateable = getUpdatesNeeded(); - getProperties().save(DatabaseProperties.LAST_CHECKED, Long.toString(System.currentTimeMillis())); if (updateable.isUpdateNeeded()) { performUpdate(updateable); } + getProperties().save(DatabaseProperties.LAST_CHECKED, Long.toString(System.currentTimeMillis())); } } catch (MalformedURLException ex) { throw new UpdateException("NVD CVE properties files contain an invalid URL, unable to update the data to use the most current data.", ex); @@ -156,93 +156,86 @@ public class NvdCveUpdater extends BaseUpdater implements CachedWebDataSource { * @throws UpdateException is thrown if there is an error updating the * database */ - public void performUpdate(UpdateableNvdCve updateable) throws UpdateException { + private void performUpdate(UpdateableNvdCve updateable) throws UpdateException { int maxUpdates = 0; - try { - for (NvdCveInfo cve : updateable) { - if (cve.getNeedsUpdate()) { - maxUpdates += 1; + for (NvdCveInfo cve : updateable) { + if (cve.getNeedsUpdate()) { + maxUpdates += 1; + } + } + if (maxUpdates <= 0) { + return; + } + if (maxUpdates > 3) { + LOGGER.info("NVD CVE requires several updates; this could take a couple of minutes."); + } + + final int poolSize = (MAX_THREAD_POOL_SIZE < maxUpdates) ? MAX_THREAD_POOL_SIZE : maxUpdates; + + final ExecutorService downloadExecutors = Executors.newFixedThreadPool(poolSize); + final ExecutorService processExecutor = Executors.newSingleThreadExecutor(); + final Set>> downloadFutures = new HashSet>>(maxUpdates); + for (NvdCveInfo cve : updateable) { + if (cve.getNeedsUpdate()) { + final DownloadTask call = new DownloadTask(cve, processExecutor, getCveDB(), Settings.getInstance()); + downloadFutures.add(downloadExecutors.submit(call)); + } + } + downloadExecutors.shutdown(); + + //next, move the future future processTasks to just future processTasks + final Set> processFutures = new HashSet>(maxUpdates); + for (Future> future : downloadFutures) { + Future task = null; + try { + task = future.get(); + } catch (InterruptedException ex) { + downloadExecutors.shutdownNow(); + processExecutor.shutdownNow(); + + LOGGER.debug("Thread was interrupted during download", ex); + throw new UpdateException("The download was interrupted", ex); + } catch (ExecutionException ex) { + downloadExecutors.shutdownNow(); + processExecutor.shutdownNow(); + + LOGGER.debug("Thread was interrupted during download execution", ex); + throw new UpdateException("The execution of the download was interrupted", ex); + } + if (task == null) { + downloadExecutors.shutdownNow(); + processExecutor.shutdownNow(); + LOGGER.debug("Thread was interrupted during download"); + throw new UpdateException("The download was interrupted; unable to complete the update"); + } else { + processFutures.add(task); + } + } + + for (Future future : processFutures) { + try { + final ProcessTask task = future.get(); + if (task.getException() != null) { + throw task.getException(); } + } catch (InterruptedException ex) { + processExecutor.shutdownNow(); + LOGGER.debug("Thread was interrupted during processing", ex); + throw new UpdateException(ex); + } catch (ExecutionException ex) { + processExecutor.shutdownNow(); + LOGGER.debug("Execution Exception during process", ex); + throw new UpdateException(ex); + } finally { + processExecutor.shutdown(); } - if (maxUpdates <= 0) { - return; - } - if (maxUpdates > 3) { - LOGGER.info("NVD CVE requires several updates; this could take a couple of minutes."); - } - if (maxUpdates > 0) { - openDataStores(); - } + } - final int poolSize = (MAX_THREAD_POOL_SIZE < maxUpdates) ? MAX_THREAD_POOL_SIZE : maxUpdates; - - final ExecutorService downloadExecutors = Executors.newFixedThreadPool(poolSize); - final ExecutorService processExecutor = Executors.newSingleThreadExecutor(); - final Set>> downloadFutures = new HashSet>>(maxUpdates); - for (NvdCveInfo cve : updateable) { - if (cve.getNeedsUpdate()) { - final DownloadTask call = new DownloadTask(cve, processExecutor, getCveDB(), Settings.getInstance()); - downloadFutures.add(downloadExecutors.submit(call)); - } - } - downloadExecutors.shutdown(); - - //next, move the future future processTasks to just future processTasks - final Set> processFutures = new HashSet>(maxUpdates); - for (Future> future : downloadFutures) { - Future task = null; - try { - task = future.get(); - } catch (InterruptedException ex) { - downloadExecutors.shutdownNow(); - processExecutor.shutdownNow(); - - LOGGER.debug("Thread was interrupted during download", ex); - throw new UpdateException("The download was interrupted", ex); - } catch (ExecutionException ex) { - downloadExecutors.shutdownNow(); - processExecutor.shutdownNow(); - - LOGGER.debug("Thread was interrupted during download execution", ex); - throw new UpdateException("The execution of the download was interrupted", ex); - } - if (task == null) { - downloadExecutors.shutdownNow(); - processExecutor.shutdownNow(); - LOGGER.debug("Thread was interrupted during download"); - throw new UpdateException("The download was interrupted; unable to complete the update"); - } else { - processFutures.add(task); - } - } - - for (Future future : processFutures) { - try { - final ProcessTask task = future.get(); - if (task.getException() != null) { - throw task.getException(); - } - } catch (InterruptedException ex) { - processExecutor.shutdownNow(); - LOGGER.debug("Thread was interrupted during processing", ex); - throw new UpdateException(ex); - } catch (ExecutionException ex) { - processExecutor.shutdownNow(); - LOGGER.debug("Execution Exception during process", ex); - throw new UpdateException(ex); - } finally { - processExecutor.shutdown(); - } - } - - if (maxUpdates >= 1) { //ensure the modified file date gets written (we may not have actually updated it) - getProperties().save(updateable.get(MODIFIED)); - LOGGER.info("Begin database maintenance."); - getCveDB().cleanupDatabase(); - LOGGER.info("End database maintenance."); - } - } finally { - closeDataStores(); + if (maxUpdates >= 1) { //ensure the modified file date gets written (we may not have actually updated it) + getProperties().save(updateable.get(MODIFIED)); + LOGGER.info("Begin database maintenance."); + getCveDB().cleanupDatabase(); + LOGGER.info("End database maintenance."); } } diff --git a/dependency-check-core/src/main/java/org/owasp/dependencycheck/dependency/Reference.java b/dependency-check-core/src/main/java/org/owasp/dependencycheck/dependency/Reference.java index 5be391a27..7e6baebd6 100644 --- a/dependency-check-core/src/main/java/org/owasp/dependencycheck/dependency/Reference.java +++ b/dependency-check-core/src/main/java/org/owasp/dependencycheck/dependency/Reference.java @@ -18,6 +18,7 @@ package org.owasp.dependencycheck.dependency; import java.io.Serializable; +import org.apache.commons.lang3.builder.CompareToBuilder; /** * An external reference for a vulnerability. This contains a name, URL, and a @@ -141,18 +142,10 @@ public class Reference implements Serializable, Comparable { */ @Override public int compareTo(Reference o) { - if (source.equals(o.source)) { - if (name.equals(o.name)) { - if (url.equals(o.url)) { - return 0; //they are equal - } else { - return url.compareTo(o.url); - } - } else { - return name.compareTo(o.name); - } - } else { - return source.compareTo(o.source); - } + return new CompareToBuilder() + .append(source, o.source) + .append(name, o.name) + .append(url, o.url) + .toComparison(); } } diff --git a/dependency-check-core/src/test/java/org/owasp/dependencycheck/analyzer/CPEAnalyzerIntegrationTest.java b/dependency-check-core/src/test/java/org/owasp/dependencycheck/analyzer/CPEAnalyzerIntegrationTest.java index 552ec2abc..75a724471 100644 --- a/dependency-check-core/src/test/java/org/owasp/dependencycheck/analyzer/CPEAnalyzerIntegrationTest.java +++ b/dependency-check-core/src/test/java/org/owasp/dependencycheck/analyzer/CPEAnalyzerIntegrationTest.java @@ -183,12 +183,17 @@ public class CPEAnalyzerIntegrationTest extends BaseDBTestCase { hintAnalyzer.analyze(spring3, null); CPEAnalyzer instance = new CPEAnalyzer(); + try { instance.open(); instance.determineCPE(commonValidator); instance.determineCPE(struts); instance.determineCPE(spring); instance.determineCPE(spring3); instance.close(); + } catch (Throwable ex) { + ex.printStackTrace(); + } + String expResult = "cpe:/a:apache:struts:2.1.2"; Identifier expIdentifier = new Identifier("cpe", expResult, expResult); diff --git a/dependency-check-core/src/test/java/org/owasp/dependencycheck/data/update/NvdCveUpdaterIntegrationTest.java b/dependency-check-core/src/test/java/org/owasp/dependencycheck/data/update/NvdCveUpdaterIntegrationTest.java index 8f43f1a09..d666c9772 100644 --- a/dependency-check-core/src/test/java/org/owasp/dependencycheck/data/update/NvdCveUpdaterIntegrationTest.java +++ b/dependency-check-core/src/test/java/org/owasp/dependencycheck/data/update/NvdCveUpdaterIntegrationTest.java @@ -40,12 +40,11 @@ public class NvdCveUpdaterIntegrationTest extends BaseTest { // /** // * Test of update method, of class StandardUpdate. // */ -// @Test -// public void testUpdate() throws Exception { -// StandardUpdate instance = getStandardUpdateTask(); -// instance.update(); -// //TODO make this an actual test -// } + @Test + public void testUpdate() throws Exception { + NvdCveUpdater instance = getUpdater(); + instance.update(); + } /** * Test of updatesNeeded method, of class StandardUpdate. */