From 2a1186c4faa84c4a0acb658666f1066697fe777a Mon Sep 17 00:00:00 2001 From: Jeremy Long Date: Sat, 18 Nov 2017 13:24:23 -0500 Subject: [PATCH 01/22] re-enable node analyzer for #993 --- .../analyzer/DependencyBundlingAnalyzer.java | 21 ++ .../analyzer/DependencyMergingAnalyzer.java | 7 +- .../analyzer/NodePackageAnalyzer.java | 54 +++-- .../dependencycheck/analyzer/NspAnalyzer.java | 216 ++++++++++-------- .../dependencycheck/data/nsp/NspSearch.java | 1 + 5 files changed, 179 insertions(+), 120 deletions(-) diff --git a/dependency-check-core/src/main/java/org/owasp/dependencycheck/analyzer/DependencyBundlingAnalyzer.java b/dependency-check-core/src/main/java/org/owasp/dependencycheck/analyzer/DependencyBundlingAnalyzer.java index 30125e419..02cf7d7d3 100644 --- a/dependency-check-core/src/main/java/org/owasp/dependencycheck/analyzer/DependencyBundlingAnalyzer.java +++ b/dependency-check-core/src/main/java/org/owasp/dependencycheck/analyzer/DependencyBundlingAnalyzer.java @@ -135,6 +135,15 @@ public class DependencyBundlingAnalyzer extends AbstractDependencyComparingAnaly mergeDependencies(nextDependency, dependency, dependenciesToRemove); return true; //since we merged into the next dependency - skip forward to the next in mainIterator } + } else if (ecoSystemIs(NspAnalyzer.DEPENDENCY_ECOSYSTEM, dependency, nextDependency) + && namesAreEqual(dependency, nextDependency) + && versionsAreEqual(dependency, nextDependency)) { + if (dependency.isVirtual()) { + DependencyMergingAnalyzer.mergeDependencies(dependency, nextDependency, dependenciesToRemove); + } else { + DependencyMergingAnalyzer.mergeDependencies(nextDependency, dependency, dependenciesToRemove); + return true; + } } return false; } @@ -452,4 +461,16 @@ public class DependencyBundlingAnalyzer extends AbstractDependencyComparingAnaly return filePath != null && filePath.matches(".*\\.(ear|war)[\\\\/].*"); } + private boolean ecoSystemIs(String ecoSystem, Dependency dependency, Dependency nextDependency) { + return ecoSystem.equals(dependency.getEcosystem()) && ecoSystem.equals(nextDependency.getEcosystem()); + } + + private boolean namesAreEqual(Dependency dependency, Dependency nextDependency) { + return dependency.getName() != null && dependency.getName().equals(nextDependency.getName()); + } + + private boolean versionsAreEqual(Dependency dependency, Dependency nextDependency) { + return dependency.getVersion() != null && dependency.getVersion().equals(nextDependency.getVersion()); + } + } diff --git a/dependency-check-core/src/main/java/org/owasp/dependencycheck/analyzer/DependencyMergingAnalyzer.java b/dependency-check-core/src/main/java/org/owasp/dependencycheck/analyzer/DependencyMergingAnalyzer.java index 49db7fd7c..5a21f2cad 100644 --- a/dependency-check-core/src/main/java/org/owasp/dependencycheck/analyzer/DependencyMergingAnalyzer.java +++ b/dependency-check-core/src/main/java/org/owasp/dependencycheck/analyzer/DependencyMergingAnalyzer.java @@ -118,7 +118,7 @@ public class DependencyMergingAnalyzer extends AbstractDependencyComparingAnalyz * removed from the main analysis loop, this function adds to this * collection */ - private void mergeDependencies(final Dependency dependency, final Dependency relatedDependency, final Set dependenciesToRemove) { + public static void mergeDependencies(final Dependency dependency, final Dependency relatedDependency, final Set dependenciesToRemove) { LOGGER.debug("Merging '{}' into '{}'", relatedDependency.getFilePath(), dependency.getFilePath()); dependency.addRelatedDependency(relatedDependency); for (Evidence e : relatedDependency.getEvidence(EvidenceType.VENDOR)) { @@ -135,9 +135,8 @@ public class DependencyMergingAnalyzer extends AbstractDependencyComparingAnalyz dependency.addRelatedDependency(d); relatedDependency.removeRelatedDependencies(d); } - if (dependency.getSha1sum().equals(relatedDependency.getSha1sum())) { - dependency.addAllProjectReferences(relatedDependency.getProjectReferences()); - } + dependency.addAllProjectReferences(relatedDependency.getProjectReferences()); + dependenciesToRemove.add(relatedDependency); } 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 f9b83008d..1f9c0d734 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 @@ -48,7 +48,6 @@ import org.owasp.dependencycheck.dependency.EvidenceType; * @author Dale Visser */ @ThreadSafe -@Retired public class NodePackageAnalyzer extends AbstractFileTypeAnalyzer { /** @@ -133,22 +132,9 @@ public class NodePackageAnalyzer extends AbstractFileTypeAnalyzer { } try (JsonReader jsonReader = Json.createReader(FileUtils.openInputStream(file))) { final JsonObject json = jsonReader.readObject(); - if (json.containsKey("name")) { - final Object value = json.get("name"); - if (value instanceof JsonString) { - final String valueString = ((JsonString) value).getString(); - dependency.setName(valueString); - dependency.addEvidence(EvidenceType.PRODUCT, PACKAGE_JSON, "name", valueString, Confidence.HIGHEST); - dependency.addEvidence(EvidenceType.VENDOR, PACKAGE_JSON, "name_project", - String.format("%s_project", valueString), Confidence.LOW); - } else { - LOGGER.warn("JSON value not string as expected: {}", value); - } - } - addToEvidence(dependency, EvidenceType.PRODUCT, json, "description"); - addToEvidence(dependency, EvidenceType.VENDOR, json, "author"); - final String version = addToEvidence(dependency, EvidenceType.VERSION, json, "version"); - dependency.setVersion(version); + + gatherEvidence(json, dependency); + } catch (JsonException e) { LOGGER.warn("Failed to parse package.json file.", e); } catch (IOException e) { @@ -156,6 +142,38 @@ public class NodePackageAnalyzer extends AbstractFileTypeAnalyzer { } } + public static void gatherEvidence(final JsonObject json, Dependency dependency) { + if (json.containsKey("name")) { + final Object value = json.get("name"); + if (value instanceof JsonString) { + final String valueString = ((JsonString) value).getString(); + dependency.setName(valueString); + dependency.setPackagePath(valueString); + dependency.addEvidence(EvidenceType.PRODUCT, PACKAGE_JSON, "name", valueString, Confidence.HIGHEST); + dependency.addEvidence(EvidenceType.VENDOR, PACKAGE_JSON, "name", valueString, Confidence.HIGH); + } else { + LOGGER.warn("JSON value not string as expected: {}", value); + } + } + addToEvidence(dependency, EvidenceType.PRODUCT, json, "description"); + addToEvidence(dependency, EvidenceType.VENDOR, json, "author"); + final String version = addToEvidence(dependency, EvidenceType.VERSION, json, "version"); + if (version != null) { + dependency.setVersion(version); + dependency.addIdentifier("npm", String.format("%s:%s", dependency.getName(), version), null, Confidence.HIGHEST); + } + + // Adds the license if defined in package.json + if (json.containsKey("license")) { + final Object value = json.get("license"); + if (value instanceof JsonString) { + dependency.setLicense(json.getString("license")); + } else { + dependency.setLicense(json.getJsonObject("license").getString("type")); + } + } + } + /** * Adds information to an evidence collection from the node json * configuration. @@ -166,7 +184,7 @@ public class NodePackageAnalyzer extends AbstractFileTypeAnalyzer { * @return the actual string set into evidence * @param key the key to obtain the data from the json information */ - private String addToEvidence(Dependency dep, EvidenceType t, JsonObject json, String key) { + private static String addToEvidence(Dependency dep, EvidenceType t, JsonObject json, String key) { String evidenceStr = null; if (json.containsKey(key)) { final JsonValue value = json.get(key); 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 da33c61a6..f36f23f4e 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 @@ -36,6 +36,7 @@ import java.io.File; import java.io.FileFilter; import java.io.IOException; import java.net.MalformedURLException; +import java.util.ArrayList; import java.util.Arrays; import java.util.HashSet; import java.util.List; @@ -71,7 +72,11 @@ public class NspAnalyzer extends AbstractFileTypeAnalyzer { * The default URL to the NSP check API. */ public static final String DEFAULT_URL = "https://api.nodesecurity.io/check"; - + /** + * A descriptor for the type of dependencies processed or added by this + * analyzer. + */ + public static final String DEPENDENCY_ECOSYSTEM = "npm"; /** * The file name to scan. */ @@ -136,10 +141,10 @@ public class NspAnalyzer extends AbstractFileTypeAnalyzer { } /** - * Returns the key used in the properties file to reference the analyzer's - * enabled property.x + * Returns the key used in the properties file to determine if the analyzer + * is enabled. * - * @return the analyzer's enabled property setting key + * @return the enabled property setting key for the analyzer */ @Override protected String getAnalyzerEnabledSettingKey() { @@ -152,16 +157,48 @@ public class NspAnalyzer extends AbstractFileTypeAnalyzer { if (!file.isFile() || file.length() == 0) { return; } + try (JsonReader jsonReader = Json.createReader(FileUtils.openInputStream(file))) { + // Retrieves the contents of package.json from the Dependency + final JsonObject packageJson = jsonReader.readObject(); + + if (dependency.getEcosystem() == null || dependency.getName() == null) { + NodePackageAnalyzer.gatherEvidence(packageJson, dependency); + dependency.setEcosystem(DEPENDENCY_ECOSYSTEM); + } + // Do not scan the node_modules directory if (file.getCanonicalPath().contains(File.separator + "node_modules" + File.separator)) { LOGGER.debug("Skipping analysis of node module: " + file.getCanonicalPath()); return; } - // Retrieves the contents of package.json from the Dependency - final JsonObject packageJson = jsonReader.readObject(); + //Processes the dependencies objects in package.json and adds all the modules as dependencies + if (packageJson.containsKey("dependencies")) { + final JsonObject dependencies = packageJson.getJsonObject("dependencies"); + processPackage(engine, dependency, dependencies, "dependencies"); + } + if (packageJson.containsKey("devDependencies")) { + final JsonObject dependencies = packageJson.getJsonObject("devDependencies"); + processPackage(engine, dependency, dependencies, "devDependencies"); + } + if (packageJson.containsKey("optionalDependencies")) { + final JsonObject dependencies = packageJson.getJsonObject("optionalDependencies"); + processPackage(engine, dependency, dependencies, "optionalDependencies"); + } + if (packageJson.containsKey("peerDependencies")) { + final JsonObject dependencies = packageJson.getJsonObject("peerDependencies"); + processPackage(engine, dependency, dependencies, "peerDependencies"); + } + if (packageJson.containsKey("bundleDependencies")) { + final JsonArray dependencies = packageJson.getJsonArray("bundleDependencies"); + processPackage(engine, dependency, dependencies, "bundleDependencies"); + } + if (packageJson.containsKey("bundledDependencies")) { + final JsonArray dependencies = packageJson.getJsonArray("bundledDependencies"); + processPackage(engine, dependency, dependencies, "bundledDependencies"); + } // Create a sanitized version of the package.json final JsonObject sanitizedJson = SanitizePackage.sanitize(packageJson); @@ -192,77 +229,21 @@ public class NspAnalyzer extends AbstractFileTypeAnalyzer { * Create a single vulnerable software object - these do not use CPEs unlike the NVD. */ final VulnerableSoftware vs = new VulnerableSoftware(); - //vs.setVersion(advisory.getVulnerableVersions()); - vs.setUpdate(advisory.getPatchedVersions()); + //TODO consider changing this to available versions on the dependency + //vs.setUpdate(advisory.getPatchedVersions()); + vs.setName(advisory.getModule() + ":" + advisory.getVulnerableVersions()); vuln.setVulnerableSoftware(new HashSet<>(Arrays.asList(vs))); - // Add the vulnerability to package.json - dependency.addVulnerability(vuln); - } - - /* - * Adds evidence about the node package itself, not any of the modules. - */ - if (packageJson.containsKey("name")) { - final Object value = packageJson.get("name"); - if (value instanceof JsonString) { - final String valueString = ((JsonString) value).getString(); - dependency.addEvidence(EvidenceType.PRODUCT, PACKAGE_JSON, "name", valueString, Confidence.HIGHEST); - dependency.addEvidence(EvidenceType.VENDOR, PACKAGE_JSON, "name_project", - String.format("%s_project", valueString), Confidence.LOW); + Dependency existing = findDependency(engine, advisory.getModule(), advisory.getVersion()); + if (existing == null) { + Dependency nodeModule = createDependency(dependency, advisory.getModule(), advisory.getVersion(), "transitive"); + nodeModule.addVulnerability(vuln); + engine.addDependency(nodeModule); } else { - LOGGER.warn("JSON value not string as expected: {}", value); + existing.addVulnerability(vuln); } } - - /* - * Processes the dependencies objects in package.json and adds all the modules as related dependencies - */ - if (packageJson.containsKey("dependencies")) { - final JsonObject dependencies = packageJson.getJsonObject("dependencies"); - processPackage(dependency, dependencies, "dependencies"); - } - if (packageJson.containsKey("devDependencies")) { - final JsonObject dependencies = packageJson.getJsonObject("devDependencies"); - processPackage(dependency, dependencies, "devDependencies"); - } - if (packageJson.containsKey("optionalDependencies")) { - final JsonObject dependencies = packageJson.getJsonObject("optionalDependencies"); - processPackage(dependency, dependencies, "optionalDependencies"); - } - if (packageJson.containsKey("peerDependencies")) { - final JsonObject dependencies = packageJson.getJsonObject("peerDependencies"); - processPackage(dependency, dependencies, "peerDependencies"); - } - if (packageJson.containsKey("bundleDependencies")) { - final JsonArray dependencies = packageJson.getJsonArray("bundleDependencies"); - processPackage(dependency, dependencies, "bundleDependencies"); - } - if (packageJson.containsKey("bundledDependencies")) { - final JsonArray dependencies = packageJson.getJsonArray("bundledDependencies"); - processPackage(dependency, dependencies, "bundledDependencies"); - } - - /* - * Adds the license if defined in package.json - */ - if (packageJson.containsKey("license")) { - final Object value = packageJson.get("license"); - if (value instanceof JsonString) { - dependency.setLicense(packageJson.getString("license")); - } else { - dependency.setLicense(packageJson.getJsonObject("license").getString("type")); - } - } - - /* - * Adds general evidence to about the package. - */ - addToEvidence(dependency, EvidenceType.PRODUCT, packageJson, "description"); - addToEvidence(dependency, EvidenceType.VENDOR, packageJson, "author"); - addToEvidence(dependency, EvidenceType.VERSION, packageJson, "version"); - dependency.setDisplayFileName(String.format("%s/%s", file.getParentFile().getName(), file.getName())); } catch (URLConnectionFailureException e) { this.setEnabled(false); throw new AnalysisException(e.getMessage(), e); @@ -275,62 +256,62 @@ public class NspAnalyzer extends AbstractFileTypeAnalyzer { } } + private Dependency createDependency(Dependency dependency, String name, String version, String scope) { + final Dependency nodeModule = new Dependency(new File(dependency.getActualFile() + "?" + name), true); + nodeModule.setEcosystem(DEPENDENCY_ECOSYSTEM); + nodeModule.addEvidence(EvidenceType.PRODUCT, "package.json", "name", name, Confidence.HIGHEST); + nodeModule.addEvidence(EvidenceType.VENDOR, "package.json", "name", name, Confidence.HIGH); + nodeModule.addEvidence(EvidenceType.VERSION, "package.json", "version", version, Confidence.HIGHEST); + nodeModule.addProjectReference(dependency.getName() + ": " + scope); + nodeModule.setName(name); + nodeModule.setVersion(version); + nodeModule.addIdentifier("npm", String.format("%s:%s", name, version), null, Confidence.HIGHEST); + return nodeModule; + } + /** * Processes a part of package.json (as defined by JsonArray) and update the * specified dependency with relevant info. * + * @param engine the dependency-check engine * @param dependency the Dependency to update * @param jsonArray the jsonArray to parse * @param depType the dependency type */ - private void processPackage(Dependency dependency, JsonArray jsonArray, String depType) { + private void processPackage(Engine engine, Dependency dependency, JsonArray jsonArray, String depType) { final JsonObjectBuilder builder = Json.createObjectBuilder(); for (JsonString str : jsonArray.getValuesAs(JsonString.class)) { builder.add(str.toString(), ""); } final JsonObject jsonObject = builder.build(); - processPackage(dependency, jsonObject, depType); + processPackage(engine, dependency, jsonObject, depType); } /** * Processes a part of package.json (as defined by JsonObject) and update * the specified dependency with relevant info. * + * @param engine the dependency-check engine * @param dependency the Dependency to update * @param jsonObject the jsonObject to parse * @param depType the dependency type */ - private void processPackage(Dependency dependency, JsonObject jsonObject, String depType) { + private void processPackage(Engine engine, Dependency dependency, JsonObject jsonObject, String depType) { for (int i = 0; i < jsonObject.size(); i++) { for (Map.Entry entry : jsonObject.entrySet()) { - /* - * Create identifies that include the npm module and version. Since these are defined, - * assign the highest confidence. - */ - final Identifier moduleName = new Identifier("npm", "Module", null, entry.getKey()); - moduleName.setConfidence(Confidence.HIGHEST); + + final String name = entry.getKey(); String version = ""; if (entry.getValue() != null && entry.getValue().getValueType() == JsonValue.ValueType.STRING) { version = ((JsonString) entry.getValue()).getString(); } - final Identifier moduleVersion = new Identifier("npm", "Version", null, version); - moduleVersion.setConfidence(Confidence.HIGHEST); - - final Identifier moduleDepType = new Identifier("npm", "Scope", null, depType); - moduleVersion.setConfidence(Confidence.HIGHEST); - - /* - * Create related dependencies for each module defined in package.json. The path to the related - * dependency will not actually exist but needs to be unique (due to the use of Set in Dependency). - * The use of related dependencies is a way to specify the actual software BOM in package.json. - */ - //TODO is this actually correct? or should these be transitive dependencies? - final Dependency nodeModule = new Dependency(new File(dependency.getActualFile() + "#" + entry.getKey()), true); - nodeModule.setDisplayFileName(entry.getKey()); - nodeModule.addIdentifier(moduleName); - nodeModule.addIdentifier(moduleVersion); - nodeModule.addIdentifier(moduleDepType); - dependency.addRelatedDependency(nodeModule); + Dependency existing = findDependency(engine, name, version); + if (existing == null) { + final Dependency nodeModule = createDependency(dependency, name, version, depType); + engine.addDependency(nodeModule); + } else { + existing.addProjectReference(dependency.getName() + ": " + depType); + } } } } @@ -368,4 +349,43 @@ public class NspAnalyzer extends AbstractFileTypeAnalyzer { } } } + + private Dependency findDependency(Engine engine, String name, String version) { + for (Dependency d : engine.getDependencies()) { + if (DEPENDENCY_ECOSYSTEM.equals(d.getEcosystem()) && name.equals(d.getName()) && version != null && d.getVersion() != null) { + String dependencyVersion = d.getVersion(); + if (dependencyVersion.startsWith("^") || dependencyVersion.startsWith("~")) { + dependencyVersion = dependencyVersion.substring(1); + } + + if (version.equals(dependencyVersion)) { + return d; + } + if (version.startsWith("^") || version.startsWith("~") || version.contains("*")) { + String type; + String tmp; + if (version.startsWith("^") || version.startsWith("~")) { + type = version.substring(0, 1); + tmp = version.substring(1); + } else { + type = "*"; + tmp = version; + } + String[] v = tmp.split(" ")[0].split("\\."); + String[] depVersion = dependencyVersion.split("\\."); + + if ("^".equals(type) && v[0].equals(depVersion[0])) { + return d; + } else if ("~".equals(type) && v.length >= 2 && depVersion.length >= 2 && v[0].equals(depVersion[0]) && v[1].equals(depVersion[1])) { + return d; + } else if (v[0].equals("*") + || (v.length >= 2 && v[0].equals(depVersion[0]) && v[1].equals("*")) + || (v.length >= 3 && depVersion.length >= 2 && v[0].equals(depVersion[0]) && v[1].equals(depVersion[1]) && v[2].equals("*"))) { + return d; + } + } + } + } + return null; + } } diff --git a/dependency-check-core/src/main/java/org/owasp/dependencycheck/data/nsp/NspSearch.java b/dependency-check-core/src/main/java/org/owasp/dependencycheck/data/nsp/NspSearch.java index 327952a25..52bdd15b8 100644 --- a/dependency-check-core/src/main/java/org/owasp/dependencycheck/data/nsp/NspSearch.java +++ b/dependency-check-core/src/main/java/org/owasp/dependencycheck/data/nsp/NspSearch.java @@ -123,6 +123,7 @@ public class NspSearch { try (InputStream in = new BufferedInputStream(conn.getInputStream()); JsonReader jsonReader = Json.createReader(in)) { final JsonArray array = jsonReader.readArray(); + if (array != null) { for (int i = 0; i < array.size(); i++) { final JsonObject object = array.getJsonObject(i); From 3440edbfb654d86e2d9f96663790a9c478a6e8e9 Mon Sep 17 00:00:00 2001 From: Jeremy Long Date: Sat, 18 Nov 2017 13:30:14 -0500 Subject: [PATCH 02/22] fix generated hyperlinks --- .../java/org/owasp/dependencycheck/analyzer/NspAnalyzer.java | 3 +++ 1 file changed, 3 insertions(+) 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 f36f23f4e..2a5956d74 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 @@ -52,6 +52,7 @@ import javax.json.JsonString; import javax.json.JsonValue; import org.owasp.dependencycheck.dependency.EvidenceType; import org.owasp.dependencycheck.exception.InitializationException; +import org.owasp.dependencycheck.utils.Checksum; import org.owasp.dependencycheck.utils.URLConnectionFailureException; /** @@ -259,6 +260,8 @@ public class NspAnalyzer extends AbstractFileTypeAnalyzer { private Dependency createDependency(Dependency dependency, String name, String version, String scope) { final Dependency nodeModule = new Dependency(new File(dependency.getActualFile() + "?" + name), true); nodeModule.setEcosystem(DEPENDENCY_ECOSYSTEM); + //this is virtual - the sha1 is purely for the hyperlink in the final html report + nodeModule.setSha1sum(Checksum.getSHA1Checksum(String.format("%s:%s", name, version))); nodeModule.addEvidence(EvidenceType.PRODUCT, "package.json", "name", name, Confidence.HIGHEST); nodeModule.addEvidence(EvidenceType.VENDOR, "package.json", "name", name, Confidence.HIGH); nodeModule.addEvidence(EvidenceType.VERSION, "package.json", "version", version, Confidence.HIGHEST); From 7e1b6d0cc7c74e7e8c929a087edd47a2981430a9 Mon Sep 17 00:00:00 2001 From: Jeremy Long Date: Sat, 18 Nov 2017 15:02:59 -0500 Subject: [PATCH 03/22] fixed test cases --- .../analyzer/AnalyzerServiceTest.java | 14 +- .../analyzer/NodePackageAnalyzerTest.java | 2 +- .../analyzer/NspAnalyzerTest.java | 127 +++++++++++------- 3 files changed, 78 insertions(+), 65 deletions(-) diff --git a/dependency-check-core/src/test/java/org/owasp/dependencycheck/analyzer/AnalyzerServiceTest.java b/dependency-check-core/src/test/java/org/owasp/dependencycheck/analyzer/AnalyzerServiceTest.java index 00063719a..efd825357 100644 --- a/dependency-check-core/src/test/java/org/owasp/dependencycheck/analyzer/AnalyzerServiceTest.java +++ b/dependency-check-core/src/test/java/org/owasp/dependencycheck/analyzer/AnalyzerServiceTest.java @@ -76,16 +76,12 @@ public class AnalyzerServiceTest extends BaseDBTestCase { AnalyzerService instance = new AnalyzerService(Thread.currentThread().getContextClassLoader(), getSettings()); List result = instance.getAnalyzers(); String experimental = "CMake Analyzer"; - String retired = "Node.js Package Analyzer"; boolean found = false; boolean retiredFound = false; for (Analyzer a : result) { if (experimental.equals(a.getName())) { found = true; } - if (retired.equals(a.getName())) { - retiredFound = true; - } } assertFalse("Experimental analyzer loaded when set to false", found); assertFalse("Retired analyzer loaded when set to false", retiredFound); @@ -99,13 +95,10 @@ public class AnalyzerServiceTest extends BaseDBTestCase { if (experimental.equals(a.getName())) { found = true; } - if (retired.equals(a.getName())) { - retiredFound = true; - } } assertTrue("Experimental analyzer not loaded when set to true", found); assertFalse("Retired analyzer loaded when set to false", retiredFound); - + getSettings().setBoolean(Settings.KEYS.ANALYZER_EXPERIMENTAL_ENABLED, false); getSettings().setBoolean(Settings.KEYS.ANALYZER_RETIRED_ENABLED, true); instance = new AnalyzerService(Thread.currentThread().getContextClassLoader(), getSettings()); @@ -116,11 +109,8 @@ public class AnalyzerServiceTest extends BaseDBTestCase { if (experimental.equals(a.getName())) { found = true; } - if (retired.equals(a.getName())) { - retiredFound = true; - } } assertFalse("Experimental analyzer loaded when set to false", found); - assertTrue("Retired analyzer not loaded when set to true", retiredFound); + //assertTrue("Retired analyzer not loaded when set to true", retiredFound); } } 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 b243ea3cd..bb21a13d8 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 @@ -98,7 +98,7 @@ public class NodePackageAnalyzerTest extends BaseTest { analyzer.analyze(result, null); final String vendorString = result.getEvidence(EvidenceType.VENDOR).toString(); assertThat(vendorString, containsString("Sanjeev Koranga")); - assertThat(vendorString, containsString("dns-sync_project")); + assertThat(vendorString, containsString("dns-sync")); assertThat(result.getEvidence(EvidenceType.PRODUCT).toString(), containsString("dns-sync")); assertThat(result.getEvidence(EvidenceType.VERSION).toString(), containsString("0.1.0")); assertEquals(NodePackageAnalyzer.DEPENDENCY_ECOSYSTEM, result.getEcosystem()); diff --git a/dependency-check-core/src/test/java/org/owasp/dependencycheck/analyzer/NspAnalyzerTest.java b/dependency-check-core/src/test/java/org/owasp/dependencycheck/analyzer/NspAnalyzerTest.java index dfcd98d3f..508b58d0d 100644 --- a/dependency-check-core/src/test/java/org/owasp/dependencycheck/analyzer/NspAnalyzerTest.java +++ b/dependency-check-core/src/test/java/org/owasp/dependencycheck/analyzer/NspAnalyzerTest.java @@ -11,91 +11,114 @@ import java.io.File; import static org.hamcrest.CoreMatchers.is; import static org.junit.Assert.*; +import org.owasp.dependencycheck.Engine; import org.owasp.dependencycheck.dependency.EvidenceType; +import org.owasp.dependencycheck.exception.InitializationException; public class NspAnalyzerTest extends BaseTest { - private NspAnalyzer analyzer; - - @Before - @Override - public void setUp() throws Exception { - super.setUp(); - analyzer = new NspAnalyzer(); - analyzer.setFilesMatched(true); - analyzer.initialize(getSettings()); - analyzer.prepare(null); - } - - @After - @Override - public void tearDown() throws Exception { - analyzer.close(); - super.tearDown(); - } - @Test public void testGetName() { + NspAnalyzer analyzer = new NspAnalyzer(); assertThat(analyzer.getName(), is("Node Security Platform Analyzer")); } @Test public void testSupportsFiles() { + NspAnalyzer analyzer = new NspAnalyzer(); assertThat(analyzer.accept(new File("package.json")), is(true)); } @Test - public void testAnalyzePackage() throws AnalysisException { - final Dependency result = new Dependency(BaseTest.getResourceAsFile(this, "nsp/package.json")); - analyzer.analyze(result, null); + public void testAnalyzePackage() throws AnalysisException, InitializationException { + try (Engine engine = new Engine(getSettings())) { + NspAnalyzer analyzer = new NspAnalyzer(); + analyzer.setFilesMatched(true); + analyzer.initialize(getSettings()); + analyzer.prepare(engine); + final Dependency result = new Dependency(BaseTest.getResourceAsFile(this, "nsp/package.json")); + analyzer.analyze(result, engine); - assertTrue(result.getEvidence(EvidenceType.VENDOR).toString().contains("owasp-nodejs-goat_project")); - assertTrue(result.getEvidence(EvidenceType.PRODUCT).toString().contains("A tool to learn OWASP Top 10 for node.js developers")); - assertTrue(result.getEvidence(EvidenceType.VERSION).toString().contains("1.3.0")); + assertTrue(result.getEvidence(EvidenceType.VENDOR).toString().contains("owasp-nodejs-goat")); + assertTrue(result.getEvidence(EvidenceType.PRODUCT).toString().contains("A tool to learn OWASP Top 10 for node.js developers")); + assertTrue(result.getEvidence(EvidenceType.VERSION).toString().contains("1.3.0")); + } } @Test - public void testAnalyzeEmpty() throws AnalysisException { - final Dependency result = new Dependency(BaseTest.getResourceAsFile(this, "nsp/empty.json")); - analyzer.analyze(result, null); + public void testAnalyzeEmpty() throws AnalysisException, InitializationException { + try (Engine engine = new Engine(getSettings())) { + NspAnalyzer analyzer = new NspAnalyzer(); + analyzer.setFilesMatched(true); + analyzer.initialize(getSettings()); + analyzer.prepare(engine); + final Dependency result = new Dependency(BaseTest.getResourceAsFile(this, "nsp/empty.json")); + analyzer.analyze(result, engine); - assertEquals(result.getEvidence(EvidenceType.VENDOR).size(), 0); - assertEquals(result.getEvidence(EvidenceType.PRODUCT).size(), 0); - assertEquals(result.getEvidence(EvidenceType.VERSION).size(), 0); + assertEquals(result.getEvidence(EvidenceType.VENDOR).size(), 0); + assertEquals(result.getEvidence(EvidenceType.PRODUCT).size(), 0); + assertEquals(result.getEvidence(EvidenceType.VERSION).size(), 0); + } } @Test - public void testAnalyzePackageJsonWithBundledDeps() throws AnalysisException { - final Dependency result = new Dependency(BaseTest.getResourceAsFile(this, "nsp/bundled.deps.package.json")); - analyzer.analyze(result, null); + public void testAnalyzePackageJsonWithBundledDeps() throws AnalysisException, InitializationException { + try (Engine engine = new Engine(getSettings())) { + NspAnalyzer analyzer = new NspAnalyzer(); + analyzer.setFilesMatched(true); + analyzer.initialize(getSettings()); + analyzer.prepare(engine); + final Dependency result = new Dependency(BaseTest.getResourceAsFile(this, "nsp/bundled.deps.package.json")); + analyzer.analyze(result, engine); - assertTrue(result.getEvidence(EvidenceType.VENDOR).toString().contains("Philipp Dunkel ")); - assertTrue(result.getEvidence(EvidenceType.PRODUCT).toString().contains("Native Access to Mac OS-X FSEvents")); - assertTrue(result.getEvidence(EvidenceType.VERSION).toString().contains("1.1.1")); + assertTrue(result.getEvidence(EvidenceType.VENDOR).toString().contains("Philipp Dunkel ")); + assertTrue(result.getEvidence(EvidenceType.PRODUCT).toString().contains("Native Access to Mac OS-X FSEvents")); + assertTrue(result.getEvidence(EvidenceType.VERSION).toString().contains("1.1.1")); + } } @Test - public void testAnalyzePackageJsonWithLicenseObject() throws AnalysisException { - final Dependency result = new Dependency(BaseTest.getResourceAsFile(this, "nsp/license.obj.package.json")); - analyzer.analyze(result, null); + public void testAnalyzePackageJsonWithLicenseObject() throws AnalysisException, InitializationException { + try (Engine engine = new Engine(getSettings())) { + NspAnalyzer analyzer = new NspAnalyzer(); + analyzer.setFilesMatched(true); + analyzer.initialize(getSettings()); + analyzer.prepare(engine); + final Dependency result = new Dependency(BaseTest.getResourceAsFile(this, "nsp/license.obj.package.json")); + analyzer.analyze(result, engine); - assertTrue(result.getEvidence(EvidenceType.VENDOR).toString().contains("Twitter, Inc.")); - assertTrue(result.getEvidence(EvidenceType.PRODUCT).toString().contains("The most popular front-end framework for developing responsive, mobile first projects on the web")); - assertTrue(result.getEvidence(EvidenceType.VERSION).toString().contains("3.2.0")); + assertTrue(result.getEvidence(EvidenceType.VENDOR).toString().contains("Twitter, Inc.")); + assertTrue(result.getEvidence(EvidenceType.PRODUCT).toString().contains("The most popular front-end framework for developing responsive, mobile first projects on the web")); + assertTrue(result.getEvidence(EvidenceType.VERSION).toString().contains("3.2.0")); + } } @Test - public void testAnalyzePackageJsonInNodeModulesDirectory() throws AnalysisException { - final Dependency result = new Dependency(BaseTest.getResourceAsFile(this, "nodejs/node_modules/dns-sync/package.json")); - analyzer.analyze(result, null); - // node modules are not scanned - no evidence is collected - assertTrue(result.size() == 0); + public void testAnalyzePackageJsonInNodeModulesDirectory() throws AnalysisException, InitializationException { + try (Engine engine = new Engine(getSettings())) { + NspAnalyzer analyzer = new NspAnalyzer(); + analyzer.setFilesMatched(true); + analyzer.initialize(getSettings()); + analyzer.prepare(engine); + final Dependency result = new Dependency(BaseTest.getResourceAsFile(this, "nodejs/node_modules/dns-sync/package.json")); + analyzer.analyze(result, engine); + // package.json adds 5 bits of evidence + assertTrue(result.size() == 5); + // but no vulnerabilities were cited + assertTrue(result.getVulnerabilities().isEmpty()); + } } @Test - public void testAnalyzeInvalidPackageMissingName() throws AnalysisException { - final Dependency result = new Dependency(BaseTest.getResourceAsFile(this, "nsp/minimal-invalid.json")); - analyzer.analyze(result, null); - // Upon analysis, not throwing an exception in this case, is all that's required to pass this test + public void testAnalyzeInvalidPackageMissingName() throws AnalysisException, InitializationException { + try (Engine engine = new Engine(getSettings())) { + NspAnalyzer analyzer = new NspAnalyzer(); + analyzer.setFilesMatched(true); + analyzer.initialize(getSettings()); + analyzer.prepare(engine); + final Dependency result = new Dependency(BaseTest.getResourceAsFile(this, "nsp/minimal-invalid.json")); + analyzer.analyze(result, engine); + // Upon analysis, not throwing an exception in this case, is all that's required to pass this test + } } } From 804f8e38da42b94978b739679cf3950e5d1effcd Mon Sep 17 00:00:00 2001 From: Jeremy Long Date: Sat, 18 Nov 2017 16:32:40 -0500 Subject: [PATCH 04/22] checkstyle suggested changes --- .../analyzer/DependencyBundlingAnalyzer.java | 23 ++++++++++++ .../analyzer/DependencyMergingAnalyzer.java | 3 +- .../analyzer/NodePackageAnalyzer.java | 5 +++ .../dependencycheck/analyzer/NspAnalyzer.java | 36 ++++++++++++++----- .../dependencycheck/data/nsp/NspSearch.java | 2 +- 5 files changed, 58 insertions(+), 11 deletions(-) diff --git a/dependency-check-core/src/main/java/org/owasp/dependencycheck/analyzer/DependencyBundlingAnalyzer.java b/dependency-check-core/src/main/java/org/owasp/dependencycheck/analyzer/DependencyBundlingAnalyzer.java index 02cf7d7d3..44aae96da 100644 --- a/dependency-check-core/src/main/java/org/owasp/dependencycheck/analyzer/DependencyBundlingAnalyzer.java +++ b/dependency-check-core/src/main/java/org/owasp/dependencycheck/analyzer/DependencyBundlingAnalyzer.java @@ -461,14 +461,37 @@ public class DependencyBundlingAnalyzer extends AbstractDependencyComparingAnaly return filePath != null && filePath.matches(".*\\.(ear|war)[\\\\/].*"); } + /** + * Determine if the dependency ecosystem is equal in the given dependencies. + * + * @param dependency a dependency to compare + * @param nextDependency a dependency to compare + * @return true if the ecosystem is equal in both dependencies; otherwise + * false + */ private boolean ecoSystemIs(String ecoSystem, Dependency dependency, Dependency nextDependency) { return ecoSystem.equals(dependency.getEcosystem()) && ecoSystem.equals(nextDependency.getEcosystem()); } + /** + * Determine if the dependency name is equal in the given dependencies. + * + * @param dependency a dependency to compare + * @param nextDependency a dependency to compare + * @return true if the name is equal in both dependencies; otherwise false + */ private boolean namesAreEqual(Dependency dependency, Dependency nextDependency) { return dependency.getName() != null && dependency.getName().equals(nextDependency.getName()); } + /** + * Determine if the dependency version is equal in the given dependencies. + * + * @param dependency a dependency to compare + * @param nextDependency a dependency to compare + * @return true if the version is equal in both dependencies; otherwise + * false + */ private boolean versionsAreEqual(Dependency dependency, Dependency nextDependency) { return dependency.getVersion() != null && dependency.getVersion().equals(nextDependency.getVersion()); } diff --git a/dependency-check-core/src/main/java/org/owasp/dependencycheck/analyzer/DependencyMergingAnalyzer.java b/dependency-check-core/src/main/java/org/owasp/dependencycheck/analyzer/DependencyMergingAnalyzer.java index 5a21f2cad..62ebd50c4 100644 --- a/dependency-check-core/src/main/java/org/owasp/dependencycheck/analyzer/DependencyMergingAnalyzer.java +++ b/dependency-check-core/src/main/java/org/owasp/dependencycheck/analyzer/DependencyMergingAnalyzer.java @@ -118,7 +118,8 @@ public class DependencyMergingAnalyzer extends AbstractDependencyComparingAnalyz * removed from the main analysis loop, this function adds to this * collection */ - public static void mergeDependencies(final Dependency dependency, final Dependency relatedDependency, final Set dependenciesToRemove) { + public static void mergeDependencies(final Dependency dependency, final Dependency relatedDependency, + final Set dependenciesToRemove) { LOGGER.debug("Merging '{}' into '{}'", relatedDependency.getFilePath(), dependency.getFilePath()); dependency.addRelatedDependency(relatedDependency); for (Evidence e : relatedDependency.getEvidence(EvidenceType.VENDOR)) { 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 1f9c0d734..ddbcc8e04 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 @@ -142,6 +142,11 @@ 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 + */ public static void gatherEvidence(final JsonObject json, Dependency dependency) { if (json.containsKey("name")) { final Object value = json.get("name"); 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 2a5956d74..4f7c3b53f 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 @@ -25,7 +25,6 @@ import org.owasp.dependencycheck.data.nsp.NspSearch; import org.owasp.dependencycheck.data.nsp.SanitizePackage; import org.owasp.dependencycheck.dependency.Confidence; import org.owasp.dependencycheck.dependency.Dependency; -import org.owasp.dependencycheck.dependency.Identifier; import org.owasp.dependencycheck.dependency.Vulnerability; import org.owasp.dependencycheck.dependency.VulnerableSoftware; import org.owasp.dependencycheck.utils.FileFilterBuilder; @@ -36,7 +35,6 @@ import java.io.File; import java.io.FileFilter; import java.io.IOException; import java.net.MalformedURLException; -import java.util.ArrayList; import java.util.Arrays; import java.util.HashSet; import java.util.List; @@ -236,9 +234,9 @@ public class NspAnalyzer extends AbstractFileTypeAnalyzer { vs.setName(advisory.getModule() + ":" + advisory.getVulnerableVersions()); vuln.setVulnerableSoftware(new HashSet<>(Arrays.asList(vs))); - Dependency existing = findDependency(engine, advisory.getModule(), advisory.getVersion()); + final Dependency existing = findDependency(engine, advisory.getModule(), advisory.getVersion()); if (existing == null) { - Dependency nodeModule = createDependency(dependency, advisory.getModule(), advisory.getVersion(), "transitive"); + final Dependency nodeModule = createDependency(dependency, advisory.getModule(), advisory.getVersion(), "transitive"); nodeModule.addVulnerability(vuln); engine.addDependency(nodeModule); } else { @@ -257,6 +255,15 @@ public class NspAnalyzer extends AbstractFileTypeAnalyzer { } } + /** + * Construct a dependency object. + * + * @param dependency the parent dependency + * @param name the name of the dependency to create + * @param version the version of the dependency to create + * @param scope the scope of the dependency being created + * @return the generated dependency + */ private Dependency createDependency(Dependency dependency, String name, String version, String scope) { final Dependency nodeModule = new Dependency(new File(dependency.getActualFile() + "?" + name), true); nodeModule.setEcosystem(DEPENDENCY_ECOSYSTEM); @@ -308,7 +315,7 @@ public class NspAnalyzer extends AbstractFileTypeAnalyzer { if (entry.getValue() != null && entry.getValue().getValueType() == JsonValue.ValueType.STRING) { version = ((JsonString) entry.getValue()).getString(); } - Dependency existing = findDependency(engine, name, version); + final Dependency existing = findDependency(engine, name, version); if (existing == null) { final Dependency nodeModule = createDependency(dependency, name, version, depType); engine.addDependency(nodeModule); @@ -353,6 +360,15 @@ public class NspAnalyzer extends AbstractFileTypeAnalyzer { } } + /** + * Locates the dependency from the list of dependencies that have been + * scanned by the engine. + * + * @param engine the dependency-check engine + * @param name the name of the dependency to find + * @param version the version of the dependency to find + * @return the identified dependency; otherwise null + */ private Dependency findDependency(Engine engine, String name, String version) { for (Dependency d : engine.getDependencies()) { if (DEPENDENCY_ECOSYSTEM.equals(d.getEcosystem()) && name.equals(d.getName()) && version != null && d.getVersion() != null) { @@ -374,16 +390,18 @@ public class NspAnalyzer extends AbstractFileTypeAnalyzer { type = "*"; tmp = version; } - String[] v = tmp.split(" ")[0].split("\\."); - String[] depVersion = dependencyVersion.split("\\."); + final String[] v = tmp.split(" ")[0].split("\\."); + final String[] depVersion = dependencyVersion.split("\\."); if ("^".equals(type) && v[0].equals(depVersion[0])) { return d; - } else if ("~".equals(type) && v.length >= 2 && depVersion.length >= 2 && v[0].equals(depVersion[0]) && v[1].equals(depVersion[1])) { + } else if ("~".equals(type) && v.length >= 2 && depVersion.length >= 2 + && v[0].equals(depVersion[0]) && v[1].equals(depVersion[1])) { return d; } else if (v[0].equals("*") || (v.length >= 2 && v[0].equals(depVersion[0]) && v[1].equals("*")) - || (v.length >= 3 && depVersion.length >= 2 && v[0].equals(depVersion[0]) && v[1].equals(depVersion[1]) && v[2].equals("*"))) { + || (v.length >= 3 && depVersion.length >= 2 && v[0].equals(depVersion[0]) + && v[1].equals(depVersion[1]) && v[2].equals("*"))) { return d; } } diff --git a/dependency-check-core/src/main/java/org/owasp/dependencycheck/data/nsp/NspSearch.java b/dependency-check-core/src/main/java/org/owasp/dependencycheck/data/nsp/NspSearch.java index 52bdd15b8..eb0815977 100644 --- a/dependency-check-core/src/main/java/org/owasp/dependencycheck/data/nsp/NspSearch.java +++ b/dependency-check-core/src/main/java/org/owasp/dependencycheck/data/nsp/NspSearch.java @@ -123,7 +123,7 @@ public class NspSearch { try (InputStream in = new BufferedInputStream(conn.getInputStream()); JsonReader jsonReader = Json.createReader(in)) { final JsonArray array = jsonReader.readArray(); - + if (array != null) { for (int i = 0; i < array.size(); i++) { final JsonObject object = array.getJsonObject(i); From 741fba51f503641bbd54a8e8db9c172cc7511214 Mon Sep 17 00:00:00 2001 From: Jeremy Long Date: Sun, 19 Nov 2017 08:29:19 -0500 Subject: [PATCH 05/22] updated as this would end up similiar to #933 --- .../java/org/owasp/dependencycheck/analyzer/NspAnalyzer.java | 1 + 1 file changed, 1 insertion(+) 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 4f7c3b53f..03f1ae1d5 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 @@ -269,6 +269,7 @@ public class NspAnalyzer extends AbstractFileTypeAnalyzer { nodeModule.setEcosystem(DEPENDENCY_ECOSYSTEM); //this is virtual - the sha1 is purely for the hyperlink in the final html report nodeModule.setSha1sum(Checksum.getSHA1Checksum(String.format("%s:%s", name, version))); + nodeModule.setMd5sum(Checksum.getMD5Checksum(String.format("%s:%s", name, version))); nodeModule.addEvidence(EvidenceType.PRODUCT, "package.json", "name", name, Confidence.HIGHEST); nodeModule.addEvidence(EvidenceType.VENDOR, "package.json", "name", name, Confidence.HIGH); nodeModule.addEvidence(EvidenceType.VERSION, "package.json", "version", version, Confidence.HIGHEST); From e4b7f7aa8fea6dbc5fa53484b5b8f3121aeecce2 Mon Sep 17 00:00:00 2001 From: Jeremy Long Date: Mon, 20 Nov 2017 06:46:25 -0500 Subject: [PATCH 06/22] 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 From 5ebc2dc24454cbe4d70d4921a7c9c129fee29053 Mon Sep 17 00:00:00 2001 From: Jeremy Long Date: Mon, 20 Nov 2017 06:55:36 -0500 Subject: [PATCH 07/22] checkstyle corrections --- .../analyzer/DependencyBundlingAnalyzer.java | 1 + .../dependencycheck/analyzer/NodePackageAnalyzer.java | 4 ++-- .../owasp/dependencycheck/analyzer/NvdCveAnalyzer.java | 9 ++++++--- 3 files changed, 9 insertions(+), 5 deletions(-) diff --git a/dependency-check-core/src/main/java/org/owasp/dependencycheck/analyzer/DependencyBundlingAnalyzer.java b/dependency-check-core/src/main/java/org/owasp/dependencycheck/analyzer/DependencyBundlingAnalyzer.java index 44aae96da..7d6a03b9e 100644 --- a/dependency-check-core/src/main/java/org/owasp/dependencycheck/analyzer/DependencyBundlingAnalyzer.java +++ b/dependency-check-core/src/main/java/org/owasp/dependencycheck/analyzer/DependencyBundlingAnalyzer.java @@ -464,6 +464,7 @@ public class DependencyBundlingAnalyzer extends AbstractDependencyComparingAnaly /** * Determine if the dependency ecosystem is equal in the given dependencies. * + * @param ecoSystem the ecosystem to validate against * @param dependency a dependency to compare * @param nextDependency a dependency to compare * @return true if the ecosystem is equal in both dependencies; otherwise 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 c0f5b2dce..9121c2680 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 @@ -102,10 +102,10 @@ public class NodePackageAnalyzer extends AbstractFileTypeAnalyzer { protected void prepareFileTypeAnalyzer(Engine engine) throws InitializationException { if (engine.getMode() != Mode.EVIDENCE_COLLECTION) { try { - Settings settings = engine.getSettings(); + final Settings settings = engine.getSettings(); final String[] tmp = settings.getArray(Settings.KEYS.ECOSYSTEM_SKIP_NVDCVE); if (tmp != null) { - List skipEcosystems = Arrays.asList(tmp); + final 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"); 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 d3826563a..21059dfe4 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 @@ -20,7 +20,6 @@ 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; @@ -46,7 +45,11 @@ public class NvdCveAnalyzer extends AbstractAnalyzer { * The Logger for use throughout the class */ private static final org.slf4j.Logger LOGGER = LoggerFactory.getLogger(NvdCveAnalyzer.class); - + /** + * The list of ecosystems to skip during analysis. These are skipped because + * there is generally a more accurate vulnerability analyzer in the + * pipeline. + */ private List skipEcosystems; /** @@ -80,7 +83,7 @@ public class NvdCveAnalyzer extends AbstractAnalyzer { if (skipEcosystems.contains(dependency.getEcosystem())) { return; } - + final CveDB cveDB = engine.getDatabase(); for (Identifier id : dependency.getIdentifiers()) { if ("cpe".equals(id.getType())) { From 9e92e2f8da24ecbca2b5bb9a6d67e57fc5997a10 Mon Sep 17 00:00:00 2001 From: Jeremy Long Date: Sat, 25 Nov 2017 10:06:47 -0500 Subject: [PATCH 08/22] added test case resources --- .../test/resources/nodejs/package-lock.json | 17 +++++++++++++++++ .../src/test/resources/nodejs/shrinkwrap.json | 18 ++++++++++++++++++ 2 files changed, 35 insertions(+) create mode 100644 dependency-check-core/src/test/resources/nodejs/package-lock.json create mode 100644 dependency-check-core/src/test/resources/nodejs/shrinkwrap.json diff --git a/dependency-check-core/src/test/resources/nodejs/package-lock.json b/dependency-check-core/src/test/resources/nodejs/package-lock.json new file mode 100644 index 000000000..16d7aca04 --- /dev/null +++ b/dependency-check-core/src/test/resources/nodejs/package-lock.json @@ -0,0 +1,17 @@ +{ + "name": "test", + "version": "0.0.1", + "lockfileVersion": 1, + "requires": true, + "dependencies": { + "dns-sync": { + "version": "0.1.0", + "resolved": "https://registry.npmjs.org/dns-sync/-/dns-sync-0.1.0.tgz", + "integrity": "sha1-gPcpFC513UtfSx0+Upcx7jEplHI=", + "requires": { + "debug": "2.6.9", + "shelljs": "0.5.3" + } + } + } +} diff --git a/dependency-check-core/src/test/resources/nodejs/shrinkwrap.json b/dependency-check-core/src/test/resources/nodejs/shrinkwrap.json new file mode 100644 index 000000000..32f7033f5 --- /dev/null +++ b/dependency-check-core/src/test/resources/nodejs/shrinkwrap.json @@ -0,0 +1,18 @@ +{ + "name": "test", + "version": "0.0.1", + "lockfileVersion": 1, + "requires": true, + "dependencies": { + "dns-sync": { + "version": "0.1.0", + "resolved": "https://registry.npmjs.org/dns-sync/-/dns-sync-0.1.0.tgz", + "integrity": "sha1-gPcpFC513UtfSx0+Upcx7jEplHI=", + "requires": { + "debug": "2.6.9", + "shelljs": "0.5.3" + } + } + } + } + \ No newline at end of file From 0b32d3b991c82bbed93b6f769bb78b90ce7780a8 Mon Sep 17 00:00:00 2001 From: Jeremy Long Date: Sat, 25 Nov 2017 11:09:46 -0500 Subject: [PATCH 09/22] fixed javadoc --- .../dependencycheck/analyzer/SwiftPackageManagerAnalyzer.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dependency-check-core/src/main/java/org/owasp/dependencycheck/analyzer/SwiftPackageManagerAnalyzer.java b/dependency-check-core/src/main/java/org/owasp/dependencycheck/analyzer/SwiftPackageManagerAnalyzer.java index 90a3cddee..fda55a799 100644 --- a/dependency-check-core/src/main/java/org/owasp/dependencycheck/analyzer/SwiftPackageManagerAnalyzer.java +++ b/dependency-check-core/src/main/java/org/owasp/dependencycheck/analyzer/SwiftPackageManagerAnalyzer.java @@ -67,7 +67,7 @@ public class SwiftPackageManagerAnalyzer extends AbstractFileTypeAnalyzer { public static final String SPM_FILE_NAME = "Package.swift"; /** - * Filter that detects files named "package.json". + * Filter that detects files named "Package.swift". */ private static final FileFilter SPM_FILE_FILTER = FileFilterBuilder.newInstance().addFilenames(SPM_FILE_NAME).build(); From 332bbe72aaa24f7ba4701d1170e11b9378f0fbc1 Mon Sep 17 00:00:00 2001 From: Jeremy Long Date: Sat, 25 Nov 2017 11:13:02 -0500 Subject: [PATCH 10/22] overhaul node package and nsp analyzer --- dependency-check-core/pom.xml | 4 + .../analyzer/AbstractAnalyzer.java | 17 +- .../analyzer/AbstractNpmAnalyzer.java | 289 ++++++++++++++++++ .../analyzer/DependencyBundlingAnalyzer.java | 82 ++++- .../main/resources/dependencycheck.properties | 2 +- .../analyzer/NodePackageAnalyzerTest.java | 36 ++- .../analyzer/NspAnalyzerTest.java | 64 ++-- .../test/resources/dependencycheck.properties | 2 +- .../owasp/dependencycheck/utils/Settings.java | 4 +- pom.xml | 5 + 10 files changed, 432 insertions(+), 73 deletions(-) create mode 100644 dependency-check-core/src/main/java/org/owasp/dependencycheck/analyzer/AbstractNpmAnalyzer.java diff --git a/dependency-check-core/pom.xml b/dependency-check-core/pom.xml index be1c2cd6e..fb93e391f 100644 --- a/dependency-check-core/pom.xml +++ b/dependency-check-core/pom.xml @@ -164,6 +164,10 @@ Copyright (c) 2012 Jeremy Long. All Rights Reserved. + + com.vdurmont + semver4j + joda-time diff --git a/dependency-check-core/src/main/java/org/owasp/dependencycheck/analyzer/AbstractAnalyzer.java b/dependency-check-core/src/main/java/org/owasp/dependencycheck/analyzer/AbstractAnalyzer.java index e208803a0..87c99d558 100644 --- a/dependency-check-core/src/main/java/org/owasp/dependencycheck/analyzer/AbstractAnalyzer.java +++ b/dependency-check-core/src/main/java/org/owasp/dependencycheck/analyzer/AbstractAnalyzer.java @@ -85,6 +85,14 @@ public abstract class AbstractAnalyzer implements Analyzer { @Override public void initialize(Settings settings) { this.settings = settings; + final String key = getAnalyzerEnabledSettingKey(); + try { + this.setEnabled(settings.getBoolean(key, true)); + } catch (InvalidSettingException ex) { + final String msg = String.format("Invalid setting for property '%s'", key); + LOGGER.warn(msg); + LOGGER.debug(msg, ex); + } } /** @@ -95,15 +103,6 @@ public abstract class AbstractAnalyzer implements Analyzer { */ @Override public final void prepare(Engine engine) throws InitializationException { - final String key = getAnalyzerEnabledSettingKey(); - try { - this.setEnabled(settings.getBoolean(key, true)); - } catch (InvalidSettingException ex) { - final String msg = String.format("Invalid setting for property '%s'", key); - LOGGER.warn(msg); - LOGGER.debug(msg, ex); - } - if (isEnabled()) { prepareAnalyzer(engine); } else { diff --git a/dependency-check-core/src/main/java/org/owasp/dependencycheck/analyzer/AbstractNpmAnalyzer.java b/dependency-check-core/src/main/java/org/owasp/dependencycheck/analyzer/AbstractNpmAnalyzer.java new file mode 100644 index 000000000..e61ea8106 --- /dev/null +++ b/dependency-check-core/src/main/java/org/owasp/dependencycheck/analyzer/AbstractNpmAnalyzer.java @@ -0,0 +1,289 @@ +/* + * This file is part of dependency-check-core. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + * + * Copyright (c) 2017 Steve Springett. All Rights Reserved. + */ +package org.owasp.dependencycheck.analyzer; + +import org.owasp.dependencycheck.Engine; +import org.owasp.dependencycheck.dependency.Confidence; +import org.owasp.dependencycheck.dependency.Dependency; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; +import java.io.File; +import java.io.IOException; +import java.util.Map; +import javax.annotation.concurrent.ThreadSafe; +import javax.json.Json; +import javax.json.JsonArray; +import javax.json.JsonObject; +import javax.json.JsonObjectBuilder; +import javax.json.JsonString; +import javax.json.JsonValue; +import org.owasp.dependencycheck.dependency.EvidenceType; +import org.owasp.dependencycheck.utils.Checksum; + +/** + * An abstract NPM analyzer that contains common methods for concrete + * implementations. + * + * @author Steve Springett + */ +@ThreadSafe +public abstract class AbstractNpmAnalyzer extends AbstractFileTypeAnalyzer { + + /** + * The logger. + */ + private static final Logger LOGGER = LoggerFactory.getLogger(AbstractNpmAnalyzer.class); + + /** + * A descriptor for the type of dependencies processed or added by this + * analyzer. + */ + public static final String NPM_DEPENDENCY_ECOSYSTEM = "npm"; + /** + * The file name to scan. + */ + private static final String PACKAGE_JSON = "package.json"; + + /** + * Determines if the file can be analyzed by the analyzer. + * + * @param pathname the path to the file + * @return true if the file can be analyzed by the given analyzer; otherwise + * false + */ + @Override + public boolean accept(File pathname) { + boolean accept = super.accept(pathname); + if (accept) { + try { + // Do not scan the node_modules directory + if (pathname.getCanonicalPath().contains(File.separator + "node_modules" + File.separator)) { + LOGGER.debug("Skipping analysis of node module: " + pathname.getCanonicalPath()); + accept = false; + } + } catch (IOException ex) { + throw new RuntimeException(ex); + } + } + + return accept; + } + + /** + * Construct a dependency object. + * + * @param dependency the parent dependency + * @param name the name of the dependency to create + * @param version the version of the dependency to create + * @param scope the scope of the dependency being created + * @return the generated dependency + */ + protected Dependency createDependency(Dependency dependency, String name, String version, String scope) { + final Dependency nodeModule = new Dependency(new File(dependency.getActualFile() + "?" + name), true); + nodeModule.setEcosystem(NPM_DEPENDENCY_ECOSYSTEM); + //this is virtual - the sha1 is purely for the hyperlink in the final html report + nodeModule.setSha1sum(Checksum.getSHA1Checksum(String.format("%s:%s", name, version))); + nodeModule.setMd5sum(Checksum.getMD5Checksum(String.format("%s:%s", name, version))); + nodeModule.addEvidence(EvidenceType.PRODUCT, "package.json", "name", name, Confidence.HIGHEST); + nodeModule.addEvidence(EvidenceType.VENDOR, "package.json", "name", name, Confidence.HIGH); + nodeModule.addEvidence(EvidenceType.VERSION, "package.json", "version", version, Confidence.HIGHEST); + nodeModule.addProjectReference(dependency.getName() + ": " + scope); + nodeModule.setName(name); + nodeModule.setVersion(version); + nodeModule.addIdentifier("npm", String.format("%s:%s", name, version), null, Confidence.HIGHEST); + return nodeModule; + } + + /** + * Processes a part of package.json (as defined by JsonArray) and update the + * specified dependency with relevant info. + * + * @param engine the dependency-check engine + * @param dependency the Dependency to update + * @param jsonArray the jsonArray to parse + * @param depType the dependency type + */ + protected void processPackage(Engine engine, Dependency dependency, JsonArray jsonArray, String depType) { + final JsonObjectBuilder builder = Json.createObjectBuilder(); + for (JsonString str : jsonArray.getValuesAs(JsonString.class)) { + builder.add(str.toString(), ""); + } + final JsonObject jsonObject = builder.build(); + processPackage(engine, dependency, jsonObject, depType); + } + + /** + * Processes a part of package.json (as defined by JsonObject) and update + * the specified dependency with relevant info. + * + * @param engine the dependency-check engine + * @param dependency the Dependency to update + * @param jsonObject the jsonObject to parse + * @param depType the dependency type + */ + protected void processPackage(Engine engine, Dependency dependency, JsonObject jsonObject, String depType) { + for (int i = 0; i < jsonObject.size(); i++) { + for (Map.Entry entry : jsonObject.entrySet()) { + + final String name = entry.getKey(); + String version = ""; + if (entry.getValue() != null && entry.getValue().getValueType() == JsonValue.ValueType.STRING) { + version = ((JsonString) entry.getValue()).getString(); + } + final Dependency existing = findDependency(engine, name, version); + if (existing == null) { + final Dependency nodeModule = createDependency(dependency, name, version, depType); + engine.addDependency(nodeModule); + } else { + existing.addProjectReference(dependency.getName() + ": " + depType); + } + } + } + } + + /** + * Adds information to an evidence collection from the node json + * configuration. + * + * @param dep the dependency to add the evidence + * @param t the type of evidence to add + * @param json information from node.js + * @return the actual string set into evidence + * @param key the key to obtain the data from the json information + */ + private static String addToEvidence(Dependency dep, EvidenceType t, JsonObject json, String key) { + String evidenceStr = null; + if (json.containsKey(key)) { + final JsonValue value = json.get(key); + if (value instanceof JsonString) { + evidenceStr = ((JsonString) value).getString(); + dep.addEvidence(t, PACKAGE_JSON, key, evidenceStr, Confidence.HIGHEST); + } else if (value instanceof JsonObject) { + final JsonObject jsonObject = (JsonObject) value; + for (final Map.Entry entry : jsonObject.entrySet()) { + final String property = entry.getKey(); + final JsonValue subValue = entry.getValue(); + if (subValue instanceof JsonString) { + evidenceStr = ((JsonString) subValue).getString(); + dep.addEvidence(t, PACKAGE_JSON, + String.format("%s.%s", key, property), + evidenceStr, + Confidence.HIGHEST); + } else { + LOGGER.warn("JSON sub-value not string as expected: {}", subValue); + } + } + } else { + LOGGER.warn("JSON value not string or JSON object as expected: {}", value); + } + } + return evidenceStr; + } + + /** + * Locates the dependency from the list of dependencies that have been + * scanned by the engine. + * + * @param engine the dependency-check engine + * @param name the name of the dependency to find + * @param version the version of the dependency to find + * @return the identified dependency; otherwise null + */ + protected Dependency findDependency(Engine engine, String name, String version) { + for (Dependency d : engine.getDependencies()) { + if (NPM_DEPENDENCY_ECOSYSTEM.equals(d.getEcosystem()) && name.equals(d.getName()) && version != null && d.getVersion() != null) { + String dependencyVersion = d.getVersion(); + if (DependencyBundlingAnalyzer.npmVersionsMatch(version, dependencyVersion)) { + return d; + } +// if (dependencyVersion.startsWith("^") || dependencyVersion.startsWith("~")) { +// dependencyVersion = dependencyVersion.substring(1); +// } +// +// if (version.equals(dependencyVersion)) { +// return d; +// } +// if (version.startsWith("^") || version.startsWith("~") || version.contains("*")) { +// String type; +// String tmp; +// if (version.startsWith("^") || version.startsWith("~")) { +// type = version.substring(0, 1); +// tmp = version.substring(1); +// } else { +// type = "*"; +// tmp = version; +// } +// final String[] v = tmp.split(" ")[0].split("\\."); +// final String[] depVersion = dependencyVersion.split("\\."); +// +// if ("^".equals(type) && v[0].equals(depVersion[0])) { +// return d; +// } else if ("~".equals(type) && v.length >= 2 && depVersion.length >= 2 +// && v[0].equals(depVersion[0]) && v[1].equals(depVersion[1])) { +// return d; +// } else if (v[0].equals("*") +// || (v.length >= 2 && v[0].equals(depVersion[0]) && v[1].equals("*")) +// || (v.length >= 3 && depVersion.length >= 2 && v[0].equals(depVersion[0]) +// && v[1].equals(depVersion[1]) && v[2].equals("*"))) { +// return d; +// } +// } + } + } + return null; + } + + /** + * 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 + */ + public void gatherEvidence(final JsonObject json, Dependency dependency) { + if (json.containsKey("name")) { + final Object value = json.get("name"); + if (value instanceof JsonString) { + final String valueString = ((JsonString) value).getString(); + dependency.setName(valueString); + dependency.setPackagePath(valueString); + dependency.addEvidence(EvidenceType.PRODUCT, PACKAGE_JSON, "name", valueString, Confidence.HIGHEST); + dependency.addEvidence(EvidenceType.VENDOR, PACKAGE_JSON, "name", valueString, Confidence.HIGH); + } else { + LOGGER.warn("JSON value not string as expected: {}", value); + } + } + final String desc = addToEvidence(dependency, EvidenceType.PRODUCT, json, "description"); + dependency.setDescription(desc); + addToEvidence(dependency, EvidenceType.VENDOR, json, "author"); + final String version = addToEvidence(dependency, EvidenceType.VERSION, json, "version"); + if (version != null) { + dependency.setVersion(version); + dependency.addIdentifier("npm", String.format("%s:%s", dependency.getName(), version), null, Confidence.HIGHEST); + } + + // Adds the license if defined in package.json + if (json.containsKey("license")) { + final Object value = json.get("license"); + if (value instanceof JsonString) { + dependency.setLicense(json.getString("license")); + } else { + dependency.setLicense(json.getJsonObject("license").getString("type")); + } + } + } +} diff --git a/dependency-check-core/src/main/java/org/owasp/dependencycheck/analyzer/DependencyBundlingAnalyzer.java b/dependency-check-core/src/main/java/org/owasp/dependencycheck/analyzer/DependencyBundlingAnalyzer.java index 7d6a03b9e..fce847893 100644 --- a/dependency-check-core/src/main/java/org/owasp/dependencycheck/analyzer/DependencyBundlingAnalyzer.java +++ b/dependency-check-core/src/main/java/org/owasp/dependencycheck/analyzer/DependencyBundlingAnalyzer.java @@ -17,6 +17,9 @@ */ package org.owasp.dependencycheck.analyzer; +import com.vdurmont.semver4j.Semver; +import com.vdurmont.semver4j.Semver.SemverType; +import com.vdurmont.semver4j.SemverException; import java.io.File; import java.util.Set; import java.util.regex.Matcher; @@ -135,10 +138,11 @@ public class DependencyBundlingAnalyzer extends AbstractDependencyComparingAnaly mergeDependencies(nextDependency, dependency, dependenciesToRemove); return true; //since we merged into the next dependency - skip forward to the next in mainIterator } - } else if (ecoSystemIs(NspAnalyzer.DEPENDENCY_ECOSYSTEM, dependency, nextDependency) + } else if (ecoSystemIs(AbstractNpmAnalyzer.NPM_DEPENDENCY_ECOSYSTEM, dependency, nextDependency) && namesAreEqual(dependency, nextDependency) - && versionsAreEqual(dependency, nextDependency)) { - if (dependency.isVirtual()) { + && npmVersionsMatch(dependency.getVersion(), nextDependency.getVersion())) { + + if (!dependency.isVirtual()) { DependencyMergingAnalyzer.mergeDependencies(dependency, nextDependency, dependenciesToRemove); } else { DependencyMergingAnalyzer.mergeDependencies(nextDependency, dependency, dependenciesToRemove); @@ -158,7 +162,7 @@ public class DependencyBundlingAnalyzer extends AbstractDependencyComparingAnaly * removed from the main analysis loop, this function adds to this * collection */ - private void mergeDependencies(final Dependency dependency, final Dependency relatedDependency, final Set dependenciesToRemove) { + public static void mergeDependencies(final Dependency dependency, final Dependency relatedDependency, final Set dependenciesToRemove) { dependency.addRelatedDependency(relatedDependency); for (Dependency d : relatedDependency.getRelatedDependencies()) { dependency.addRelatedDependency(d); @@ -167,7 +171,9 @@ public class DependencyBundlingAnalyzer extends AbstractDependencyComparingAnaly if (dependency.getSha1sum().equals(relatedDependency.getSha1sum())) { dependency.addAllProjectReferences(relatedDependency.getProjectReferences()); } - dependenciesToRemove.add(relatedDependency); + if (dependenciesToRemove != null) { + dependenciesToRemove.add(relatedDependency); + } } /** @@ -487,14 +493,72 @@ public class DependencyBundlingAnalyzer extends AbstractDependencyComparingAnaly /** * Determine if the dependency version is equal in the given dependencies. + * This method attempts to evaluate version range checks. * - * @param dependency a dependency to compare - * @param nextDependency a dependency to compare + * @param current a dependency version to compare + * @param nextDependency a dependency version to compare * @return true if the version is equal in both dependencies; otherwise * false */ - private boolean versionsAreEqual(Dependency dependency, Dependency nextDependency) { - return dependency.getVersion() != null && dependency.getVersion().equals(nextDependency.getVersion()); + public static boolean npmVersionsMatch(String current, String next) { + String left = current; + String right = next; + if (left == null || right == null) { + return false; + } + if (left.equals(right) || "*".equals(left) || "*".equals(right)) { + return true; + } + if (left.contains(" ")) { // we have a version string from package.json + if (right.contains(" ")) { // we can't evaluate this ">=1.5.4 <2.0.0" vs "2 || 3" + return false; + } + if (!right.matches("^\\d.*$")) { + right = stripLeadingNonNumeric(right); + if (right == null) { + return false; + } + } + try { + Semver v = new Semver(right, SemverType.NPM); + return v.satisfies(left); + } catch (SemverException ex) { + LOGGER.trace("ignore", ex); + } + } else { + if (!left.matches("^\\d.*$")) { + left = stripLeadingNonNumeric(left); + if (left == null) { + return false; + } + } + try { + Semver v = new Semver(left, SemverType.NPM); + if (v.satisfies(right)) { + return true; + } + if (!right.contains((" "))) { + left = current; + right = stripLeadingNonNumeric(right); + if (right != null) { + v = new Semver(right, SemverType.NPM); + return v.satisfies(left); + } + } + } catch (SemverException ex) { + LOGGER.trace("ignore", ex); + } + } + return false; + } + + private static String stripLeadingNonNumeric(String str) { + for (int x = 0; x < str.length(); x++) { + if (Character.isDigit(str.codePointAt(x))) { + return str.substring(x); + } + } + return null; } } diff --git a/dependency-check-core/src/main/resources/dependencycheck.properties b/dependency-check-core/src/main/resources/dependencycheck.properties index ed7e23256..259c724d4 100644 --- a/dependency-check-core/src/main/resources/dependencycheck.properties +++ b/dependency-check-core/src/main/resources/dependencycheck.properties @@ -128,4 +128,4 @@ updater.nvdcve.enabled=true updater.versioncheck.enabled=true analyzer.versionfilter.enabled=true -ecosystem.skip.nvdcve=npm \ No newline at end of file +ecosystem.skip.cpeanalyzer=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 9b0cc2b41..578b39e17 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 @@ -43,6 +43,9 @@ public class NodePackageAnalyzerTest extends BaseTest { * The analyzer to test. */ private NodePackageAnalyzer analyzer; + /** + * A reference to the engine. + */ private Engine engine; /** @@ -87,7 +90,8 @@ public class NodePackageAnalyzerTest extends BaseTest { */ @Test public void testSupportsFiles() { - assertThat(analyzer.accept(new File("package.json")), is(true)); + assertThat(analyzer.accept(new File("package-lock.json")), is(true)); + assertThat(analyzer.accept(new File("shrinkwrap.json")), is(true)); } /** @@ -96,10 +100,12 @@ public class NodePackageAnalyzerTest extends BaseTest { * @throws AnalysisException is thrown when an exception occurs. */ @Test - public void testAnalyzePackageJson() throws AnalysisException { - final Dependency result = new Dependency(BaseTest.getResourceAsFile(this, - "nodejs/node_modules/dns-sync/package.json")); - analyzer.analyze(result, null); + public void testAnalyzeShrinkwrapJson() throws AnalysisException { + final Dependency toScan = new Dependency(BaseTest.getResourceAsFile(this, + "nodejs/shrinkwrap.json")); + analyzer.analyze(toScan, engine); + assertEquals("Expected 1 dependency", engine.getDependencies().length, 1); + final Dependency result = engine.getDependencies()[0]; final String vendorString = result.getEvidence(EvidenceType.VENDOR).toString(); assertThat(vendorString, containsString("Sanjeev Koranga")); assertThat(vendorString, containsString("dns-sync")); @@ -109,4 +115,24 @@ public class NodePackageAnalyzerTest extends BaseTest { assertEquals("dns-sync", result.getName()); assertEquals("0.1.0", result.getVersion()); } + + /** + * Test of inspect method, of class PythonDistributionAnalyzer. + * + * @throws AnalysisException is thrown when an exception occurs. + */ + @Test + public void testAnalyzePackageJsonWithShrinkwrap() throws AnalysisException { + final Dependency packageLock = new Dependency(BaseTest.getResourceAsFile(this, + "nodejs/package-lock.json")); + final Dependency shrinkwrap = new Dependency(BaseTest.getResourceAsFile(this, + "nodejs/shrinkwrap.json")); + engine.addDependency(packageLock); + engine.addDependency(shrinkwrap); + assertEquals(2, engine.getDependencies().length); + analyzer.analyze(packageLock, engine); + assertEquals(1, engine.getDependencies().length); //package-lock was removed without analysis + analyzer.analyze(shrinkwrap, engine); + assertEquals(1, engine.getDependencies().length); //shrinkwrap was removed with analysis adding 1 dependency + } } diff --git a/dependency-check-core/src/test/java/org/owasp/dependencycheck/analyzer/NspAnalyzerTest.java b/dependency-check-core/src/test/java/org/owasp/dependencycheck/analyzer/NspAnalyzerTest.java index 508b58d0d..edb56b113 100644 --- a/dependency-check-core/src/test/java/org/owasp/dependencycheck/analyzer/NspAnalyzerTest.java +++ b/dependency-check-core/src/test/java/org/owasp/dependencycheck/analyzer/NspAnalyzerTest.java @@ -1,7 +1,5 @@ package org.owasp.dependencycheck.analyzer; -import org.junit.After; -import org.junit.Before; import org.junit.Test; import org.owasp.dependencycheck.BaseTest; import org.owasp.dependencycheck.analyzer.exception.AnalysisException; @@ -36,12 +34,20 @@ public class NspAnalyzerTest extends BaseTest { analyzer.setFilesMatched(true); analyzer.initialize(getSettings()); analyzer.prepare(engine); - final Dependency result = new Dependency(BaseTest.getResourceAsFile(this, "nsp/package.json")); - analyzer.analyze(result, engine); - - assertTrue(result.getEvidence(EvidenceType.VENDOR).toString().contains("owasp-nodejs-goat")); - assertTrue(result.getEvidence(EvidenceType.PRODUCT).toString().contains("A tool to learn OWASP Top 10 for node.js developers")); - assertTrue(result.getEvidence(EvidenceType.VERSION).toString().contains("1.3.0")); + final Dependency toScan = new Dependency(BaseTest.getResourceAsFile(this, "nsp/package.json")); + analyzer.analyze(toScan, engine); + boolean found = false; + assertEquals("4 dependencies should be identified", 4, engine.getDependencies().length); + for (Dependency result : engine.getDependencies()) { + if ("package.json?uglify-js".equals(result.getFileName())) { + found = true; + assertTrue(result.getEvidence(EvidenceType.VENDOR).toString().contains("uglify-js")); + assertTrue(result.getEvidence(EvidenceType.PRODUCT).toString().contains("uglify-js")); + assertTrue(result.getEvidence(EvidenceType.VERSION).toString().contains("2.4.24")); + assertTrue(result.isVirtual()); + } + } + assertTrue("Uglify was not found", found); } } @@ -61,38 +67,6 @@ public class NspAnalyzerTest extends BaseTest { } } - @Test - public void testAnalyzePackageJsonWithBundledDeps() throws AnalysisException, InitializationException { - try (Engine engine = new Engine(getSettings())) { - NspAnalyzer analyzer = new NspAnalyzer(); - analyzer.setFilesMatched(true); - analyzer.initialize(getSettings()); - analyzer.prepare(engine); - final Dependency result = new Dependency(BaseTest.getResourceAsFile(this, "nsp/bundled.deps.package.json")); - analyzer.analyze(result, engine); - - assertTrue(result.getEvidence(EvidenceType.VENDOR).toString().contains("Philipp Dunkel ")); - assertTrue(result.getEvidence(EvidenceType.PRODUCT).toString().contains("Native Access to Mac OS-X FSEvents")); - assertTrue(result.getEvidence(EvidenceType.VERSION).toString().contains("1.1.1")); - } - } - - @Test - public void testAnalyzePackageJsonWithLicenseObject() throws AnalysisException, InitializationException { - try (Engine engine = new Engine(getSettings())) { - NspAnalyzer analyzer = new NspAnalyzer(); - analyzer.setFilesMatched(true); - analyzer.initialize(getSettings()); - analyzer.prepare(engine); - final Dependency result = new Dependency(BaseTest.getResourceAsFile(this, "nsp/license.obj.package.json")); - analyzer.analyze(result, engine); - - assertTrue(result.getEvidence(EvidenceType.VENDOR).toString().contains("Twitter, Inc.")); - assertTrue(result.getEvidence(EvidenceType.PRODUCT).toString().contains("The most popular front-end framework for developing responsive, mobile first projects on the web")); - assertTrue(result.getEvidence(EvidenceType.VERSION).toString().contains("3.2.0")); - } - } - @Test public void testAnalyzePackageJsonInNodeModulesDirectory() throws AnalysisException, InitializationException { try (Engine engine = new Engine(getSettings())) { @@ -100,12 +74,10 @@ public class NspAnalyzerTest extends BaseTest { analyzer.setFilesMatched(true); analyzer.initialize(getSettings()); analyzer.prepare(engine); - final Dependency result = new Dependency(BaseTest.getResourceAsFile(this, "nodejs/node_modules/dns-sync/package.json")); - analyzer.analyze(result, engine); - // package.json adds 5 bits of evidence - assertTrue(result.size() == 5); - // but no vulnerabilities were cited - assertTrue(result.getVulnerabilities().isEmpty()); + final Dependency toScan = new Dependency(BaseTest.getResourceAsFile(this, "nodejs/node_modules/dns-sync/package.json")); + engine.addDependency(toScan); + analyzer.analyze(toScan, engine); + assertEquals("No dependencies should exist", 0, engine.getDependencies().length); } } diff --git a/dependency-check-core/src/test/resources/dependencycheck.properties b/dependency-check-core/src/test/resources/dependencycheck.properties index 133ee11c0..ce2875943 100644 --- a/dependency-check-core/src/test/resources/dependencycheck.properties +++ b/dependency-check-core/src/test/resources/dependencycheck.properties @@ -124,4 +124,4 @@ analyzer.vulnerabilitysuppression.enabled=true updater.nvdcve.enabled=true updater.versioncheck.enabled=true -ecosystem.skip.nvdcve=npm \ No newline at end of file +ecosystem.skip.cpeanalyzer=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 730c2de5d..a33ba86cd 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 @@ -443,9 +443,9 @@ public final class Settings { */ public static final String UPDATE_VERSION_CHECK_ENABLED = "updater.versioncheck.enabled"; /** - * The key to determine which ecosystems should skip the NVD CVE analysis. + * The key to determine which ecosystems should skip the CPE analysis. */ - public static final String ECOSYSTEM_SKIP_NVDCVE = "ecosystem.skip.nvdcve"; + public static final String ECOSYSTEM_SKIP_CPEANALYZER = "ecosystem.skip.cpeanalyzer"; /** * private constructor because this is a "utility" class containing diff --git a/pom.xml b/pom.xml index 86da3e0be..b2410c6ac 100644 --- a/pom.xml +++ b/pom.xml @@ -625,6 +625,11 @@ Copyright (c) 2012 - Jeremy Long + + com.vdurmont + semver4j + 2.1.0 + joda-time From 9e6cf2e6f30f252d10142743271dd4b7b53fb139 Mon Sep 17 00:00:00 2001 From: Jeremy Long Date: Sat, 25 Nov 2017 11:13:16 -0500 Subject: [PATCH 11/22] overhaul node package and nsp analyzer --- .../resources/nsp/bundled.deps.package.json | 48 ----------- .../resources/nsp/license.obj.package.json | 81 ------------------- 2 files changed, 129 deletions(-) delete mode 100644 dependency-check-core/src/test/resources/nsp/bundled.deps.package.json delete mode 100644 dependency-check-core/src/test/resources/nsp/license.obj.package.json diff --git a/dependency-check-core/src/test/resources/nsp/bundled.deps.package.json b/dependency-check-core/src/test/resources/nsp/bundled.deps.package.json deleted file mode 100644 index 281347992..000000000 --- a/dependency-check-core/src/test/resources/nsp/bundled.deps.package.json +++ /dev/null @@ -1,48 +0,0 @@ -{ - "name": "fsevents", - "version": "1.1.1", - "description": "Native Access to Mac OS-X FSEvents", - "main": "fsevents.js", - "dependencies": { - "nan": "^2.3.0", - "node-pre-gyp": "^0.6.29" - }, - "os": [ - "darwin" - ], - "engines": { - "node": ">=0.8.0" - }, - "scripts": { - "install": "node install", - "prepublish": "if [ $(npm -v | head -c 1) -lt 3 ]; then exit 1; fi && npm dedupe", - "test": "tap ./test" - }, - "binary": { - "module_name": "fse", - "module_path": "./lib/binding/{configuration}/{node_abi}-{platform}-{arch}/", - "remote_path": "./v{version}/", - "package_name": "{module_name}-v{version}-{node_abi}-{platform}-{arch}.tar.gz", - "host": "https://fsevents-binaries.s3-us-west-2.amazonaws.com" - }, - "repository": { - "type": "git", - "url": "https://github.com/strongloop/fsevents.git" - }, - "keywords": [ - "fsevents", - "mac" - ], - "author": "Philipp Dunkel ", - "license": "MIT", - "bugs": { - "url": "https://github.com/strongloop/fsevents/issues" - }, - "bundledDependencies": [ - "node-pre-gyp" - ], - "homepage": "https://github.com/strongloop/fsevents", - "devDependencies": { - "tap": "~0.4.8" - } -} diff --git a/dependency-check-core/src/test/resources/nsp/license.obj.package.json b/dependency-check-core/src/test/resources/nsp/license.obj.package.json deleted file mode 100644 index 3243fa8ba..000000000 --- a/dependency-check-core/src/test/resources/nsp/license.obj.package.json +++ /dev/null @@ -1,81 +0,0 @@ -{ - "name": "bootstrap", - "description": "The most popular front-end framework for developing responsive, mobile first projects on the web.", - "version": "3.2.0", - "keywords": [ - "css", - "less", - "mobile-first", - "responsive", - "front-end", - "framework", - "web" - ], - "homepage": "http://getbootstrap.com", - "author": "Twitter, Inc.", - "scripts": { - "test": "grunt test" - }, - "style": "dist/css/bootstrap.css", - "less": "less/bootstrap.less", - "repository": { - "type": "git", - "url": "https://github.com/twbs/bootstrap.git" - }, - "bugs": { - "url": "https://github.com/twbs/bootstrap/issues" - }, - "license": { - "type": "MIT", - "url": "https://github.com/twbs/bootstrap/blob/master/LICENSE" - }, - "devDependencies": { - "btoa": "~1.1.2", - "glob": "~4.0.2", - "grunt": "~0.4.5", - "grunt-autoprefixer": "~0.7.6", - "grunt-banner": "~0.2.3", - "grunt-contrib-clean": "~0.5.0", - "grunt-contrib-concat": "~0.4.0", - "grunt-contrib-connect": "~0.8.0", - "grunt-contrib-copy": "~0.5.0", - "grunt-contrib-csslint": "~0.2.0", - "grunt-contrib-cssmin": "~0.10.0", - "grunt-contrib-jade": "~0.12.0", - "grunt-contrib-jshint": "~0.10.0", - "grunt-contrib-less": "~0.11.3", - "grunt-contrib-qunit": "~0.5.1", - "grunt-contrib-uglify": "~0.5.0", - "grunt-contrib-watch": "~0.6.1", - "grunt-csscomb": "~2.0.1", - "grunt-exec": "~0.4.5", - "grunt-html-validation": "~0.1.18", - "grunt-jekyll": "~0.4.2", - "grunt-jscs-checker": "~0.6.0", - "grunt-saucelabs": "~8.1.0", - "grunt-sed": "~0.1.1", - "load-grunt-tasks": "~0.6.0", - "markdown": "~0.5.0", - "npm-shrinkwrap": "~3.1.6", - "time-grunt": "~0.3.2" - }, - "engines": { - "node": "~0.10.1" - }, - "jspm": { - "main": "js/bootstrap", - "directories": { - "example": "examples", - "lib": "dist" - }, - "shim": { - "js/bootstrap": { - "imports": "jquery", - "exports": "$" - } - }, - "buildConfig": { - "uglify": true - } - } -} From c58ec0ff8cbd2cbf790b1fdfb4d75135ecfb05a8 Mon Sep 17 00:00:00 2001 From: Jeremy Long Date: Sat, 25 Nov 2017 11:14:29 -0500 Subject: [PATCH 12/22] overhaul node package and nsp analyzer --- .../dependencycheck/analyzer/CPEAnalyzer.java | 18 ++ .../analyzer/DependencyMergingAnalyzer.java | 5 +- .../analyzer/NodePackageAnalyzer.java | 217 ++++++++++------- .../dependencycheck/analyzer/NspAnalyzer.java | 218 ++---------------- .../analyzer/NvdCveAnalyzer.java | 35 +-- 5 files changed, 167 insertions(+), 326 deletions(-) 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 ae2a55d35..34685d6bf 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 @@ -21,6 +21,7 @@ import java.io.IOException; import java.io.UnsupportedEncodingException; import java.net.URLEncoder; import java.util.ArrayList; +import java.util.Arrays; import java.util.Collections; import java.util.List; import java.util.Set; @@ -106,6 +107,12 @@ public class CPEAnalyzer extends AbstractAnalyzer { * The CVE Database. */ private CveDB cve; + /** + * The list of ecosystems to skip during analysis. These are skipped because + * there is generally a more accurate vulnerability analyzer in the + * pipeline. + */ + private List skipEcosystems; /** * Returns the name of this analyzer. @@ -136,6 +143,7 @@ public class CPEAnalyzer extends AbstractAnalyzer { */ @Override public void prepareAnalyzer(Engine engine) throws InitializationException { + super.prepareAnalyzer(engine); try { this.open(engine.getDatabase()); } catch (IOException ex) { @@ -145,6 +153,13 @@ public class CPEAnalyzer extends AbstractAnalyzer { LOGGER.debug("Exception accessing the database", ex); throw new InitializationException("An exception occurred accessing the database", ex); } + final String[] tmp = engine.getSettings().getArray(Settings.KEYS.ECOSYSTEM_SKIP_CPEANALYZER); + if (tmp == null) { + skipEcosystems = new ArrayList<>(); + } else { + LOGGER.info("Skipping CPE Analysis for {}", tmp); + skipEcosystems = Arrays.asList(tmp); + } } /** @@ -525,6 +540,9 @@ public class CPEAnalyzer extends AbstractAnalyzer { */ @Override protected void analyzeDependency(Dependency dependency, Engine engine) throws AnalysisException { + if (skipEcosystems.contains(dependency.getEcosystem())) { + return; + } try { determineCPE(dependency); } catch (CorruptIndexException ex) { diff --git a/dependency-check-core/src/main/java/org/owasp/dependencycheck/analyzer/DependencyMergingAnalyzer.java b/dependency-check-core/src/main/java/org/owasp/dependencycheck/analyzer/DependencyMergingAnalyzer.java index 22d37c8a0..fb98e7145 100644 --- a/dependency-check-core/src/main/java/org/owasp/dependencycheck/analyzer/DependencyMergingAnalyzer.java +++ b/dependency-check-core/src/main/java/org/owasp/dependencycheck/analyzer/DependencyMergingAnalyzer.java @@ -139,8 +139,9 @@ public class DependencyMergingAnalyzer extends AbstractDependencyComparingAnalyz relatedDependency.removeRelatedDependencies(d); } dependency.addAllProjectReferences(relatedDependency.getProjectReferences()); - - dependenciesToRemove.add(relatedDependency); + if (dependenciesToRemove != null) { + dependenciesToRemove.add(relatedDependency); + } } /** 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 9121c2680..609bed1f6 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 @@ -20,7 +20,6 @@ package org.owasp.dependencycheck.analyzer; import org.apache.commons.io.FileUtils; import org.owasp.dependencycheck.Engine; import org.owasp.dependencycheck.analyzer.exception.AnalysisException; -import org.owasp.dependencycheck.dependency.Confidence; import org.owasp.dependencycheck.dependency.Dependency; import org.owasp.dependencycheck.utils.FileFilterBuilder; import org.owasp.dependencycheck.utils.Settings; @@ -30,6 +29,7 @@ import org.slf4j.LoggerFactory; import java.io.File; import java.io.FileFilter; import java.io.IOException; +import java.nio.file.Paths; import java.util.Arrays; import java.util.List; import java.util.Map; @@ -38,11 +38,12 @@ import javax.json.Json; import javax.json.JsonException; 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.Confidence; import org.owasp.dependencycheck.dependency.EvidenceType; +import org.owasp.dependencycheck.exception.InitializationException; +import org.owasp.dependencycheck.utils.Checksum; import org.owasp.dependencycheck.utils.InvalidSettingException; /** @@ -52,7 +53,7 @@ import org.owasp.dependencycheck.utils.InvalidSettingException; * @author Dale Visser */ @ThreadSafe -public class NodePackageAnalyzer extends AbstractFileTypeAnalyzer { +public class NodePackageAnalyzer extends AbstractNpmAnalyzer { /** * The logger. @@ -62,7 +63,7 @@ public class NodePackageAnalyzer 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 = NPM_DEPENDENCY_ECOSYSTEM; /** * The name of the analyzer. */ @@ -76,10 +77,18 @@ public class NodePackageAnalyzer extends AbstractFileTypeAnalyzer { */ public static final String PACKAGE_JSON = "package.json"; /** - * Filter that detects files named "package.json". + * The file name to scan. + */ + public static final String PACKAGE_LOCK_JSON = "package-lock.json"; + /** + * The file name to scan. + */ + public static final String SHRINKWRAP_JSON = "shrinkwrap.json"; + /** + * Filter that detects files named "package-lock.json" or "shrinkwrap.json". */ private static final FileFilter PACKAGE_JSON_FILTER = FileFilterBuilder.newInstance() - .addFilenames(PACKAGE_JSON).build(); + .addFilenames(PACKAGE_LOCK_JSON, SHRINKWRAP_JSON).build(); /** * Returns the FileFilter @@ -103,7 +112,7 @@ public class NodePackageAnalyzer extends AbstractFileTypeAnalyzer { if (engine.getMode() != Mode.EVIDENCE_COLLECTION) { try { final Settings settings = engine.getSettings(); - final String[] tmp = settings.getArray(Settings.KEYS.ECOSYSTEM_SKIP_NVDCVE); + final String[] tmp = settings.getArray(Settings.KEYS.ECOSYSTEM_SKIP_CPEANALYZER); if (tmp != null) { final List skipEcosystems = Arrays.asList(tmp); if (skipEcosystems.contains(DEPENDENCY_ECOSYSTEM) @@ -113,7 +122,7 @@ public class NodePackageAnalyzer extends AbstractFileTypeAnalyzer { + "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."); + LOGGER.warn("Using the CPE Analyzer with Node.js can result in many false positives."); } } } catch (InvalidSettingException ex) { @@ -143,10 +152,10 @@ public class NodePackageAnalyzer extends AbstractFileTypeAnalyzer { } /** - * Returns the key used in the properties file to reference the analyzer's - * enabled property. + * Returns the key used in the properties file to reference the enabled + * property for the analyzer. * - * @return the analyzer's enabled property setting key + * @return the enabled property setting key for the analyzer */ @Override protected String getAnalyzerEnabledSettingKey() { @@ -155,16 +164,34 @@ public class NodePackageAnalyzer extends AbstractFileTypeAnalyzer { @Override protected void analyzeDependency(Dependency dependency, Engine engine) throws AnalysisException { - dependency.setEcosystem(DEPENDENCY_ECOSYSTEM); - final File file = dependency.getActualFile(); + engine.removeDependency(dependency); + File file = dependency.getActualFile(); if (!file.isFile() || file.length() == 0) { return; } + try { + // Do not scan the node_modules directory + if (file.getCanonicalPath().contains(File.separator + "node_modules" + File.separator)) { + LOGGER.debug("Skipping analysis of node module: " + file.getCanonicalPath()); + return; + } + } catch (IOException ex) { + throw new RuntimeException(ex); + } + File baseDir = file.getParentFile(); + if (PACKAGE_LOCK_JSON.equals(dependency.getFileName())) { + File shrinkwrap = new File(baseDir, SHRINKWRAP_JSON); + if (shrinkwrap.exists()) { + return; + } + } + try (JsonReader jsonReader = Json.createReader(FileUtils.openInputStream(file))) { final JsonObject json = jsonReader.readObject(); - - gatherEvidence(json, dependency); - + final String parentName = json.getString("name"); + final String parentVersion = json.getString("version"); + final String parentPackage = String.format("%s:%s", parentName, parentVersion); + processDependencies(json, baseDir, file, parentPackage, engine); } catch (JsonException e) { LOGGER.warn("Failed to parse package.json file.", e); } catch (IOException e) { @@ -172,80 +199,94 @@ 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 - */ - public static void gatherEvidence(final JsonObject json, Dependency dependency) { - if (json.containsKey("name")) { - final Object value = json.get("name"); - if (value instanceof JsonString) { - final String valueString = ((JsonString) value).getString(); - dependency.setName(valueString); - dependency.setPackagePath(valueString); - dependency.addEvidence(EvidenceType.PRODUCT, PACKAGE_JSON, "name", valueString, Confidence.HIGHEST); - dependency.addEvidence(EvidenceType.VENDOR, PACKAGE_JSON, "name", valueString, Confidence.HIGH); - } else { - LOGGER.warn("JSON value not string as expected: {}", value); - } - } - addToEvidence(dependency, EvidenceType.PRODUCT, json, "description"); - addToEvidence(dependency, EvidenceType.VENDOR, json, "author"); - final String version = addToEvidence(dependency, EvidenceType.VERSION, json, "version"); - if (version != null) { - dependency.setVersion(version); - dependency.addIdentifier("npm", String.format("%s:%s", dependency.getName(), version), null, Confidence.HIGHEST); - } + private void processDependencies(final JsonObject json, File baseDir, File rootFile, final String parentPackage, Engine engine) throws AnalysisException { + if (json.containsKey("dependencies")) { + JsonObject deps = json.getJsonObject("dependencies"); + for (Map.Entry entry : deps.entrySet()) { + JsonObject jo = (JsonObject) entry.getValue(); + final String name = entry.getKey(); + final String version = jo.getString("version"); + File base = Paths.get(baseDir.getPath(), "node_modules", name).toFile(); + File f = new File(base, PACKAGE_JSON); - // Adds the license if defined in package.json - if (json.containsKey("license")) { - final Object value = json.get("license"); - if (value instanceof JsonString) { - dependency.setLicense(json.getString("license")); - } else { - dependency.setLicense(json.getJsonObject("license").getString("type")); - } - } - } - - /** - * Adds information to an evidence collection from the node json - * configuration. - * - * @param dep the dependency to add the evidence - * @param t the type of evidence to add - * @param json information from node.js - * @return the actual string set into evidence - * @param key the key to obtain the data from the json information - */ - private static String addToEvidence(Dependency dep, EvidenceType t, JsonObject json, String key) { - String evidenceStr = null; - if (json.containsKey(key)) { - final JsonValue value = json.get(key); - if (value instanceof JsonString) { - evidenceStr = ((JsonString) value).getString(); - dep.addEvidence(t, PACKAGE_JSON, key, evidenceStr, Confidence.HIGHEST); - } else if (value instanceof JsonObject) { - final JsonObject jsonObject = (JsonObject) value; - for (final Map.Entry entry : jsonObject.entrySet()) { - final String property = entry.getKey(); - final JsonValue subValue = entry.getValue(); - if (subValue instanceof JsonString) { - evidenceStr = ((JsonString) subValue).getString(); - dep.addEvidence(t, PACKAGE_JSON, - String.format("%s.%s", key, property), - evidenceStr, - Confidence.HIGHEST); - } else { - LOGGER.warn("JSON sub-value not string as expected: {}", subValue); - } + if (jo.containsKey("dependencies")) { + final String subPackageName = String.format("%s/%s:%s", parentPackage, name, version); + processDependencies(jo, base, rootFile, subPackageName, engine); + } + + Dependency child; + if (f.exists()) { + //TOOD - we should use the integrity value instead of calculating the SHA1/MD5 + child = new Dependency(f); + try (JsonReader jr = Json.createReader(FileUtils.openInputStream(f))) { + JsonObject childJson = jr.readObject(); + gatherEvidence(childJson, child); + + } catch (JsonException e) { + LOGGER.warn("Failed to parse package.json file from dependency.", e); + } catch (IOException e) { + throw new AnalysisException("Problem occurred while reading dependency file.", e); + } + } else { + LOGGER.error("Unable to find child file {}", f.toString()); + child = new Dependency(rootFile, true); + //TOOD - we should use the integrity value instead of calculating the SHA1/MD5 + child.setSha1sum(Checksum.getSHA1Checksum(String.format("%s:%s", name, version))); + child.setMd5sum(Checksum.getMD5Checksum(String.format("%s:%s", name, version))); + child.addEvidence(EvidenceType.VENDOR, rootFile.getName(), "name", name, Confidence.HIGHEST); + child.addEvidence(EvidenceType.PRODUCT, rootFile.getName(), "name", name, Confidence.HIGHEST); + child.addEvidence(EvidenceType.VERSION, rootFile.getName(), "version", version, Confidence.HIGHEST); + } + child.setName(name); + child.setVersion(version); + child.addProjectReference(parentPackage); + child.setEcosystem(DEPENDENCY_ECOSYSTEM); + + Dependency existing = findDependency(engine, name, version); + if (existing != null) { + if (existing.isVirtual()) { + DependencyMergingAnalyzer.mergeDependencies(child, existing, null); + engine.removeDependency(existing); + engine.addDependency(child); + } else { + DependencyBundlingAnalyzer.mergeDependencies(existing, child, null); + } + } else { + engine.addDependency(child); } - } else { - LOGGER.warn("JSON value not string or JSON object as expected: {}", value); } } - return evidenceStr; + +// gatherEvidence(json, dependency); +// +// // only run this if we are in evidence collection or the NSP analyzer has been disabled +// if (engine.getMode() == Mode.EVIDENCE_COLLECTION +// || !engine.getSettings().getBoolean(Settings.KEYS.ANALYZER_NSP_PACKAGE_ENABLED)) { +// //Processes the dependencies objects in package.json and adds all the modules as dependencies +// if (json.containsKey("dependencies")) { +// final JsonObject dependencies = json.getJsonObject("dependencies"); +// processPackage(engine, dependency, dependencies, "dependencies"); +// } +// if (json.containsKey("devDependencies")) { +// final JsonObject dependencies = json.getJsonObject("devDependencies"); +// processPackage(engine, dependency, dependencies, "devDependencies"); +// } +// if (json.containsKey("optionalDependencies")) { +// final JsonObject dependencies = json.getJsonObject("optionalDependencies"); +// processPackage(engine, dependency, dependencies, "optionalDependencies"); +// } +// if (json.containsKey("peerDependencies")) { +// final JsonObject dependencies = json.getJsonObject("peerDependencies"); +// processPackage(engine, dependency, dependencies, "peerDependencies"); +// } +// if (json.containsKey("bundleDependencies")) { +// final JsonArray dependencies = json.getJsonArray("bundleDependencies"); +// processPackage(engine, dependency, dependencies, "bundleDependencies"); +// } +// if (json.containsKey("bundledDependencies")) { +// final JsonArray dependencies = json.getJsonArray("bundledDependencies"); +// processPackage(engine, dependency, dependencies, "bundledDependencies"); +// } +// } } } 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 cd04af940..088df25fb 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 @@ -23,7 +23,6 @@ import org.owasp.dependencycheck.analyzer.exception.AnalysisException; import org.owasp.dependencycheck.data.nsp.Advisory; import org.owasp.dependencycheck.data.nsp.NspSearch; import org.owasp.dependencycheck.data.nsp.SanitizePackage; -import org.owasp.dependencycheck.dependency.Confidence; import org.owasp.dependencycheck.dependency.Dependency; import org.owasp.dependencycheck.dependency.Vulnerability; import org.owasp.dependencycheck.dependency.VulnerableSoftware; @@ -38,19 +37,13 @@ import java.net.MalformedURLException; import java.util.Arrays; import java.util.HashSet; import java.util.List; -import java.util.Map; import javax.annotation.concurrent.ThreadSafe; import javax.json.Json; -import javax.json.JsonArray; import javax.json.JsonException; import javax.json.JsonObject; import javax.json.JsonObjectBuilder; import javax.json.JsonReader; -import javax.json.JsonString; -import javax.json.JsonValue; -import org.owasp.dependencycheck.dependency.EvidenceType; import org.owasp.dependencycheck.exception.InitializationException; -import org.owasp.dependencycheck.utils.Checksum; import org.owasp.dependencycheck.utils.URLConnectionFailureException; /** @@ -60,7 +53,7 @@ import org.owasp.dependencycheck.utils.URLConnectionFailureException; * @author Steve Springett */ @ThreadSafe -public class NspAnalyzer extends AbstractFileTypeAnalyzer { +public class NspAnalyzer extends AbstractNpmAnalyzer { /** * The logger. @@ -75,7 +68,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 = NodePackageAnalyzer.DEPENDENCY_ECOSYSTEM; + public static final String DEPENDENCY_ECOSYSTEM = NPM_DEPENDENCY_ECOSYSTEM; /** * The file name to scan. */ @@ -152,53 +145,27 @@ public class NspAnalyzer extends AbstractFileTypeAnalyzer { @Override protected void analyzeDependency(Dependency dependency, Engine engine) throws AnalysisException { + engine.removeDependency(dependency); final File file = dependency.getActualFile(); if (!file.isFile() || file.length() == 0) { return; } + try { + // Do not scan the node_modules directory + if (file.getCanonicalPath().contains(File.separator + "node_modules" + File.separator)) { + LOGGER.debug("Skipping analysis of node module: " + file.getCanonicalPath()); + return; + } + } catch (IOException ex) { + throw new RuntimeException(ex); + } + try (JsonReader jsonReader = Json.createReader(FileUtils.openInputStream(file))) { // Retrieves the contents of package.json from the Dependency final JsonObject packageJson = jsonReader.readObject(); - if (dependency.getEcosystem() == null || dependency.getName() == null) { - NodePackageAnalyzer.gatherEvidence(packageJson, dependency); - dependency.setEcosystem(DEPENDENCY_ECOSYSTEM); - } - - // Do not scan the node_modules directory - if (file.getCanonicalPath().contains(File.separator + "node_modules" + File.separator)) { - LOGGER.debug("Skipping analysis of node module: " + file.getCanonicalPath()); - return; - } - - //Processes the dependencies objects in package.json and adds all the modules as dependencies - if (packageJson.containsKey("dependencies")) { - final JsonObject dependencies = packageJson.getJsonObject("dependencies"); - processPackage(engine, dependency, dependencies, "dependencies"); - } - if (packageJson.containsKey("devDependencies")) { - final JsonObject dependencies = packageJson.getJsonObject("devDependencies"); - processPackage(engine, dependency, dependencies, "devDependencies"); - } - if (packageJson.containsKey("optionalDependencies")) { - final JsonObject dependencies = packageJson.getJsonObject("optionalDependencies"); - processPackage(engine, dependency, dependencies, "optionalDependencies"); - } - if (packageJson.containsKey("peerDependencies")) { - final JsonObject dependencies = packageJson.getJsonObject("peerDependencies"); - processPackage(engine, dependency, dependencies, "peerDependencies"); - } - if (packageJson.containsKey("bundleDependencies")) { - final JsonArray dependencies = packageJson.getJsonArray("bundleDependencies"); - processPackage(engine, dependency, dependencies, "bundleDependencies"); - } - if (packageJson.containsKey("bundledDependencies")) { - final JsonArray dependencies = packageJson.getJsonArray("bundledDependencies"); - processPackage(engine, dependency, dependencies, "bundledDependencies"); - } - // Create a sanitized version of the package.json final JsonObject sanitizedJson = SanitizePackage.sanitize(packageJson); @@ -228,7 +195,8 @@ public class NspAnalyzer extends AbstractFileTypeAnalyzer { * Create a single vulnerable software object - these do not use CPEs unlike the NVD. */ final VulnerableSoftware vs = new VulnerableSoftware(); - //TODO consider changing this to available versions on the dependency + //TODO consider changing this to available versions on the dependency + // - the update is a part of the version, not versions to update to //vs.setUpdate(advisory.getPatchedVersions()); vs.setName(advisory.getModule() + ":" + advisory.getVulnerableVersions()); @@ -254,160 +222,4 @@ public class NspAnalyzer extends AbstractFileTypeAnalyzer { throw new AnalysisException(String.format("Failed to parse %s file.", file.getPath()), e); } } - - /** - * Construct a dependency object. - * - * @param dependency the parent dependency - * @param name the name of the dependency to create - * @param version the version of the dependency to create - * @param scope the scope of the dependency being created - * @return the generated dependency - */ - private Dependency createDependency(Dependency dependency, String name, String version, String scope) { - final Dependency nodeModule = new Dependency(new File(dependency.getActualFile() + "?" + name), true); - nodeModule.setEcosystem(DEPENDENCY_ECOSYSTEM); - //this is virtual - the sha1 is purely for the hyperlink in the final html report - nodeModule.setSha1sum(Checksum.getSHA1Checksum(String.format("%s:%s", name, version))); - nodeModule.setMd5sum(Checksum.getMD5Checksum(String.format("%s:%s", name, version))); - nodeModule.addEvidence(EvidenceType.PRODUCT, "package.json", "name", name, Confidence.HIGHEST); - nodeModule.addEvidence(EvidenceType.VENDOR, "package.json", "name", name, Confidence.HIGH); - nodeModule.addEvidence(EvidenceType.VERSION, "package.json", "version", version, Confidence.HIGHEST); - nodeModule.addProjectReference(dependency.getName() + ": " + scope); - nodeModule.setName(name); - nodeModule.setVersion(version); - nodeModule.addIdentifier("npm", String.format("%s:%s", name, version), null, Confidence.HIGHEST); - return nodeModule; - } - - /** - * Processes a part of package.json (as defined by JsonArray) and update the - * specified dependency with relevant info. - * - * @param engine the dependency-check engine - * @param dependency the Dependency to update - * @param jsonArray the jsonArray to parse - * @param depType the dependency type - */ - private void processPackage(Engine engine, Dependency dependency, JsonArray jsonArray, String depType) { - final JsonObjectBuilder builder = Json.createObjectBuilder(); - for (JsonString str : jsonArray.getValuesAs(JsonString.class)) { - builder.add(str.toString(), ""); - } - final JsonObject jsonObject = builder.build(); - processPackage(engine, dependency, jsonObject, depType); - } - - /** - * Processes a part of package.json (as defined by JsonObject) and update - * the specified dependency with relevant info. - * - * @param engine the dependency-check engine - * @param dependency the Dependency to update - * @param jsonObject the jsonObject to parse - * @param depType the dependency type - */ - private void processPackage(Engine engine, Dependency dependency, JsonObject jsonObject, String depType) { - for (int i = 0; i < jsonObject.size(); i++) { - for (Map.Entry entry : jsonObject.entrySet()) { - - final String name = entry.getKey(); - String version = ""; - if (entry.getValue() != null && entry.getValue().getValueType() == JsonValue.ValueType.STRING) { - version = ((JsonString) entry.getValue()).getString(); - } - final Dependency existing = findDependency(engine, name, version); - if (existing == null) { - final Dependency nodeModule = createDependency(dependency, name, version, depType); - engine.addDependency(nodeModule); - } else { - existing.addProjectReference(dependency.getName() + ": " + depType); - } - } - } - } - - /** - * Adds information to an evidence collection from the node json - * configuration. - * - * @param dep the dependency to which the evidence will be added - * @param type the type of evidence to be added - * @param json information from node.js - * @param key the key to obtain the data from the json information - */ - private void addToEvidence(Dependency dep, EvidenceType type, JsonObject json, String key) { - if (json.containsKey(key)) { - final JsonValue value = json.get(key); - if (value instanceof JsonString) { - dep.addEvidence(type, PACKAGE_JSON, key, ((JsonString) value).getString(), Confidence.HIGHEST); - } else if (value instanceof JsonObject) { - final JsonObject jsonObject = (JsonObject) value; - for (final Map.Entry entry : jsonObject.entrySet()) { - final String property = entry.getKey(); - final JsonValue subValue = entry.getValue(); - if (subValue instanceof JsonString) { - dep.addEvidence(type, PACKAGE_JSON, - String.format("%s.%s", key, property), - ((JsonString) subValue).getString(), - Confidence.HIGHEST); - } else { - LOGGER.warn("JSON sub-value not string as expected: {}", subValue); - } - } - } else { - LOGGER.warn("JSON value not string or JSON object as expected: {}", value); - } - } - } - - /** - * Locates the dependency from the list of dependencies that have been - * scanned by the engine. - * - * @param engine the dependency-check engine - * @param name the name of the dependency to find - * @param version the version of the dependency to find - * @return the identified dependency; otherwise null - */ - private Dependency findDependency(Engine engine, String name, String version) { - for (Dependency d : engine.getDependencies()) { - if (DEPENDENCY_ECOSYSTEM.equals(d.getEcosystem()) && name.equals(d.getName()) && version != null && d.getVersion() != null) { - String dependencyVersion = d.getVersion(); - if (dependencyVersion.startsWith("^") || dependencyVersion.startsWith("~")) { - dependencyVersion = dependencyVersion.substring(1); - } - - if (version.equals(dependencyVersion)) { - return d; - } - if (version.startsWith("^") || version.startsWith("~") || version.contains("*")) { - String type; - String tmp; - if (version.startsWith("^") || version.startsWith("~")) { - type = version.substring(0, 1); - tmp = version.substring(1); - } else { - type = "*"; - tmp = version; - } - final String[] v = tmp.split(" ")[0].split("\\."); - final String[] depVersion = dependencyVersion.split("\\."); - - if ("^".equals(type) && v[0].equals(depVersion[0])) { - return d; - } else if ("~".equals(type) && v.length >= 2 && depVersion.length >= 2 - && v[0].equals(depVersion[0]) && v[1].equals(depVersion[1])) { - return d; - } else if (v[0].equals("*") - || (v.length >= 2 && v[0].equals(depVersion[0]) && v[1].equals("*")) - || (v.length >= 3 && depVersion.length >= 2 && v[0].equals(depVersion[0]) - && v[1].equals(depVersion[1]) && v[2].equals("*"))) { - return d; - } - } - } - } - return null; - } } 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 21059dfe4..0962f9ddc 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,8 +17,6 @@ */ package org.owasp.dependencycheck.analyzer; -import java.util.ArrayList; -import java.util.Arrays; import java.util.List; import javax.annotation.concurrent.ThreadSafe; import org.owasp.dependencycheck.Engine; @@ -29,7 +27,6 @@ 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 @@ -44,31 +41,7 @@ public class NvdCveAnalyzer extends AbstractAnalyzer { /** * The Logger for use throughout the class */ - private static final org.slf4j.Logger LOGGER = LoggerFactory.getLogger(NvdCveAnalyzer.class); - /** - * The list of ecosystems to skip during analysis. These are skipped because - * there is generally a more accurate vulnerability analyzer in the - * pipeline. - */ - 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); - } - } - + //private static final org.slf4j.Logger LOGGER = LoggerFactory.getLogger(NvdCveAnalyzer.class); /** * Analyzes a dependency and attempts to determine if there are any CPE * identifiers for this dependency. @@ -80,10 +53,6 @@ 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())) { @@ -139,4 +108,4 @@ public class NvdCveAnalyzer extends AbstractAnalyzer { protected String getAnalyzerEnabledSettingKey() { return Settings.KEYS.ANALYZER_NVD_CVE_ENABLED; } -} +} \ No newline at end of file From f1631e9ff33893d65f0acd4bd4c978808223e052 Mon Sep 17 00:00:00 2001 From: Jeremy Long Date: Sun, 26 Nov 2017 09:03:39 -0500 Subject: [PATCH 13/22] cleanup of code and added warning messages --- .../analyzer/NodePackageAnalyzer.java | 54 +++++-------------- .../dependencycheck/analyzer/NspAnalyzer.java | 12 +++++ 2 files changed, 26 insertions(+), 40 deletions(-) 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 609bed1f6..ac898b9ee 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 @@ -165,33 +165,39 @@ public class NodePackageAnalyzer extends AbstractNpmAnalyzer { @Override protected void analyzeDependency(Dependency dependency, Engine engine) throws AnalysisException { engine.removeDependency(dependency); - File file = dependency.getActualFile(); - if (!file.isFile() || file.length() == 0) { + File dependencyFile = dependency.getActualFile(); + if (!dependencyFile.isFile() || dependencyFile.length() == 0) { return; } try { // Do not scan the node_modules directory - if (file.getCanonicalPath().contains(File.separator + "node_modules" + File.separator)) { - LOGGER.debug("Skipping analysis of node module: " + file.getCanonicalPath()); + if (dependencyFile.getCanonicalPath().contains(File.separator + "node_modules" + File.separator)) { + LOGGER.debug("Skipping analysis of node module: " + dependencyFile.getCanonicalPath()); return; } } catch (IOException ex) { throw new RuntimeException(ex); } - File baseDir = file.getParentFile(); + File baseDir = dependencyFile.getParentFile(); if (PACKAGE_LOCK_JSON.equals(dependency.getFileName())) { File shrinkwrap = new File(baseDir, SHRINKWRAP_JSON); if (shrinkwrap.exists()) { return; } } + final File nodeModules = new File(baseDir, "node_modules"); + if (!nodeModules.isDirectory()) { + LOGGER.warn("Analyzing `{}` - however, the node_modules directory does not exist. " + + "Please run `npm install` prior to running dependency-check", dependencyFile.toString()); + return; + } - try (JsonReader jsonReader = Json.createReader(FileUtils.openInputStream(file))) { + try (JsonReader jsonReader = Json.createReader(FileUtils.openInputStream(dependencyFile))) { final JsonObject json = jsonReader.readObject(); final String parentName = json.getString("name"); final String parentVersion = json.getString("version"); final String parentPackage = String.format("%s:%s", parentName, parentVersion); - processDependencies(json, baseDir, file, parentPackage, engine); + processDependencies(json, baseDir, dependencyFile, parentPackage, engine); } catch (JsonException e) { LOGGER.warn("Failed to parse package.json file.", e); } catch (IOException e) { @@ -228,7 +234,7 @@ public class NodePackageAnalyzer extends AbstractNpmAnalyzer { throw new AnalysisException("Problem occurred while reading dependency file.", e); } } else { - LOGGER.error("Unable to find child file {}", f.toString()); + LOGGER.warn("Unable to find node module: {}", f.toString()); child = new Dependency(rootFile, true); //TOOD - we should use the integrity value instead of calculating the SHA1/MD5 child.setSha1sum(Checksum.getSHA1Checksum(String.format("%s:%s", name, version))); @@ -256,37 +262,5 @@ public class NodePackageAnalyzer extends AbstractNpmAnalyzer { } } } - -// gatherEvidence(json, dependency); -// -// // only run this if we are in evidence collection or the NSP analyzer has been disabled -// if (engine.getMode() == Mode.EVIDENCE_COLLECTION -// || !engine.getSettings().getBoolean(Settings.KEYS.ANALYZER_NSP_PACKAGE_ENABLED)) { -// //Processes the dependencies objects in package.json and adds all the modules as dependencies -// if (json.containsKey("dependencies")) { -// final JsonObject dependencies = json.getJsonObject("dependencies"); -// processPackage(engine, dependency, dependencies, "dependencies"); -// } -// if (json.containsKey("devDependencies")) { -// final JsonObject dependencies = json.getJsonObject("devDependencies"); -// processPackage(engine, dependency, dependencies, "devDependencies"); -// } -// if (json.containsKey("optionalDependencies")) { -// final JsonObject dependencies = json.getJsonObject("optionalDependencies"); -// processPackage(engine, dependency, dependencies, "optionalDependencies"); -// } -// if (json.containsKey("peerDependencies")) { -// final JsonObject dependencies = json.getJsonObject("peerDependencies"); -// processPackage(engine, dependency, dependencies, "peerDependencies"); -// } -// if (json.containsKey("bundleDependencies")) { -// final JsonArray dependencies = json.getJsonArray("bundleDependencies"); -// processPackage(engine, dependency, dependencies, "bundleDependencies"); -// } -// if (json.containsKey("bundledDependencies")) { -// final JsonArray dependencies = json.getJsonArray("bundledDependencies"); -// processPackage(engine, dependency, dependencies, "bundledDependencies"); -// } -// } } } 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 088df25fb..f6de7f441 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 @@ -43,7 +43,9 @@ import javax.json.JsonException; import javax.json.JsonObject; import javax.json.JsonObjectBuilder; import javax.json.JsonReader; +import static org.owasp.dependencycheck.analyzer.NodePackageAnalyzer.DEPENDENCY_ECOSYSTEM; import org.owasp.dependencycheck.exception.InitializationException; +import org.owasp.dependencycheck.utils.InvalidSettingException; import org.owasp.dependencycheck.utils.URLConnectionFailureException; /** @@ -110,6 +112,16 @@ public class NspAnalyzer extends AbstractNpmAnalyzer { setEnabled(false); throw new InitializationException("The configured URL to Node Security Platform is malformed", ex); } + try { + final Settings settings = engine.getSettings(); + final boolean nodeEnabled = settings.getBoolean(Settings.KEYS.ANALYZER_NODE_PACKAGE_ENABLED); + if (!nodeEnabled) { + LOGGER.warn("The Node Package Analyzer has been disabled; the resulting report will only " + + " contain the known vulnerable dependency - not a bill of materials for the node project."); + } + } catch (InvalidSettingException ex) { + throw new InitializationException("Unable to read configuration settings", ex); + } } /** From 93f25abd994c5d8b5440bb4745fc31c27cc2f8a2 Mon Sep 17 00:00:00 2001 From: Jeremy Long Date: Sun, 26 Nov 2017 09:05:42 -0500 Subject: [PATCH 14/22] checkstyle suggestions --- .../analyzer/AbstractNpmAnalyzer.java | 2 +- .../analyzer/DependencyBundlingAnalyzer.java | 14 +++++++-- .../analyzer/NodePackageAnalyzer.java | 30 +++++++++++++------ .../dependencycheck/analyzer/NspAnalyzer.java | 1 - .../analyzer/NvdCveAnalyzer.java | 2 +- 5 files changed, 34 insertions(+), 15 deletions(-) diff --git a/dependency-check-core/src/main/java/org/owasp/dependencycheck/analyzer/AbstractNpmAnalyzer.java b/dependency-check-core/src/main/java/org/owasp/dependencycheck/analyzer/AbstractNpmAnalyzer.java index e61ea8106..671a13147 100644 --- a/dependency-check-core/src/main/java/org/owasp/dependencycheck/analyzer/AbstractNpmAnalyzer.java +++ b/dependency-check-core/src/main/java/org/owasp/dependencycheck/analyzer/AbstractNpmAnalyzer.java @@ -207,7 +207,7 @@ public abstract class AbstractNpmAnalyzer extends AbstractFileTypeAnalyzer { protected Dependency findDependency(Engine engine, String name, String version) { for (Dependency d : engine.getDependencies()) { if (NPM_DEPENDENCY_ECOSYSTEM.equals(d.getEcosystem()) && name.equals(d.getName()) && version != null && d.getVersion() != null) { - String dependencyVersion = d.getVersion(); + final String dependencyVersion = d.getVersion(); if (DependencyBundlingAnalyzer.npmVersionsMatch(version, dependencyVersion)) { return d; } diff --git a/dependency-check-core/src/main/java/org/owasp/dependencycheck/analyzer/DependencyBundlingAnalyzer.java b/dependency-check-core/src/main/java/org/owasp/dependencycheck/analyzer/DependencyBundlingAnalyzer.java index fce847893..c6cceee9d 100644 --- a/dependency-check-core/src/main/java/org/owasp/dependencycheck/analyzer/DependencyBundlingAnalyzer.java +++ b/dependency-check-core/src/main/java/org/owasp/dependencycheck/analyzer/DependencyBundlingAnalyzer.java @@ -162,7 +162,8 @@ public class DependencyBundlingAnalyzer extends AbstractDependencyComparingAnaly * removed from the main analysis loop, this function adds to this * collection */ - public static void mergeDependencies(final Dependency dependency, final Dependency relatedDependency, final Set dependenciesToRemove) { + public static void mergeDependencies(final Dependency dependency, + final Dependency relatedDependency, final Set dependenciesToRemove) { dependency.addRelatedDependency(relatedDependency); for (Dependency d : relatedDependency.getRelatedDependencies()) { dependency.addRelatedDependency(d); @@ -496,7 +497,7 @@ public class DependencyBundlingAnalyzer extends AbstractDependencyComparingAnaly * This method attempts to evaluate version range checks. * * @param current a dependency version to compare - * @param nextDependency a dependency version to compare + * @param next a dependency version to compare * @return true if the version is equal in both dependencies; otherwise * false */ @@ -520,7 +521,7 @@ public class DependencyBundlingAnalyzer extends AbstractDependencyComparingAnaly } } try { - Semver v = new Semver(right, SemverType.NPM); + final Semver v = new Semver(right, SemverType.NPM); return v.satisfies(left); } catch (SemverException ex) { LOGGER.trace("ignore", ex); @@ -552,6 +553,13 @@ public class DependencyBundlingAnalyzer extends AbstractDependencyComparingAnaly return false; } + /** + * Strips leading non-numeric values from the start of the string. If no + * numbers are present this will return null. + * + * @param str the string to modify + * @return the string without leading non-numeric characters + */ private static String stripLeadingNonNumeric(String str) { for (int x = 0; x < str.length(); x++) { if (Character.isDigit(str.codePointAt(x))) { 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 ac898b9ee..9f7e2d9cf 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 @@ -165,7 +165,7 @@ public class NodePackageAnalyzer extends AbstractNpmAnalyzer { @Override protected void analyzeDependency(Dependency dependency, Engine engine) throws AnalysisException { engine.removeDependency(dependency); - File dependencyFile = dependency.getActualFile(); + final File dependencyFile = dependency.getActualFile(); if (!dependencyFile.isFile() || dependencyFile.length() == 0) { return; } @@ -178,9 +178,9 @@ public class NodePackageAnalyzer extends AbstractNpmAnalyzer { } catch (IOException ex) { throw new RuntimeException(ex); } - File baseDir = dependencyFile.getParentFile(); + final File baseDir = dependencyFile.getParentFile(); if (PACKAGE_LOCK_JSON.equals(dependency.getFileName())) { - File shrinkwrap = new File(baseDir, SHRINKWRAP_JSON); + final File shrinkwrap = new File(baseDir, SHRINKWRAP_JSON); if (shrinkwrap.exists()) { return; } @@ -205,15 +205,27 @@ public class NodePackageAnalyzer extends AbstractNpmAnalyzer { } } + /** + * Process the dependencies in the lock file by first parsing its + * dependencies and then finding the package.json for the module and adding + * it as a dependency. + * + * @param json + * @param baseDir + * @param rootFile + * @param parentPackage + * @param engine + * @throws AnalysisException + */ private void processDependencies(final JsonObject json, File baseDir, File rootFile, final String parentPackage, Engine engine) throws AnalysisException { if (json.containsKey("dependencies")) { - JsonObject deps = json.getJsonObject("dependencies"); + final JsonObject deps = json.getJsonObject("dependencies"); for (Map.Entry entry : deps.entrySet()) { - JsonObject jo = (JsonObject) entry.getValue(); + final JsonObject jo = (JsonObject) entry.getValue(); final String name = entry.getKey(); final String version = jo.getString("version"); - File base = Paths.get(baseDir.getPath(), "node_modules", name).toFile(); - File f = new File(base, PACKAGE_JSON); + final File base = Paths.get(baseDir.getPath(), "node_modules", name).toFile(); + final File f = new File(base, PACKAGE_JSON); if (jo.containsKey("dependencies")) { final String subPackageName = String.format("%s/%s:%s", parentPackage, name, version); @@ -225,7 +237,7 @@ public class NodePackageAnalyzer extends AbstractNpmAnalyzer { //TOOD - we should use the integrity value instead of calculating the SHA1/MD5 child = new Dependency(f); try (JsonReader jr = Json.createReader(FileUtils.openInputStream(f))) { - JsonObject childJson = jr.readObject(); + final JsonObject childJson = jr.readObject(); gatherEvidence(childJson, child); } catch (JsonException e) { @@ -248,7 +260,7 @@ public class NodePackageAnalyzer extends AbstractNpmAnalyzer { child.addProjectReference(parentPackage); child.setEcosystem(DEPENDENCY_ECOSYSTEM); - Dependency existing = findDependency(engine, name, version); + final Dependency existing = findDependency(engine, name, version); if (existing != null) { if (existing.isVirtual()) { DependencyMergingAnalyzer.mergeDependencies(child, existing, null); 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 f6de7f441..0a9ae2cf4 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 @@ -43,7 +43,6 @@ import javax.json.JsonException; import javax.json.JsonObject; import javax.json.JsonObjectBuilder; import javax.json.JsonReader; -import static org.owasp.dependencycheck.analyzer.NodePackageAnalyzer.DEPENDENCY_ECOSYSTEM; import org.owasp.dependencycheck.exception.InitializationException; import org.owasp.dependencycheck.utils.InvalidSettingException; import org.owasp.dependencycheck.utils.URLConnectionFailureException; 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 0962f9ddc..6a6e82060 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 @@ -108,4 +108,4 @@ public class NvdCveAnalyzer extends AbstractAnalyzer { protected String getAnalyzerEnabledSettingKey() { return Settings.KEYS.ANALYZER_NVD_CVE_ENABLED; } -} \ No newline at end of file +} From 0e3fa6645d526842e6a3de64b6efc9456ddf7042 Mon Sep 17 00:00:00 2001 From: Jeremy Long Date: Sun, 26 Nov 2017 09:08:36 -0500 Subject: [PATCH 15/22] due to removing the retired attribute from NodePackageAnalyzer we should increment the minor version --- build-reporting/pom.xml | 2 +- dependency-check-ant/pom.xml | 2 +- dependency-check-cli/pom.xml | 2 +- dependency-check-core/pom.xml | 2 +- dependency-check-maven/pom.xml | 2 +- dependency-check-plugin/pom.xml | 2 +- dependency-check-utils/pom.xml | 2 +- pom.xml | 2 +- 8 files changed, 8 insertions(+), 8 deletions(-) diff --git a/build-reporting/pom.xml b/build-reporting/pom.xml index 7d869e274..e5705b3f5 100644 --- a/build-reporting/pom.xml +++ b/build-reporting/pom.xml @@ -20,7 +20,7 @@ Copyright (c) 2017 - Jeremy Long. All Rights Reserved. org.owasp dependency-check-parent - 3.0.3-SNAPSHOT + 3.1.0-SNAPSHOT Dependency-Check Build-Reporting build-reporting diff --git a/dependency-check-ant/pom.xml b/dependency-check-ant/pom.xml index 539ddedcf..eb46c51ed 100644 --- a/dependency-check-ant/pom.xml +++ b/dependency-check-ant/pom.xml @@ -20,7 +20,7 @@ Copyright (c) 2013 - Jeremy Long. All Rights Reserved. org.owasp dependency-check-parent - 3.0.3-SNAPSHOT + 3.1.0-SNAPSHOT dependency-check-ant diff --git a/dependency-check-cli/pom.xml b/dependency-check-cli/pom.xml index e7b89ce9d..869fbff83 100644 --- a/dependency-check-cli/pom.xml +++ b/dependency-check-cli/pom.xml @@ -20,7 +20,7 @@ Copyright (c) 2012 - Jeremy Long. All Rights Reserved. org.owasp dependency-check-parent - 3.0.3-SNAPSHOT + 3.1.0-SNAPSHOT dependency-check-cli diff --git a/dependency-check-core/pom.xml b/dependency-check-core/pom.xml index fb93e391f..e56a04adb 100644 --- a/dependency-check-core/pom.xml +++ b/dependency-check-core/pom.xml @@ -20,7 +20,7 @@ Copyright (c) 2012 Jeremy Long. All Rights Reserved. org.owasp dependency-check-parent - 3.0.3-SNAPSHOT + 3.1.0-SNAPSHOT dependency-check-core diff --git a/dependency-check-maven/pom.xml b/dependency-check-maven/pom.xml index 0a238dcbb..6b8a3024c 100644 --- a/dependency-check-maven/pom.xml +++ b/dependency-check-maven/pom.xml @@ -20,7 +20,7 @@ Copyright (c) 2013 Jeremy Long. All Rights Reserved. org.owasp dependency-check-parent - 3.0.3-SNAPSHOT + 3.1.0-SNAPSHOT dependency-check-maven maven-plugin diff --git a/dependency-check-plugin/pom.xml b/dependency-check-plugin/pom.xml index 828348391..0edee5725 100644 --- a/dependency-check-plugin/pom.xml +++ b/dependency-check-plugin/pom.xml @@ -21,7 +21,7 @@ Copyright (c) 2017 Jeremy Long. All Rights Reserved. org.owasp dependency-check-parent - 3.0.3-SNAPSHOT + 3.1.0-SNAPSHOT org.owasp dependency-check-plugin diff --git a/dependency-check-utils/pom.xml b/dependency-check-utils/pom.xml index 15ed50461..ce4225b80 100644 --- a/dependency-check-utils/pom.xml +++ b/dependency-check-utils/pom.xml @@ -20,7 +20,7 @@ Copyright (c) 2014 - Jeremy Long. All Rights Reserved. org.owasp dependency-check-parent - 3.0.3-SNAPSHOT + 3.1.0-SNAPSHOT dependency-check-utils diff --git a/pom.xml b/pom.xml index b2410c6ac..ee1b189e0 100644 --- a/pom.xml +++ b/pom.xml @@ -20,7 +20,7 @@ Copyright (c) 2012 - Jeremy Long org.owasp dependency-check-parent - 3.0.3-SNAPSHOT + 3.1.0-SNAPSHOT pom From eb023c0c994c6470d9e78d2d0f2393ea86392533 Mon Sep 17 00:00:00 2001 From: Jeremy Long Date: Sun, 26 Nov 2017 10:05:50 -0500 Subject: [PATCH 16/22] updated to better support npm --- .../src/main/resources/templates/htmlReport.vsl | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/dependency-check-core/src/main/resources/templates/htmlReport.vsl b/dependency-check-core/src/main/resources/templates/htmlReport.vsl index 08170c044..5d3ec38b6 100644 --- a/dependency-check-core/src/main/resources/templates/htmlReport.vsl +++ b/dependency-check-core/src/main/resources/templates/htmlReport.vsl @@ -623,7 +623,7 @@ Getting Help: Dependency CPE - GAV + Coordinates Highest Severity CVE Count CPE Confidence @@ -638,7 +638,7 @@ Getting Help: #end #if ($id.type=="npm") -
  • $enc.html($id.value): $enc.html($id.description)
    • +
  • $enc.html($id.value)
    • #end #end From f51edf52e797903c49b21c7fbfaa2f7eff3ead86 Mon Sep 17 00:00:00 2001 From: Jeremy Long Date: Sun, 26 Nov 2017 10:13:32 -0500 Subject: [PATCH 17/22] updates for issue #991 --- .../dependencycheck/reporting/EscapeTool.java | 13 +++++----- .../main/resources/templates/csvReport.vsl | 2 +- .../main/resources/templates/jsonReport.vsl | 13 +++------- .../reporting/EscapeToolTest.java | 26 +++++++++---------- 4 files changed, 23 insertions(+), 31 deletions(-) diff --git a/dependency-check-core/src/main/java/org/owasp/dependencycheck/reporting/EscapeTool.java b/dependency-check-core/src/main/java/org/owasp/dependencycheck/reporting/EscapeTool.java index f2e544be0..612385eb6 100644 --- a/dependency-check-core/src/main/java/org/owasp/dependencycheck/reporting/EscapeTool.java +++ b/dependency-check-core/src/main/java/org/owasp/dependencycheck/reporting/EscapeTool.java @@ -123,7 +123,7 @@ public class EscapeTool { */ public String csv(String text) { if (text == null || text.isEmpty()) { - return text; + return "\"\""; } return StringEscapeUtils.escapeCsv(text.trim().replace("\n", " ")); } @@ -137,7 +137,7 @@ public class EscapeTool { */ public String csvIdentifiers(Set ids) { if (ids == null || ids.isEmpty()) { - return ""; + return "\"\""; } boolean addComma = false; final StringBuilder sb = new StringBuilder(); @@ -163,7 +163,7 @@ public class EscapeTool { */ public String csvCpe(Set ids) { if (ids == null || ids.isEmpty()) { - return ""; + return "\"\""; } boolean addComma = false; final StringBuilder sb = new StringBuilder(); @@ -189,7 +189,7 @@ public class EscapeTool { */ public String csvCpeConfidence(Set ids) { if (ids == null || ids.isEmpty()) { - return ""; + return "\"\""; } boolean addComma = false; final StringBuilder sb = new StringBuilder(); @@ -215,12 +215,12 @@ public class EscapeTool { */ public String csvGav(Set ids) { if (ids == null || ids.isEmpty()) { - return ""; + return "\"\""; } boolean addComma = false; final StringBuilder sb = new StringBuilder(); for (Identifier id : ids) { - if ("maven".equals(id.getType())) { + if ("maven".equals(id.getType()) || "npm".equals(id.getType())) { if (addComma) { sb.append(", "); } else { @@ -231,5 +231,4 @@ public class EscapeTool { } return StringEscapeUtils.escapeCsv(sb.toString()); } - } diff --git a/dependency-check-core/src/main/resources/templates/csvReport.vsl b/dependency-check-core/src/main/resources/templates/csvReport.vsl index 816384cee..18d71503c 100644 --- a/dependency-check-core/src/main/resources/templates/csvReport.vsl +++ b/dependency-check-core/src/main/resources/templates/csvReport.vsl @@ -17,7 +17,7 @@ Copyright (c) 2017 Jeremy Long. All Rights Reserved. @author Jeremy Long @version 1 *### -"Project","ScanDate","DependencyName","DependencyPath","Description","License","Md5","Sha1","Identifiers","CPE","CVE","CWE","Vulnerability","Source","Severity","CVSSv2","GAV","CPE Confidence","Evidence Count" +"Project","ScanDate","DependencyName","DependencyPath","Description","License","Md5","Sha1","Identifiers","CPE","CVE","CWE","Vulnerability","Source","Severity","CVSSv2","Build Coordinates","CPE Confidence","Evidence Count" #macro(writeSev $score)#if($score<4.0)"Low"#elseif($score>=7.0)"High"#else"Medium"#end#end #foreach($dependency in $dependencies)#if($dependency.getVulnerabilities().size()>0) #foreach($vuln in $dependency.getVulnerabilities(true)) diff --git a/dependency-check-core/src/main/resources/templates/jsonReport.vsl b/dependency-check-core/src/main/resources/templates/jsonReport.vsl index decfeaa83..dada8239e 100644 --- a/dependency-check-core/src/main/resources/templates/jsonReport.vsl +++ b/dependency-check-core/src/main/resources/templates/jsonReport.vsl @@ -41,22 +41,15 @@ "identifiers": [ #set($loopCount=0) #foreach($id in $related.getIdentifiers()) - #if ($id.type=="maven") + #if ($id.type=="maven" || $id.type=="npm") #set($loopCount=$loopCount+1) #if($loopCount>1),#end { "type": "$enc.json($id.type)", - "name": "$id.value" + "id": "$id.value" #if ($id.url),"url": "$enc.json($id.url)"#end #if ($id.notes),"notes": "$enc.json($id.notes)"#end - } - #end - #if ($id.type=="npm") - #set($loopCount=$loopCount+1) - #if($loopCount>1),#end - { - "id":"$enc.json($id.value)" - ,"description":"$enc.json($id.description)" + #if ($id.description),"description":"$enc.json($id.description)"#end } #end #end diff --git a/dependency-check-core/src/test/java/org/owasp/dependencycheck/reporting/EscapeToolTest.java b/dependency-check-core/src/test/java/org/owasp/dependencycheck/reporting/EscapeToolTest.java index 6890d2469..b9b730e5f 100644 --- a/dependency-check-core/src/test/java/org/owasp/dependencycheck/reporting/EscapeToolTest.java +++ b/dependency-check-core/src/test/java/org/owasp/dependencycheck/reporting/EscapeToolTest.java @@ -131,7 +131,7 @@ public class EscapeToolTest { assertEquals(expResult, result); text = ""; - expResult = ""; + expResult = "\"\""; result = instance.csv(text); assertEquals(expResult, result); @@ -148,18 +148,18 @@ public class EscapeToolTest { public void testCsvIdentifiers() { EscapeTool instance = new EscapeTool(); Set ids = null; - String expResult = ""; + String expResult = "\"\""; String result = instance.csvIdentifiers(ids); assertEquals(expResult, result); ids = new HashSet<>(); - expResult = ""; + expResult = "\"\""; result = instance.csvIdentifiers(ids); assertEquals(expResult, result); ids = new HashSet<>(); ids.add(new Identifier("cpe", "cpe:/a:somegroup:something:1.0", "")); - expResult = ""; + expResult = "\"\""; result = instance.csvIdentifiers(ids); assertEquals(expResult, result); @@ -193,18 +193,18 @@ public class EscapeToolTest { public void testCsvCpe() { EscapeTool instance = new EscapeTool(); Set ids = null; - String expResult = ""; + String expResult = "\"\""; String result = instance.csvCpe(ids); assertEquals(expResult, result); ids = new HashSet<>(); - expResult = ""; + expResult = "\"\""; result = instance.csvCpe(ids); assertEquals(expResult, result); ids = new HashSet<>(); ids.add(new Identifier("gav", "somegroup:something:1.0", "")); - expResult = ""; + expResult = "\"\""; result = instance.csvCpe(ids); assertEquals(expResult, result); @@ -238,18 +238,18 @@ public class EscapeToolTest { public void testCsvCpeConfidence() { EscapeTool instance = new EscapeTool(); Set ids = null; - String expResult = ""; + String expResult = "\"\""; String result = instance.csvCpeConfidence(ids); assertEquals(expResult, result); ids = new HashSet<>(); - expResult = ""; + expResult = "\"\""; result = instance.csvCpeConfidence(ids); assertEquals(expResult, result); ids = new HashSet<>(); ids.add(new Identifier("gav", "somegroup:something:1.0", "")); - expResult = ""; + expResult = "\"\""; result = instance.csvCpeConfidence(ids); assertEquals(expResult, result); @@ -285,18 +285,18 @@ public class EscapeToolTest { public void testCsvGav() { EscapeTool instance = new EscapeTool(); Set ids = null; - String expResult = ""; + String expResult = "\"\""; String result = instance.csvGav(ids); assertEquals(expResult, result); ids = new HashSet<>(); - expResult = ""; + expResult = "\"\""; result = instance.csvGav(ids); assertEquals(expResult, result); ids = new HashSet<>(); ids.add(new Identifier("cpe", "somegroup:something:1.0", "")); - expResult = ""; + expResult = "\"\""; result = instance.csvGav(ids); assertEquals(expResult, result); From 72c121797fef38b223e4db48926fd37b392da11f Mon Sep 17 00:00:00 2001 From: Jeremy Long Date: Sun, 26 Nov 2017 10:26:37 -0500 Subject: [PATCH 18/22] fixed test cases --- .../dependencycheck/reporting/EscapeTool.java | 18 ++++++++++++++++- .../reporting/EscapeToolTest.java | 20 +++++++++---------- 2 files changed, 27 insertions(+), 11 deletions(-) diff --git a/dependency-check-core/src/main/java/org/owasp/dependencycheck/reporting/EscapeTool.java b/dependency-check-core/src/main/java/org/owasp/dependencycheck/reporting/EscapeTool.java index 612385eb6..e0829d0da 100644 --- a/dependency-check-core/src/main/java/org/owasp/dependencycheck/reporting/EscapeTool.java +++ b/dependency-check-core/src/main/java/org/owasp/dependencycheck/reporting/EscapeTool.java @@ -125,7 +125,11 @@ public class EscapeTool { if (text == null || text.isEmpty()) { return "\"\""; } - return StringEscapeUtils.escapeCsv(text.trim().replace("\n", " ")); + final String str = text.trim().replace("\n", " "); + if (str.length()==0) { + return "\"\""; + } + return StringEscapeUtils.escapeCsv(str); } /** @@ -151,6 +155,9 @@ public class EscapeTool { sb.append(id.getValue()); } } + if (sb.length()==0) { + return "\"\""; + } return StringEscapeUtils.escapeCsv(sb.toString()); } @@ -177,6 +184,9 @@ public class EscapeTool { sb.append(id.getValue()); } } + if (sb.length()==0) { + return "\"\""; + } return StringEscapeUtils.escapeCsv(sb.toString()); } @@ -203,6 +213,9 @@ public class EscapeTool { sb.append(id.getConfidence()); } } + if (sb.length()==0) { + return "\"\""; + } return StringEscapeUtils.escapeCsv(sb.toString()); } @@ -229,6 +242,9 @@ public class EscapeTool { sb.append(id.getValue()); } } + if (sb.length()==0) { + return "\"\""; + } return StringEscapeUtils.escapeCsv(sb.toString()); } } diff --git a/dependency-check-core/src/test/java/org/owasp/dependencycheck/reporting/EscapeToolTest.java b/dependency-check-core/src/test/java/org/owasp/dependencycheck/reporting/EscapeToolTest.java index b9b730e5f..a94180c11 100644 --- a/dependency-check-core/src/test/java/org/owasp/dependencycheck/reporting/EscapeToolTest.java +++ b/dependency-check-core/src/test/java/org/owasp/dependencycheck/reporting/EscapeToolTest.java @@ -126,7 +126,7 @@ public class EscapeToolTest { public void testCsv() { String text = null; EscapeTool instance = new EscapeTool(); - String expResult = null; + String expResult = "\"\""; String result = instance.csv(text); assertEquals(expResult, result); @@ -164,22 +164,22 @@ public class EscapeToolTest { assertEquals(expResult, result); ids = new HashSet<>(); - ids.add(new Identifier("gav", "somegroup:something:1.0", "")); + ids.add(new Identifier("maven", "somegroup:something:1.0", "")); expResult = "somegroup:something:1.0"; result = instance.csvIdentifiers(ids); assertEquals(expResult, result); ids = new HashSet<>(); ids.add(new Identifier("cpe", "cpe:/a:somegroup:something:1.0", "")); - ids.add(new Identifier("gav", "somegroup:something:1.0", "")); + ids.add(new Identifier("maven", "somegroup:something:1.0", "")); expResult = "somegroup:something:1.0"; result = instance.csvIdentifiers(ids); assertEquals(expResult, result); ids = new HashSet<>(); ids.add(new Identifier("cpe", "cpe:/a:somegroup:something:1.0", "")); - ids.add(new Identifier("gav", "somegroup:something:1.0", "")); - ids.add(new Identifier("gav", "somegroup2:something:1.2", "")); + ids.add(new Identifier("maven", "somegroup:something:1.0", "")); + ids.add(new Identifier("maven", "somegroup2:something:1.2", "")); expResult = "\"somegroup:something:1.0, somegroup2:something:1.2\""; String expResult2 = "\"somegroup2:something:1.2, somegroup:something:1.0\""; result = instance.csvIdentifiers(ids); @@ -203,7 +203,7 @@ public class EscapeToolTest { assertEquals(expResult, result); ids = new HashSet<>(); - ids.add(new Identifier("gav", "somegroup:something:1.0", "")); + ids.add(new Identifier("maven", "somegroup:something:1.0", "")); expResult = "\"\""; result = instance.csvCpe(ids); assertEquals(expResult, result); @@ -216,14 +216,14 @@ public class EscapeToolTest { ids = new HashSet<>(); ids.add(new Identifier("cpe", "cpe:/a:somegroup:something:1.0", "")); - ids.add(new Identifier("gav", "somegroup:something:1.0", "")); + ids.add(new Identifier("maven", "somegroup:something:1.0", "")); expResult = "cpe:/a:somegroup:something:1.0"; result = instance.csvCpe(ids); assertEquals(expResult, result); ids = new HashSet<>(); ids.add(new Identifier("cpe", "cpe:/a:somegroup:something:1.0", "")); - ids.add(new Identifier("gav", "somegroup:something:1.0", "")); + ids.add(new Identifier("maven", "somegroup:something:1.0", "")); ids.add(new Identifier("cpe", "cpe:/a:somegroup2:something:1.2", "")); expResult = "\"cpe:/a:somegroup:something:1.0, cpe:/a:somegroup2:something:1.2\""; String expResult2 = "\"cpe:/a:somegroup2:something:1.2, cpe:/a:somegroup:something:1.0\""; @@ -248,7 +248,7 @@ public class EscapeToolTest { assertEquals(expResult, result); ids = new HashSet<>(); - ids.add(new Identifier("gav", "somegroup:something:1.0", "")); + ids.add(new Identifier("maven", "somegroup:something:1.0", "")); expResult = "\"\""; result = instance.csvCpeConfidence(ids); assertEquals(expResult, result); @@ -268,7 +268,7 @@ public class EscapeToolTest { Identifier i2 = new Identifier("cpe", "cpe:/a:somegroup:something2:1.0", ""); i2.setConfidence(Confidence.MEDIUM); ids.add(i2); - Identifier i3 = new Identifier("gav", "somegroup:something:1.0", ""); + Identifier i3 = new Identifier("maven", "somegroup:something:1.0", ""); i3.setConfidence(Confidence.LOW); ids.add(i3); From c465bc9fc70b89bb44cc1e9c0d9df032c098cc0a Mon Sep 17 00:00:00 2001 From: Jeremy Long Date: Mon, 27 Nov 2017 21:46:33 -0500 Subject: [PATCH 19/22] fixed incorrect parsing of license information --- .../analyzer/AbstractNpmAnalyzer.java | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/dependency-check-core/src/main/java/org/owasp/dependencycheck/analyzer/AbstractNpmAnalyzer.java b/dependency-check-core/src/main/java/org/owasp/dependencycheck/analyzer/AbstractNpmAnalyzer.java index 671a13147..a6d266e71 100644 --- a/dependency-check-core/src/main/java/org/owasp/dependencycheck/analyzer/AbstractNpmAnalyzer.java +++ b/dependency-check-core/src/main/java/org/owasp/dependencycheck/analyzer/AbstractNpmAnalyzer.java @@ -281,6 +281,20 @@ public abstract class AbstractNpmAnalyzer extends AbstractFileTypeAnalyzer { final Object value = json.get("license"); if (value instanceof JsonString) { dependency.setLicense(json.getString("license")); + } else if (value instanceof JsonArray) { + final JsonArray array = (JsonArray) value; + final StringBuilder sb = new StringBuilder(); + boolean addComma = false; + for (int x = 0; x < array.size(); x++) { + if (!array.isNull(x)) { + if (addComma) { + sb.append(", "); + } else { + addComma = true; + } + sb.append(array.getString(x)); + } + } } else { dependency.setLicense(json.getJsonObject("license").getString("type")); } From a7dddfa9050d5ced14fffab2d778d5d139fbf56f Mon Sep 17 00:00:00 2001 From: Jeremy Long Date: Mon, 27 Nov 2017 21:59:50 -0500 Subject: [PATCH 20/22] fixed incorrect name of shrinkwrap.json --- .../owasp/dependencycheck/analyzer/NodePackageAnalyzer.java | 4 ++-- .../dependencycheck/analyzer/NodePackageAnalyzerTest.java | 6 +++--- .../nodejs/{shrinkwrap.json => npm-shrinkwrap.json} | 0 3 files changed, 5 insertions(+), 5 deletions(-) rename dependency-check-core/src/test/resources/nodejs/{shrinkwrap.json => npm-shrinkwrap.json} (100%) 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 9f7e2d9cf..7d6916406 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 @@ -83,9 +83,9 @@ public class NodePackageAnalyzer extends AbstractNpmAnalyzer { /** * The file name to scan. */ - public static final String SHRINKWRAP_JSON = "shrinkwrap.json"; + public static final String SHRINKWRAP_JSON = "npm-shrinkwrap.json"; /** - * Filter that detects files named "package-lock.json" or "shrinkwrap.json". + * Filter that detects files named "package-lock.json" or "npm-shrinkwrap.json". */ private static final FileFilter PACKAGE_JSON_FILTER = FileFilterBuilder.newInstance() .addFilenames(PACKAGE_LOCK_JSON, SHRINKWRAP_JSON).build(); 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 578b39e17..b8ecf1a5f 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 @@ -91,7 +91,7 @@ public class NodePackageAnalyzerTest extends BaseTest { @Test public void testSupportsFiles() { assertThat(analyzer.accept(new File("package-lock.json")), is(true)); - assertThat(analyzer.accept(new File("shrinkwrap.json")), is(true)); + assertThat(analyzer.accept(new File("npm-shrinkwrap.json")), is(true)); } /** @@ -102,7 +102,7 @@ public class NodePackageAnalyzerTest extends BaseTest { @Test public void testAnalyzeShrinkwrapJson() throws AnalysisException { final Dependency toScan = new Dependency(BaseTest.getResourceAsFile(this, - "nodejs/shrinkwrap.json")); + "nodejs/npm-shrinkwrap.json")); analyzer.analyze(toScan, engine); assertEquals("Expected 1 dependency", engine.getDependencies().length, 1); final Dependency result = engine.getDependencies()[0]; @@ -126,7 +126,7 @@ public class NodePackageAnalyzerTest extends BaseTest { final Dependency packageLock = new Dependency(BaseTest.getResourceAsFile(this, "nodejs/package-lock.json")); final Dependency shrinkwrap = new Dependency(BaseTest.getResourceAsFile(this, - "nodejs/shrinkwrap.json")); + "nodejs/npm-shrinkwrap.json")); engine.addDependency(packageLock); engine.addDependency(shrinkwrap); assertEquals(2, engine.getDependencies().length); diff --git a/dependency-check-core/src/test/resources/nodejs/shrinkwrap.json b/dependency-check-core/src/test/resources/nodejs/npm-shrinkwrap.json similarity index 100% rename from dependency-check-core/src/test/resources/nodejs/shrinkwrap.json rename to dependency-check-core/src/test/resources/nodejs/npm-shrinkwrap.json From c6363fde7a3a8f916bc030b11704c0faf4710837 Mon Sep 17 00:00:00 2001 From: Jeremy Long Date: Sat, 2 Dec 2017 08:06:16 -0500 Subject: [PATCH 21/22] code cleanup, checkstyle, codacy, findbugs, etc. --- .../analyzer/AbstractNpmAnalyzer.java | 35 ++----------------- .../analyzer/NodePackageAnalyzer.java | 20 ++++++----- .../dependencycheck/analyzer/NspAnalyzer.java | 4 +-- .../dependencycheck/reporting/EscapeTool.java | 10 +++--- .../analyzer/NspAnalyzerTest.java | 3 ++ 5 files changed, 23 insertions(+), 49 deletions(-) diff --git a/dependency-check-core/src/main/java/org/owasp/dependencycheck/analyzer/AbstractNpmAnalyzer.java b/dependency-check-core/src/main/java/org/owasp/dependencycheck/analyzer/AbstractNpmAnalyzer.java index a6d266e71..1dd2b41ba 100644 --- a/dependency-check-core/src/main/java/org/owasp/dependencycheck/analyzer/AbstractNpmAnalyzer.java +++ b/dependency-check-core/src/main/java/org/owasp/dependencycheck/analyzer/AbstractNpmAnalyzer.java @@ -77,7 +77,7 @@ public abstract class AbstractNpmAnalyzer extends AbstractFileTypeAnalyzer { accept = false; } } catch (IOException ex) { - throw new RuntimeException(ex); + throw new RuntimeException("Unable to process dependency", ex); } } @@ -211,38 +211,6 @@ public abstract class AbstractNpmAnalyzer extends AbstractFileTypeAnalyzer { if (DependencyBundlingAnalyzer.npmVersionsMatch(version, dependencyVersion)) { return d; } -// if (dependencyVersion.startsWith("^") || dependencyVersion.startsWith("~")) { -// dependencyVersion = dependencyVersion.substring(1); -// } -// -// if (version.equals(dependencyVersion)) { -// return d; -// } -// if (version.startsWith("^") || version.startsWith("~") || version.contains("*")) { -// String type; -// String tmp; -// if (version.startsWith("^") || version.startsWith("~")) { -// type = version.substring(0, 1); -// tmp = version.substring(1); -// } else { -// type = "*"; -// tmp = version; -// } -// final String[] v = tmp.split(" ")[0].split("\\."); -// final String[] depVersion = dependencyVersion.split("\\."); -// -// if ("^".equals(type) && v[0].equals(depVersion[0])) { -// return d; -// } else if ("~".equals(type) && v.length >= 2 && depVersion.length >= 2 -// && v[0].equals(depVersion[0]) && v[1].equals(depVersion[1])) { -// return d; -// } else if (v[0].equals("*") -// || (v.length >= 2 && v[0].equals(depVersion[0]) && v[1].equals("*")) -// || (v.length >= 3 && depVersion.length >= 2 && v[0].equals(depVersion[0]) -// && v[1].equals(depVersion[1]) && v[2].equals("*"))) { -// return d; -// } -// } } } return null; @@ -295,6 +263,7 @@ public abstract class AbstractNpmAnalyzer extends AbstractFileTypeAnalyzer { sb.append(array.getString(x)); } } + dependency.setLicense(sb.toString()); } else { dependency.setLicense(json.getJsonObject("license").getString("type")); } 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 7d6916406..4ee2f8dea 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 @@ -85,7 +85,8 @@ public class NodePackageAnalyzer extends AbstractNpmAnalyzer { */ public static final String SHRINKWRAP_JSON = "npm-shrinkwrap.json"; /** - * Filter that detects files named "package-lock.json" or "npm-shrinkwrap.json". + * Filter that detects files named "package-lock.json" or + * "npm-shrinkwrap.json". */ private static final FileFilter PACKAGE_JSON_FILTER = FileFilterBuilder.newInstance() .addFilenames(PACKAGE_LOCK_JSON, SHRINKWRAP_JSON).build(); @@ -176,7 +177,7 @@ public class NodePackageAnalyzer extends AbstractNpmAnalyzer { return; } } catch (IOException ex) { - throw new RuntimeException(ex); + throw new AnalysisException("Unable to process dependency", ex); } final File baseDir = dependencyFile.getParentFile(); if (PACKAGE_LOCK_JSON.equals(dependency.getFileName())) { @@ -210,14 +211,15 @@ public class NodePackageAnalyzer extends AbstractNpmAnalyzer { * dependencies and then finding the package.json for the module and adding * it as a dependency. * - * @param json - * @param baseDir - * @param rootFile - * @param parentPackage - * @param engine - * @throws AnalysisException + * @param json the data to process + * @param baseDir the base directory being scanned + * @param rootFile the root package-lock/npm-shrinkwrap being analyzed + * @param parentPackage the parent package name of the current node + * @param engine a reference to the dependency-check engine + * @throws AnalysisException thrown if there is an exception */ - private void processDependencies(final JsonObject json, File baseDir, File rootFile, final String parentPackage, Engine engine) throws AnalysisException { + private void processDependencies(JsonObject json, File baseDir, File rootFile, + String parentPackage, Engine engine) throws AnalysisException { if (json.containsKey("dependencies")) { final JsonObject deps = json.getJsonObject("dependencies"); for (Map.Entry entry : deps.entrySet()) { 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 0a9ae2cf4..1d983171c 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 @@ -169,7 +169,7 @@ public class NspAnalyzer extends AbstractNpmAnalyzer { return; } } catch (IOException ex) { - throw new RuntimeException(ex); + throw new AnalysisException("Unable to process dependency", ex); } try (JsonReader jsonReader = Json.createReader(FileUtils.openInputStream(file))) { @@ -206,7 +206,7 @@ public class NspAnalyzer extends AbstractNpmAnalyzer { * Create a single vulnerable software object - these do not use CPEs unlike the NVD. */ final VulnerableSoftware vs = new VulnerableSoftware(); - //TODO consider changing this to available versions on the dependency + //TODO consider changing this to available versions on the dependency // - the update is a part of the version, not versions to update to //vs.setUpdate(advisory.getPatchedVersions()); diff --git a/dependency-check-core/src/main/java/org/owasp/dependencycheck/reporting/EscapeTool.java b/dependency-check-core/src/main/java/org/owasp/dependencycheck/reporting/EscapeTool.java index e0829d0da..7911b47e6 100644 --- a/dependency-check-core/src/main/java/org/owasp/dependencycheck/reporting/EscapeTool.java +++ b/dependency-check-core/src/main/java/org/owasp/dependencycheck/reporting/EscapeTool.java @@ -126,7 +126,7 @@ public class EscapeTool { return "\"\""; } final String str = text.trim().replace("\n", " "); - if (str.length()==0) { + if (str.length() == 0) { return "\"\""; } return StringEscapeUtils.escapeCsv(str); @@ -155,7 +155,7 @@ public class EscapeTool { sb.append(id.getValue()); } } - if (sb.length()==0) { + if (sb.length() == 0) { return "\"\""; } return StringEscapeUtils.escapeCsv(sb.toString()); @@ -184,7 +184,7 @@ public class EscapeTool { sb.append(id.getValue()); } } - if (sb.length()==0) { + if (sb.length() == 0) { return "\"\""; } return StringEscapeUtils.escapeCsv(sb.toString()); @@ -213,7 +213,7 @@ public class EscapeTool { sb.append(id.getConfidence()); } } - if (sb.length()==0) { + if (sb.length() == 0) { return "\"\""; } return StringEscapeUtils.escapeCsv(sb.toString()); @@ -242,7 +242,7 @@ public class EscapeTool { sb.append(id.getValue()); } } - if (sb.length()==0) { + if (sb.length() == 0) { return "\"\""; } return StringEscapeUtils.escapeCsv(sb.toString()); diff --git a/dependency-check-core/src/test/java/org/owasp/dependencycheck/analyzer/NspAnalyzerTest.java b/dependency-check-core/src/test/java/org/owasp/dependencycheck/analyzer/NspAnalyzerTest.java index edb56b113..9cc3c7ea6 100644 --- a/dependency-check-core/src/test/java/org/owasp/dependencycheck/analyzer/NspAnalyzerTest.java +++ b/dependency-check-core/src/test/java/org/owasp/dependencycheck/analyzer/NspAnalyzerTest.java @@ -91,6 +91,9 @@ public class NspAnalyzerTest extends BaseTest { final Dependency result = new Dependency(BaseTest.getResourceAsFile(this, "nsp/minimal-invalid.json")); analyzer.analyze(result, engine); // Upon analysis, not throwing an exception in this case, is all that's required to pass this test + } catch(Throwable ex) { + fail("This test should not throw an exception"); + throw ex; } } } From d713e5d7d7723c0f7070a589151c770f8483c983 Mon Sep 17 00:00:00 2001 From: Jeremy Long Date: Sun, 3 Dec 2017 05:57:20 -0500 Subject: [PATCH 22/22] remove code duplication --- .../analyzer/AbstractNpmAnalyzer.java | 33 +++++++++++++++---- .../analyzer/NodePackageAnalyzer.java | 11 +------ .../dependencycheck/analyzer/NspAnalyzer.java | 12 +------ 3 files changed, 28 insertions(+), 28 deletions(-) diff --git a/dependency-check-core/src/main/java/org/owasp/dependencycheck/analyzer/AbstractNpmAnalyzer.java b/dependency-check-core/src/main/java/org/owasp/dependencycheck/analyzer/AbstractNpmAnalyzer.java index 1dd2b41ba..c88b22540 100644 --- a/dependency-check-core/src/main/java/org/owasp/dependencycheck/analyzer/AbstractNpmAnalyzer.java +++ b/dependency-check-core/src/main/java/org/owasp/dependencycheck/analyzer/AbstractNpmAnalyzer.java @@ -32,6 +32,7 @@ import javax.json.JsonObject; import javax.json.JsonObjectBuilder; import javax.json.JsonString; import javax.json.JsonValue; +import org.owasp.dependencycheck.analyzer.exception.AnalysisException; import org.owasp.dependencycheck.dependency.EvidenceType; import org.owasp.dependencycheck.utils.Checksum; @@ -71,19 +72,37 @@ public abstract class AbstractNpmAnalyzer extends AbstractFileTypeAnalyzer { boolean accept = super.accept(pathname); if (accept) { try { - // Do not scan the node_modules directory - if (pathname.getCanonicalPath().contains(File.separator + "node_modules" + File.separator)) { - LOGGER.debug("Skipping analysis of node module: " + pathname.getCanonicalPath()); - accept = false; - } - } catch (IOException ex) { - throw new RuntimeException("Unable to process dependency", ex); + accept |= shouldProcess(pathname); + } catch (AnalysisException ex) { + throw new RuntimeException(ex.getMessage(), ex.getCause()); } } return accept; } + /** + * Determines if the path contains "/node_modules/" (i.e. it is a child + * module. This analyzer does not scan child modules. + * + * @param pathname the path to test + * @return true if the path does not contain "/node_modules/" + * @throws AnalysisException thrown if the canonical path cannot be obtained + * from the given file + */ + protected boolean shouldProcess(File pathname) throws AnalysisException { + try { + // Do not scan the node_modules directory + if (pathname.getCanonicalPath().contains(File.separator + "node_modules" + File.separator)) { + LOGGER.debug("Skipping analysis of node module: " + pathname.getCanonicalPath()); + return false; + } + } catch (IOException ex) { + throw new AnalysisException("Unable to process dependency", ex); + } + return true; + } + /** * Construct a dependency object. * 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 4ee2f8dea..f5821d8de 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 @@ -167,18 +167,9 @@ public class NodePackageAnalyzer extends AbstractNpmAnalyzer { protected void analyzeDependency(Dependency dependency, Engine engine) throws AnalysisException { engine.removeDependency(dependency); final File dependencyFile = dependency.getActualFile(); - if (!dependencyFile.isFile() || dependencyFile.length() == 0) { + if (!dependencyFile.isFile() || dependencyFile.length() == 0 || !shouldProcess(dependencyFile)) { return; } - try { - // Do not scan the node_modules directory - if (dependencyFile.getCanonicalPath().contains(File.separator + "node_modules" + File.separator)) { - LOGGER.debug("Skipping analysis of node module: " + dependencyFile.getCanonicalPath()); - return; - } - } catch (IOException ex) { - throw new AnalysisException("Unable to process dependency", ex); - } final File baseDir = dependencyFile.getParentFile(); if (PACKAGE_LOCK_JSON.equals(dependency.getFileName())) { final File shrinkwrap = new File(baseDir, SHRINKWRAP_JSON); 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 1d983171c..41b6a36c6 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 @@ -158,20 +158,10 @@ public class NspAnalyzer extends AbstractNpmAnalyzer { protected void analyzeDependency(Dependency dependency, Engine engine) throws AnalysisException { engine.removeDependency(dependency); final File file = dependency.getActualFile(); - if (!file.isFile() || file.length() == 0) { + if (!file.isFile() || file.length() == 0 || !shouldProcess(file)) { return; } - try { - // Do not scan the node_modules directory - if (file.getCanonicalPath().contains(File.separator + "node_modules" + File.separator)) { - LOGGER.debug("Skipping analysis of node module: " + file.getCanonicalPath()); - return; - } - } catch (IOException ex) { - throw new AnalysisException("Unable to process dependency", ex); - } - try (JsonReader jsonReader = Json.createReader(FileUtils.openInputStream(file))) { // Retrieves the contents of package.json from the Dependency