From ece4a51b942e52f646e356efb3e6a99c49a71d3b Mon Sep 17 00:00:00 2001 From: Anthony Whitford Date: Wed, 9 Sep 2015 23:18:38 -0700 Subject: [PATCH 1/7] Replaced update or insert property logic with merge property logic. --- .../dependencycheck/data/nvdcve/CveDB.java | 43 ++++++------------- .../resources/data/dbStatements.properties | 3 +- 2 files changed, 13 insertions(+), 33 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 4ab780755..741893289 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,12 +258,10 @@ public class CveDB { * @param props a collection of properties */ void saveProperties(Properties props) { - PreparedStatement updateProperty = null; - PreparedStatement insertProperty = null; + PreparedStatement mergeProperty = null; try { try { - updateProperty = getConnection().prepareStatement(statementBundle.getString("UPDATE_PROPERTY")); - insertProperty = getConnection().prepareStatement(statementBundle.getString("INSERT_PROPERTY")); + mergeProperty = getConnection().prepareStatement(statementBundle.getString("MERGE_PROPERTY")); } catch (SQLException ex) { LOGGER.warn("Unable to save properties to the database"); LOGGER.debug("Unable to save properties to the database", ex); @@ -273,20 +271,16 @@ public class CveDB { final String key = entry.getKey().toString(); final String value = entry.getValue().toString(); try { - updateProperty.setString(1, value); - updateProperty.setString(2, key); - if (updateProperty.executeUpdate() == 0) { - insertProperty.setString(1, key); - insertProperty.setString(2, value); - } + mergeProperty.setString(1, key); + mergeProperty.setString(2, value); + mergeProperty.executeUpdate(); } catch (SQLException ex) { LOGGER.warn("Unable to save property '{}' with a value of '{}' to the database", key, value); LOGGER.debug("", ex); } } } finally { - DBUtils.closeStatement(updateProperty); - DBUtils.closeStatement(insertProperty); + DBUtils.closeStatement(mergeProperty); } } @@ -297,38 +291,25 @@ public class CveDB { * @param value the property value */ void saveProperty(String key, String value) { - PreparedStatement updateProperty = null; - PreparedStatement insertProperty = null; + PreparedStatement mergeProperty = null; try { try { - updateProperty = getConnection().prepareStatement(statementBundle.getString("UPDATE_PROPERTY")); + mergeProperty = getConnection().prepareStatement(statementBundle.getString("MERGE_PROPERTY")); } catch (SQLException ex) { LOGGER.warn("Unable to save properties to the database"); LOGGER.debug("Unable to save properties to the database", ex); return; } try { - updateProperty.setString(1, value); - updateProperty.setString(2, key); - if (updateProperty.executeUpdate() == 0) { - try { - insertProperty = getConnection().prepareStatement(statementBundle.getString("INSERT_PROPERTY")); - } catch (SQLException ex) { - LOGGER.warn("Unable to save properties to the database"); - LOGGER.debug("Unable to save properties to the database", ex); - return; - } - insertProperty.setString(1, key); - insertProperty.setString(2, value); - insertProperty.execute(); - } + mergeProperty.setString(1, key); + mergeProperty.setString(2, value); + mergeProperty.executeUpdate(); } catch (SQLException ex) { LOGGER.warn("Unable to save property '{}' with a value of '{}' to the database", key, value); LOGGER.debug("", ex); } } finally { - DBUtils.closeStatement(updateProperty); - DBUtils.closeStatement(insertProperty); + DBUtils.closeStatement(mergeProperty); } } diff --git a/dependency-check-core/src/main/resources/data/dbStatements.properties b/dependency-check-core/src/main/resources/data/dbStatements.properties index e612f259e..02f3bca8c 100644 --- a/dependency-check-core/src/main/resources/data/dbStatements.properties +++ b/dependency-check-core/src/main/resources/data/dbStatements.properties @@ -31,8 +31,7 @@ SELECT_VULNERABILITY=SELECT id, description, cwe, cvssScore, cvssAccessVector, c SELECT_VULNERABILITY_ID=SELECT id FROM vulnerability WHERE cve = ? SELECT_PROPERTIES=SELECT id, value FROM properties SELECT_PROPERTY=SELECT id, value FROM properties WHERE id = ? -INSERT_PROPERTY=INSERT INTO properties (id, value) VALUES (?, ?) -UPDATE_PROPERTY=UPDATE properties SET value = ? WHERE id = ? +MERGE_PROPERTY=MERGE INTO properties (id, value) KEY(id) VALUES(?, ?) DELETE_PROPERTY=DELETE FROM properties WHERE id = ? DELETE_UNUSED_DICT_CPE=DELETE FROM cpeEntry WHERE dictionaryEntry=true AND id NOT IN (SELECT cpeEntryId FROM software) From 45658afd89a7a33c95300c0592b2fdc94a3edd90 Mon Sep 17 00:00:00 2001 From: Anthony Whitford Date: Wed, 9 Sep 2015 23:20:51 -0700 Subject: [PATCH 2/7] Replaced empty string equals check with an isEmpty check. --- .../org/owasp/dependencycheck/analyzer/AssemblyAnalyzer.java | 4 ++-- .../java/org/owasp/dependencycheck/analyzer/CPEAnalyzer.java | 2 +- .../org/owasp/dependencycheck/data/nexus/NexusSearch.java | 4 ++-- 3 files changed, 5 insertions(+), 5 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 dc60c485b..8e5e20a1c 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 @@ -147,7 +147,7 @@ public class AssemblyAnalyzer extends AbstractFileTypeAnalyzer { // First, see if there was an error final String error = xpath.evaluate("/assembly/error", doc); - if (error != null && !"".equals(error)) { + if (error != null && !error.isEmpty()) { throw new AnalysisException(error); } @@ -246,7 +246,7 @@ public class AssemblyAnalyzer extends AbstractFileTypeAnalyzer { final Document doc = DocumentBuilderFactory.newInstance().newDocumentBuilder().parse(p.getInputStream()); final XPath xpath = XPathFactory.newInstance().newXPath(); final String error = xpath.evaluate("/assembly/error", doc); - if (p.waitFor() != 1 || error == null || "".equals(error)) { + if (p.waitFor() != 1 || error == null || error.isEmpty()) { LOGGER.warn("An error occurred with the .NET AssemblyAnalyzer, please see the log for more details."); LOGGER.debug("GrokAssembly.exe is not working properly"); grokAssemblyExe = null; diff --git a/dependency-check-core/src/main/java/org/owasp/dependencycheck/analyzer/CPEAnalyzer.java b/dependency-check-core/src/main/java/org/owasp/dependencycheck/analyzer/CPEAnalyzer.java index e153ff2a3..367fe376a 100644 --- a/dependency-check-core/src/main/java/org/owasp/dependencycheck/analyzer/CPEAnalyzer.java +++ b/dependency-check-core/src/main/java/org/owasp/dependencycheck/analyzer/CPEAnalyzer.java @@ -339,7 +339,7 @@ public class CPEAnalyzer implements Analyzer { final String cleanText = cleanseText(searchText); - if ("".equals(cleanText)) { + if (cleanText.isEmpty()) { return false; } diff --git a/dependency-check-core/src/main/java/org/owasp/dependencycheck/data/nexus/NexusSearch.java b/dependency-check-core/src/main/java/org/owasp/dependencycheck/data/nexus/NexusSearch.java index e0863d7f3..3c53504b1 100644 --- a/dependency-check-core/src/main/java/org/owasp/dependencycheck/data/nexus/NexusSearch.java +++ b/dependency-check-core/src/main/java/org/owasp/dependencycheck/data/nexus/NexusSearch.java @@ -132,10 +132,10 @@ public class NexusSearch { "/org.sonatype.nexus.rest.model.NexusArtifact/pomLink", doc); final MavenArtifact ma = new MavenArtifact(groupId, artifactId, version); - if (link != null && !"".equals(link)) { + if (link != null && !link.isEmpty()) { ma.setArtifactUrl(link); } - if (pomLink != null && !"".equals(pomLink)) { + if (pomLink != null && !pomLink.isEmpty()) { ma.setPomUrl(pomLink); } return ma; From 5702f3918151914cc6c06c369f153f73c90ffd78 Mon Sep 17 00:00:00 2001 From: Anthony Whitford Date: Wed, 9 Sep 2015 23:54:20 -0700 Subject: [PATCH 3/7] Addressed possible resource leak. --- .../org/owasp/dependencycheck/data/nvdcve/CveDB.java | 11 +++++------ 1 file changed, 5 insertions(+), 6 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 741893289..b550b2458 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 @@ -321,7 +321,6 @@ public class CveDB { * @throws DatabaseException thrown if there is an exception retrieving data */ public List getVulnerabilities(String cpeStr) throws DatabaseException { - ResultSet rs = null; final VulnerableSoftware cpe = new VulnerableSoftware(); try { cpe.parseName(cpeStr); @@ -331,7 +330,8 @@ public class CveDB { final DependencyVersion detectedVersion = parseDependencyVersion(cpe); final List vulnerabilities = new ArrayList(); - PreparedStatement ps; + PreparedStatement ps = null; + ResultSet rs = null; try { ps = getConnection().prepareStatement(statementBundle.getString("SELECT_CVE_FROM_SOFTWARE")); ps.setString(1, cpe.getVendor()); @@ -365,12 +365,11 @@ public class CveDB { v.setMatchedCPE(matchedCPE.getKey(), matchedCPE.getValue() ? "Y" : null); vulnerabilities.add(v); } - DBUtils.closeResultSet(rs); - DBUtils.closeStatement(ps); } catch (SQLException ex) { throw new DatabaseException("Exception retrieving vulnerability for " + cpeStr, ex); } finally { DBUtils.closeResultSet(rs); + DBUtils.closeStatement(ps); } return vulnerabilities; } @@ -748,9 +747,9 @@ public class CveDB { * @return a dependency version */ private DependencyVersion parseDependencyVersion(VulnerableSoftware cpe) { - DependencyVersion cpeVersion; + final DependencyVersion cpeVersion; if (cpe.getVersion() != null && !cpe.getVersion().isEmpty()) { - String versionText; + final String versionText; if (cpe.getUpdate() != null && !cpe.getUpdate().isEmpty()) { versionText = String.format("%s.%s", cpe.getVersion(), cpe.getUpdate()); } else { From fde415e25174adc3e30214c12beaee9b6e2c5ddc Mon Sep 17 00:00:00 2001 From: Anthony Whitford Date: Thu, 10 Sep 2015 00:05:04 -0700 Subject: [PATCH 4/7] Added missing serialVersionUID. --- .../org/owasp/dependencycheck/dependency/Dependency.java | 4 ++++ .../java/org/owasp/dependencycheck/dependency/Evidence.java | 4 ++++ .../owasp/dependencycheck/dependency/EvidenceCollection.java | 4 ++++ .../org/owasp/dependencycheck/dependency/Identifier.java | 5 +++++ .../org/owasp/dependencycheck/xml/pom/PomParseException.java | 5 +++++ 5 files changed, 22 insertions(+) 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 20c896dc6..a847aba13 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 @@ -43,6 +43,10 @@ import org.slf4j.LoggerFactory; */ public class Dependency implements Serializable, Comparable { + /** + * The serial version UID for serialization. + */ + private static final long serialVersionUID = 1L; /** * The logger. */ diff --git a/dependency-check-core/src/main/java/org/owasp/dependencycheck/dependency/Evidence.java b/dependency-check-core/src/main/java/org/owasp/dependencycheck/dependency/Evidence.java index de550e60c..e95fe7d11 100644 --- a/dependency-check-core/src/main/java/org/owasp/dependencycheck/dependency/Evidence.java +++ b/dependency-check-core/src/main/java/org/owasp/dependencycheck/dependency/Evidence.java @@ -29,6 +29,10 @@ import java.io.Serializable; */ public class Evidence implements Serializable, Comparable { + /** + * The serial version UID for serialization. + */ + private static final long serialVersionUID = 1L; /** * Used as starting point for generating the value in {@link #hashCode()}. */ diff --git a/dependency-check-core/src/main/java/org/owasp/dependencycheck/dependency/EvidenceCollection.java b/dependency-check-core/src/main/java/org/owasp/dependencycheck/dependency/EvidenceCollection.java index 6cadd85a9..eabfee9ed 100644 --- a/dependency-check-core/src/main/java/org/owasp/dependencycheck/dependency/EvidenceCollection.java +++ b/dependency-check-core/src/main/java/org/owasp/dependencycheck/dependency/EvidenceCollection.java @@ -39,6 +39,10 @@ import org.slf4j.LoggerFactory; */ public class EvidenceCollection implements Serializable, Iterable { + /** + * The serial version UID for serialization. + */ + private static final long serialVersionUID = 1L; /** * The logger. */ diff --git a/dependency-check-core/src/main/java/org/owasp/dependencycheck/dependency/Identifier.java b/dependency-check-core/src/main/java/org/owasp/dependencycheck/dependency/Identifier.java index e392429ae..ff09efeb9 100644 --- a/dependency-check-core/src/main/java/org/owasp/dependencycheck/dependency/Identifier.java +++ b/dependency-check-core/src/main/java/org/owasp/dependencycheck/dependency/Identifier.java @@ -25,6 +25,11 @@ import java.io.Serializable; */ public class Identifier implements Serializable, Comparable { + /** + * The serial version UID for serialization. + */ + private static final long serialVersionUID = 1L; + /** * Default constructor. Should only be used for automatic class * creation as is the case with many XML parsers (for the parsing diff --git a/dependency-check-core/src/main/java/org/owasp/dependencycheck/xml/pom/PomParseException.java b/dependency-check-core/src/main/java/org/owasp/dependencycheck/xml/pom/PomParseException.java index 0a9e5be98..be98e127c 100644 --- a/dependency-check-core/src/main/java/org/owasp/dependencycheck/xml/pom/PomParseException.java +++ b/dependency-check-core/src/main/java/org/owasp/dependencycheck/xml/pom/PomParseException.java @@ -26,6 +26,11 @@ import java.io.IOException; */ public class PomParseException extends IOException { + /** + * The serial version UID for serialization. + */ + private static final long serialVersionUID = 1L; + /** * Creates a new SuppressionParseException. */ From d98f67eab9bae184a245f44165636739afba5c10 Mon Sep 17 00:00:00 2001 From: Anthony Whitford Date: Thu, 10 Sep 2015 00:20:03 -0700 Subject: [PATCH 5/7] Added missing serialVersionUID. --- .../suppression/SuppressionParseException.java | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/dependency-check-core/src/main/java/org/owasp/dependencycheck/suppression/SuppressionParseException.java b/dependency-check-core/src/main/java/org/owasp/dependencycheck/suppression/SuppressionParseException.java index 1ee622791..6c8e938de 100644 --- a/dependency-check-core/src/main/java/org/owasp/dependencycheck/suppression/SuppressionParseException.java +++ b/dependency-check-core/src/main/java/org/owasp/dependencycheck/suppression/SuppressionParseException.java @@ -26,6 +26,11 @@ import java.io.IOException; */ public class SuppressionParseException extends IOException { + /** + * The serial version UID for serialization. + */ + private static final long serialVersionUID = 1L; + /** * Creates a new SuppressionParseException. */ From 3746df49ee5cc156d5223df46d43710e5a275c83 Mon Sep 17 00:00:00 2001 From: Anthony Whitford Date: Thu, 10 Sep 2015 00:21:54 -0700 Subject: [PATCH 6/7] Added type declarations. --- .../org/owasp/dependencycheck/data/cpe/CpeMemoryIndex.java | 2 +- .../org/owasp/dependencycheck/utils/DependencyVersion.java | 4 ++-- 2 files changed, 3 insertions(+), 3 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 3f80b2d8f..dc8a5edf5 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 @@ -151,7 +151,7 @@ public final class CpeMemoryIndex { */ @SuppressWarnings("unchecked") private Analyzer createIndexingAnalyzer() { - final Map fieldAnalyzers = new HashMap(); + final Map fieldAnalyzers = new HashMap(); fieldAnalyzers.put(Fields.DOCUMENT_KEY, new KeywordAnalyzer()); return new PerFieldAnalyzerWrapper(new FieldAnalyzer(LuceneUtils.CURRENT_VERSION), fieldAnalyzers); } diff --git a/dependency-check-core/src/main/java/org/owasp/dependencycheck/utils/DependencyVersion.java b/dependency-check-core/src/main/java/org/owasp/dependencycheck/utils/DependencyVersion.java index 7f27a0db0..ec1036d28 100644 --- a/dependency-check-core/src/main/java/org/owasp/dependencycheck/utils/DependencyVersion.java +++ b/dependency-check-core/src/main/java/org/owasp/dependencycheck/utils/DependencyVersion.java @@ -37,7 +37,7 @@ import org.apache.commons.lang3.StringUtils; * * @author Jeremy Long */ -public class DependencyVersion implements Iterable, Comparable { +public class DependencyVersion implements Iterable, Comparable { /** * Constructor for a empty DependencyVersion. @@ -103,7 +103,7 @@ public class DependencyVersion implements Iterable, Comparable iterator() { return versionParts.iterator(); } From 11a3db5d645a38273b6554397bfdd7527a98ceb1 Mon Sep 17 00:00:00 2001 From: Anthony Whitford Date: Thu, 10 Sep 2015 23:21:44 -0700 Subject: [PATCH 7/7] Revert "Replaced update or insert property logic with merge property logic." This reverts commit ece4a51b942e52f646e356efb3e6a99c49a71d3b. --- .../dependencycheck/data/nvdcve/CveDB.java | 43 +++++++++++++------ .../resources/data/dbStatements.properties | 3 +- 2 files changed, 33 insertions(+), 13 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 b550b2458..b6adc49bf 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,10 +258,12 @@ public class CveDB { * @param props a collection of properties */ void saveProperties(Properties props) { - PreparedStatement mergeProperty = null; + PreparedStatement updateProperty = null; + PreparedStatement insertProperty = null; try { try { - mergeProperty = getConnection().prepareStatement(statementBundle.getString("MERGE_PROPERTY")); + updateProperty = getConnection().prepareStatement(statementBundle.getString("UPDATE_PROPERTY")); + insertProperty = getConnection().prepareStatement(statementBundle.getString("INSERT_PROPERTY")); } catch (SQLException ex) { LOGGER.warn("Unable to save properties to the database"); LOGGER.debug("Unable to save properties to the database", ex); @@ -271,16 +273,20 @@ public class CveDB { final String key = entry.getKey().toString(); final String value = entry.getValue().toString(); try { - mergeProperty.setString(1, key); - mergeProperty.setString(2, value); - mergeProperty.executeUpdate(); + updateProperty.setString(1, value); + updateProperty.setString(2, key); + if (updateProperty.executeUpdate() == 0) { + insertProperty.setString(1, key); + insertProperty.setString(2, value); + } } catch (SQLException ex) { LOGGER.warn("Unable to save property '{}' with a value of '{}' to the database", key, value); LOGGER.debug("", ex); } } } finally { - DBUtils.closeStatement(mergeProperty); + DBUtils.closeStatement(updateProperty); + DBUtils.closeStatement(insertProperty); } } @@ -291,25 +297,38 @@ public class CveDB { * @param value the property value */ void saveProperty(String key, String value) { - PreparedStatement mergeProperty = null; + PreparedStatement updateProperty = null; + PreparedStatement insertProperty = null; try { try { - mergeProperty = getConnection().prepareStatement(statementBundle.getString("MERGE_PROPERTY")); + updateProperty = getConnection().prepareStatement(statementBundle.getString("UPDATE_PROPERTY")); } catch (SQLException ex) { LOGGER.warn("Unable to save properties to the database"); LOGGER.debug("Unable to save properties to the database", ex); return; } try { - mergeProperty.setString(1, key); - mergeProperty.setString(2, value); - mergeProperty.executeUpdate(); + updateProperty.setString(1, value); + updateProperty.setString(2, key); + if (updateProperty.executeUpdate() == 0) { + try { + insertProperty = getConnection().prepareStatement(statementBundle.getString("INSERT_PROPERTY")); + } catch (SQLException ex) { + LOGGER.warn("Unable to save properties to the database"); + LOGGER.debug("Unable to save properties to the database", ex); + return; + } + insertProperty.setString(1, key); + insertProperty.setString(2, value); + insertProperty.execute(); + } } catch (SQLException ex) { LOGGER.warn("Unable to save property '{}' with a value of '{}' to the database", key, value); LOGGER.debug("", ex); } } finally { - DBUtils.closeStatement(mergeProperty); + DBUtils.closeStatement(updateProperty); + DBUtils.closeStatement(insertProperty); } } diff --git a/dependency-check-core/src/main/resources/data/dbStatements.properties b/dependency-check-core/src/main/resources/data/dbStatements.properties index 02f3bca8c..e612f259e 100644 --- a/dependency-check-core/src/main/resources/data/dbStatements.properties +++ b/dependency-check-core/src/main/resources/data/dbStatements.properties @@ -31,7 +31,8 @@ SELECT_VULNERABILITY=SELECT id, description, cwe, cvssScore, cvssAccessVector, c SELECT_VULNERABILITY_ID=SELECT id FROM vulnerability WHERE cve = ? SELECT_PROPERTIES=SELECT id, value FROM properties SELECT_PROPERTY=SELECT id, value FROM properties WHERE id = ? -MERGE_PROPERTY=MERGE INTO properties (id, value) KEY(id) VALUES(?, ?) +INSERT_PROPERTY=INSERT INTO properties (id, value) VALUES (?, ?) +UPDATE_PROPERTY=UPDATE properties SET value = ? WHERE id = ? DELETE_PROPERTY=DELETE FROM properties WHERE id = ? DELETE_UNUSED_DICT_CPE=DELETE FROM cpeEntry WHERE dictionaryEntry=true AND id NOT IN (SELECT cpeEntryId FROM software)