From e4b7f7aa8fea6dbc5fa53484b5b8f3121aeecce2 Mon Sep 17 00:00:00 2001 From: Jeremy Long Date: Mon, 20 Nov 2017 06:46:25 -0500 Subject: [PATCH] update to ensure NodePackageAnalyzer will not run without a backing vulnerability analyzer --- .../org/owasp/dependencycheck/Engine.java | 9 +++++ .../analyzer/NodePackageAnalyzer.java | 33 ++++++++++++++++++- .../dependencycheck/analyzer/NspAnalyzer.java | 2 +- .../analyzer/NvdCveAnalyzer.java | 30 ++++++++++++++++- .../main/resources/dependencycheck.properties | 4 ++- .../analyzer/NodePackageAnalyzerTest.java | 8 +++-- .../test/resources/dependencycheck.properties | 2 ++ .../owasp/dependencycheck/utils/Settings.java | 4 +++ 8 files changed, 86 insertions(+), 6 deletions(-) diff --git a/dependency-check-core/src/main/java/org/owasp/dependencycheck/Engine.java b/dependency-check-core/src/main/java/org/owasp/dependencycheck/Engine.java index 41219c2aa..09a0066d9 100644 --- a/dependency-check-core/src/main/java/org/owasp/dependencycheck/Engine.java +++ b/dependency-check-core/src/main/java/org/owasp/dependencycheck/Engine.java @@ -1038,6 +1038,15 @@ public class Engine implements FileFilter, AutoCloseable { return settings; } + /** + * Returns the mode of the engine. + * + * @return the mode of the engine + */ + public Mode getMode() { + return mode; + } + /** * Adds a file type analyzer. This has been added solely to assist in unit * testing the Engine. diff --git a/dependency-check-core/src/main/java/org/owasp/dependencycheck/analyzer/NodePackageAnalyzer.java b/dependency-check-core/src/main/java/org/owasp/dependencycheck/analyzer/NodePackageAnalyzer.java index ddbcc8e04..c0f5b2dce 100644 --- a/dependency-check-core/src/main/java/org/owasp/dependencycheck/analyzer/NodePackageAnalyzer.java +++ b/dependency-check-core/src/main/java/org/owasp/dependencycheck/analyzer/NodePackageAnalyzer.java @@ -30,6 +30,8 @@ import org.slf4j.LoggerFactory; import java.io.File; import java.io.FileFilter; import java.io.IOException; +import java.util.Arrays; +import java.util.List; import java.util.Map; import javax.annotation.concurrent.ThreadSafe; import javax.json.Json; @@ -38,8 +40,10 @@ import javax.json.JsonObject; import javax.json.JsonReader; import javax.json.JsonString; import javax.json.JsonValue; +import org.owasp.dependencycheck.Engine.Mode; import org.owasp.dependencycheck.exception.InitializationException; import org.owasp.dependencycheck.dependency.EvidenceType; +import org.owasp.dependencycheck.utils.InvalidSettingException; /** * Used to analyze Node Package Manager (npm) package.json files, and collect @@ -87,9 +91,35 @@ public class NodePackageAnalyzer extends AbstractFileTypeAnalyzer { return PACKAGE_JSON_FILTER; } + /** + * Performs validation on the configuration to ensure that the correct + * analyzers are in place. + * + * @param engine the dependency-check engine + * @throws InitializationException thrown if there is a configuration error + */ @Override protected void prepareFileTypeAnalyzer(Engine engine) throws InitializationException { - // NO-OP + if (engine.getMode() != Mode.EVIDENCE_COLLECTION) { + try { + Settings settings = engine.getSettings(); + final String[] tmp = settings.getArray(Settings.KEYS.ECOSYSTEM_SKIP_NVDCVE); + if (tmp != null) { + List skipEcosystems = Arrays.asList(tmp); + if (skipEcosystems.contains(DEPENDENCY_ECOSYSTEM) + && !settings.getBoolean(Settings.KEYS.ANALYZER_NSP_PACKAGE_ENABLED)) { + LOGGER.debug("NodePackageAnalyzer enabled without a corresponding vulnerability analyzer"); + final String msg = "Invalid Configuration: enabling the Node Package Analyzer without " + + "using the NSP Analyzer is not supported."; + throw new InitializationException(msg); + } else if (!skipEcosystems.contains(DEPENDENCY_ECOSYSTEM)) { + LOGGER.warn("Using the NVD CVE Analyzer with Node.js can result in many false positives."); + } + } + } catch (InvalidSettingException ex) { + throw new InitializationException("Unable to read configuration settings", ex); + } + } } /** @@ -144,6 +174,7 @@ public class NodePackageAnalyzer extends AbstractFileTypeAnalyzer { /** * Collects evidence from the given JSON for the associated dependency. + * * @param json the JSON that contains the evidence to collect * @param dependency the dependency to add the evidence too */ diff --git a/dependency-check-core/src/main/java/org/owasp/dependencycheck/analyzer/NspAnalyzer.java b/dependency-check-core/src/main/java/org/owasp/dependencycheck/analyzer/NspAnalyzer.java index 03f1ae1d5..cd04af940 100644 --- a/dependency-check-core/src/main/java/org/owasp/dependencycheck/analyzer/NspAnalyzer.java +++ b/dependency-check-core/src/main/java/org/owasp/dependencycheck/analyzer/NspAnalyzer.java @@ -75,7 +75,7 @@ public class NspAnalyzer extends AbstractFileTypeAnalyzer { * A descriptor for the type of dependencies processed or added by this * analyzer. */ - public static final String DEPENDENCY_ECOSYSTEM = "npm"; + public static final String DEPENDENCY_ECOSYSTEM = NodePackageAnalyzer.DEPENDENCY_ECOSYSTEM; /** * The file name to scan. */ diff --git a/dependency-check-core/src/main/java/org/owasp/dependencycheck/analyzer/NvdCveAnalyzer.java b/dependency-check-core/src/main/java/org/owasp/dependencycheck/analyzer/NvdCveAnalyzer.java index 6a6e82060..d3826563a 100644 --- a/dependency-check-core/src/main/java/org/owasp/dependencycheck/analyzer/NvdCveAnalyzer.java +++ b/dependency-check-core/src/main/java/org/owasp/dependencycheck/analyzer/NvdCveAnalyzer.java @@ -17,7 +17,10 @@ */ package org.owasp.dependencycheck.analyzer; +import java.util.ArrayList; +import java.util.Arrays; import java.util.List; +import java.util.Set; import javax.annotation.concurrent.ThreadSafe; import org.owasp.dependencycheck.Engine; import org.owasp.dependencycheck.analyzer.exception.AnalysisException; @@ -27,6 +30,7 @@ import org.owasp.dependencycheck.dependency.Dependency; import org.owasp.dependencycheck.dependency.Identifier; import org.owasp.dependencycheck.dependency.Vulnerability; import org.owasp.dependencycheck.utils.Settings; +import org.slf4j.LoggerFactory; /** * NvdCveAnalyzer is a utility class that takes a project dependency and @@ -41,7 +45,27 @@ public class NvdCveAnalyzer extends AbstractAnalyzer { /** * The Logger for use throughout the class */ - //private static final org.slf4j.Logger LOGGER = LoggerFactory.getLogger(NvdCveAnalyzer.class); + private static final org.slf4j.Logger LOGGER = LoggerFactory.getLogger(NvdCveAnalyzer.class); + + private List skipEcosystems; + + /** + * Initializes the analyzer with the configured settings. + * + * @param settings the configured settings to use + */ + @Override + public void initialize(Settings settings) { + super.initialize(settings); + final String[] tmp = settings.getArray(Settings.KEYS.ECOSYSTEM_SKIP_NVDCVE); + if (tmp == null) { + skipEcosystems = new ArrayList<>(); + } else { + LOGGER.info("Skipping NVD CVE Analysis for {}", tmp); + skipEcosystems = Arrays.asList(tmp); + } + } + /** * Analyzes a dependency and attempts to determine if there are any CPE * identifiers for this dependency. @@ -53,6 +77,10 @@ public class NvdCveAnalyzer extends AbstractAnalyzer { */ @Override protected void analyzeDependency(Dependency dependency, Engine engine) throws AnalysisException { + if (skipEcosystems.contains(dependency.getEcosystem())) { + return; + } + final CveDB cveDB = engine.getDatabase(); for (Identifier id : dependency.getIdentifiers()) { if ("cpe".equals(id.getType())) { diff --git a/dependency-check-core/src/main/resources/dependencycheck.properties b/dependency-check-core/src/main/resources/dependencycheck.properties index 0a6053d36..ed7e23256 100644 --- a/dependency-check-core/src/main/resources/dependencycheck.properties +++ b/dependency-check-core/src/main/resources/dependencycheck.properties @@ -126,4 +126,6 @@ analyzer.nvdcve.enabled=true analyzer.vulnerabilitysuppression.enabled=true updater.nvdcve.enabled=true updater.versioncheck.enabled=true -analyzer.versionfilter.enabled=true \ No newline at end of file +analyzer.versionfilter.enabled=true + +ecosystem.skip.nvdcve=npm \ No newline at end of file diff --git a/dependency-check-core/src/test/java/org/owasp/dependencycheck/analyzer/NodePackageAnalyzerTest.java b/dependency-check-core/src/test/java/org/owasp/dependencycheck/analyzer/NodePackageAnalyzerTest.java index bb21a13d8..9b0cc2b41 100644 --- a/dependency-check-core/src/test/java/org/owasp/dependencycheck/analyzer/NodePackageAnalyzerTest.java +++ b/dependency-check-core/src/test/java/org/owasp/dependencycheck/analyzer/NodePackageAnalyzerTest.java @@ -29,6 +29,7 @@ import java.io.File; import static org.hamcrest.CoreMatchers.containsString; import static org.hamcrest.CoreMatchers.is; import static org.junit.Assert.*; +import org.owasp.dependencycheck.Engine; import org.owasp.dependencycheck.dependency.EvidenceType; /** @@ -42,6 +43,7 @@ public class NodePackageAnalyzerTest extends BaseTest { * The analyzer to test. */ private NodePackageAnalyzer analyzer; + private Engine engine; /** * Correctly setup the analyzer for testing. @@ -52,14 +54,15 @@ public class NodePackageAnalyzerTest extends BaseTest { @Override public void setUp() throws Exception { super.setUp(); + engine = new Engine(this.getSettings()); analyzer = new NodePackageAnalyzer(); analyzer.setFilesMatched(true); analyzer.initialize(getSettings()); - analyzer.prepare(null); + analyzer.prepare(engine); } /** - * Cleanup the analyzer's temp files, etc. + * Cleanup temp files, close resources, etc. * * @throws Exception thrown if there is a problem */ @@ -67,6 +70,7 @@ public class NodePackageAnalyzerTest extends BaseTest { @Override public void tearDown() throws Exception { analyzer.close(); + engine.close(); super.tearDown(); } diff --git a/dependency-check-core/src/test/resources/dependencycheck.properties b/dependency-check-core/src/test/resources/dependencycheck.properties index 94a2cdd25..133ee11c0 100644 --- a/dependency-check-core/src/test/resources/dependencycheck.properties +++ b/dependency-check-core/src/test/resources/dependencycheck.properties @@ -123,3 +123,5 @@ analyzer.nvdcve.enabled=true analyzer.vulnerabilitysuppression.enabled=true updater.nvdcve.enabled=true updater.versioncheck.enabled=true + +ecosystem.skip.nvdcve=npm \ No newline at end of file diff --git a/dependency-check-utils/src/main/java/org/owasp/dependencycheck/utils/Settings.java b/dependency-check-utils/src/main/java/org/owasp/dependencycheck/utils/Settings.java index dff450027..730c2de5d 100644 --- a/dependency-check-utils/src/main/java/org/owasp/dependencycheck/utils/Settings.java +++ b/dependency-check-utils/src/main/java/org/owasp/dependencycheck/utils/Settings.java @@ -442,6 +442,10 @@ public final class Settings { * new version available. */ public static final String UPDATE_VERSION_CHECK_ENABLED = "updater.versioncheck.enabled"; + /** + * The key to determine which ecosystems should skip the NVD CVE analysis. + */ + public static final String ECOSYSTEM_SKIP_NVDCVE = "ecosystem.skip.nvdcve"; /** * private constructor because this is a "utility" class containing