Skip to content

Commit

Permalink
fix: Fix several issues on NPM Audit reporting (#5546)
Browse files Browse the repository at this point in the history
  • Loading branch information
aikebah authored Mar 19, 2023
2 parents b54db81 + 7b55d8a commit 1cb2f20
Show file tree
Hide file tree
Showing 9 changed files with 155 additions and 33 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@
import java.io.File;
import java.io.IOException;
import java.net.MalformedURLException;
import java.net.URL;
import java.util.Collection;
import java.util.List;
import java.util.Map;
Expand Down Expand Up @@ -451,14 +452,28 @@ protected void processResults(final List<Advisory> advisories, Engine engine,
//Create a new vulnerability out of the advisory returned by nsp.
final Vulnerability vuln = new Vulnerability();
vuln.setDescription(advisory.getOverview());
vuln.setName(String.valueOf(advisory.getId()));
vuln.setName(String.valueOf(advisory.getGhsaId()));
vuln.setUnscoredSeverity(advisory.getSeverity());
vuln.setCvssV3(advisory.getCvssV3());
vuln.setSource(Vulnerability.Source.NPM);
vuln.addReference(
"Advisory " + advisory.getId() + ": " + advisory.getTitle(),
advisory.getReferences(),
null
);
for (String cwe : advisory.getCwes()) {
vuln.addCwe(cwe);
}
if (advisory.getReferences() != null) {
final String[] references = advisory.getReferences().split("\\n");
for (String reference : references) {
if (reference.length() > 3) {
String url = reference.substring(2);
try {
new URL(url);
} catch (MalformedURLException ignored) {
// reference is not a format-valid URL, so null it to make the reference be used as plaintext
url = null;
}
vuln.addReference("NPM Advisory reference: ", url == null ? reference : url, url);
}
}
}

//Create a single vulnerable software object - these do not use CPEs unlike the NVD.
final VulnerableSoftwareBuilder builder = new VulnerableSoftwareBuilder();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@
*/
package org.owasp.dependencycheck.data.nodeaudit;

import org.owasp.dependencycheck.dependency.CvssV3;

import java.io.Serializable;
import java.util.List;
import javax.annotation.concurrent.ThreadSafe;
Expand All @@ -33,12 +35,12 @@ public class Advisory implements Serializable {
/**
* Serial version UID.
*/
private static final long serialVersionUID = -6157232800626565476L;
private static final long serialVersionUID = -6157232800626565475L;

/**
* The unique ID of the advisory as issued by NPM.
* The github_advisory_id of the advisory as issued by GHSA-hosted NPM Audit API.
*/
private int id;
private String ghsaId;

/**
* The timestamp of which the advisory was created.
Expand Down Expand Up @@ -118,17 +120,14 @@ public class Advisory implements Serializable {
private String severity;

/**
* The CWE of the advisory.
* The CWEs of the advisory.
*/
private String cwe;

public int getId() {
return id;
}
private List<String> cwes;

public void setId(int id) {
this.id = id;
}
/**
* The CVSSv3 of the advisory.
*/
private CvssV3 cvssV3;

public String getCreated() {
return created;
Expand Down Expand Up @@ -250,12 +249,27 @@ public void setSeverity(String severity) {
this.severity = severity;
}

public String getCwe() {
return cwe;
public List<String> getCwes() {
return cwes;
}

public void setCwe(String cwe) {
this.cwe = cwe;
public void setCwes(List<String> cwes) {
this.cwes = cwes;
}

public String getGhsaId() {
return ghsaId;
}

public void setGhsaId(String ghsaId) {
this.ghsaId = ghsaId;
}

public CvssV3 getCvssV3() {
return cvssV3;
}

public void setCvssV3(CvssV3 cvssV3) {
this.cvssV3 = cvssV3;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@

import org.json.JSONArray;
import org.json.JSONObject;
import org.owasp.dependencycheck.dependency.CvssV3;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import java.util.ArrayList;
Expand Down Expand Up @@ -70,7 +71,7 @@ public List<Advisory> parse(JSONObject jsonResponse) throws JSONException {
*/
private Advisory parseAdvisory(JSONObject object) throws JSONException {
final Advisory advisory = new Advisory();
advisory.setId(object.getInt("id"));
advisory.setGhsaId(object.getString("github_advisory_id"));
advisory.setOverview(object.optString("overview", null));
advisory.setReferences(object.optString("references", null));
advisory.setCreated(object.optString("created", null));
Expand All @@ -84,7 +85,15 @@ private Advisory parseAdvisory(JSONObject object) throws JSONException {
advisory.setPatchedVersions(object.optString("patched_versions", null));
advisory.setAccess(object.optString("access", null));
advisory.setSeverity(object.optString("severity", null));
advisory.setCwe(object.optString("cwe", null));

final JSONArray jsonCwes = object.optJSONArray("cwe");
final List<String> stringCwes = new ArrayList<>();
if (jsonCwes != null) {
for (int j = 0; j < jsonCwes.length(); j++) {
stringCwes.add(jsonCwes.getString(j));
}
}
advisory.setCwes(stringCwes);

final JSONArray findings = object.optJSONArray("findings");
for (int i = 0; i < findings.length(); i++) {
Expand All @@ -107,6 +116,36 @@ private Advisory parseAdvisory(JSONObject object) throws JSONException {
}
advisory.setCves(stringCves);
}
final JSONObject jsonCvss = object.optJSONObject("cvss");
if (jsonCvss != null) {
float baseScore = -1.0f;
final String score = jsonCvss.optString("score");
if (score != null) {
try {
baseScore = Float.parseFloat(score);
} catch (NumberFormatException ignored) {
LOGGER.trace("Swallowed NumberFormatException", ignored);
baseScore = -1.0f;
}
}
if (baseScore >= 0.0) {
final String vector = jsonCvss.optString("vectorString");
if (vector != null) {
if (vector.startsWith("CVSS:3") && baseScore >= 0.0) {
try {
final CvssV3 cvss = new CvssV3(vector, baseScore);
advisory.setCvssV3(cvss);
} catch (IllegalArgumentException iae) {
LOGGER.warn("Invalid CVSS vector format encountered in NPM Audit results '{}' ", vector, iae);
}
} else {
LOGGER.warn("Unsupported CVSS vector format in NPM Audit results, please file a feature "
+ "request at https://github.com/jeremylong/DependencyCheck/issues/new/choose to "
+ "support vector format '{}' ", vector);
}
}
}
}
return advisory;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,12 @@
*/
package org.owasp.dependencycheck.dependency;

import org.sonatype.ossindex.service.api.cvss.Cvss3Severity;

import java.io.Serializable;
import java.util.Arrays;
import java.util.HashMap;
import java.util.List;

/**
* CVSS V3 scoring information.
Expand All @@ -31,6 +36,11 @@ public class CvssV3 implements Serializable {
*/
private static final long serialVersionUID = -315810090425928920L;

/**
* The CVSS v3 Base Metrics (that are required by the spec for any CVSS v3 Vector String)
*/
private static final List<String> BASE_METRICS = Arrays.asList("AV", "AC", "PR", "UI", "S", "C", "I", "A");

/**
* CVSS Availability Impact.
*/
Expand Down Expand Up @@ -142,6 +152,50 @@ public CvssV3(String attackVector, String attackComplexity, String privilegesReq
}
//CSON: ParameterNumber

/**
* Constructs a new CVSS V3 object from a CVSS v3.x Vector String representation and
* a CVSS V3 Base score.
*
* @param vectorString a CVSS v3 Vector String
* @param baseScore the CVSS v3 base score
* @throws IllegalArgumentException when the provided vectorString is not a valid
* <a href="https://www.first.org/cvss/specification-document#Vector-String">CVSS v3.x vector string</a>.
*/
public CvssV3(String vectorString, float baseScore) {
if (!vectorString.startsWith("CVSS:3")) {
throw new IllegalArgumentException("Not a valid CVSSv3 vector string: " + vectorString);
}
this.version = vectorString.substring(5, vectorString.indexOf('/'));
final String[] metricStrings = vectorString.substring(vectorString.indexOf('/') + 1).split("/");
final HashMap<String, String> metrics = new HashMap<>();
for (int i = 0; i < metricStrings.length; i++) {
final String[] metricKeyVal = metricStrings[i].split(":");
if (metricKeyVal.length != 2) {
throw new IllegalArgumentException(
String.format("Not a valid CVSSv3 vector string '%s', invalid metric component '%s'",
vectorString, metricStrings[i]));
}
metrics.put(metricKeyVal[0], metricKeyVal[1]);
}
if (!metrics.keySet().containsAll(BASE_METRICS)) {
throw new IllegalArgumentException(
String.format("Not a valid CVSSv3 vector string '%s'; missing one or more required Base Metrics;",
vectorString));
}
this.attackVector = metrics.get("AV");
this.attackComplexity = metrics.get("AC");
this.privilegesRequired = metrics.get("PR");
this.userInteraction = metrics.get("UI");
this.scope = metrics.get("S");
this.confidentialityImpact = metrics.get("C");
this.integrityImpact = metrics.get("I");
this.availabilityImpact = metrics.get("A");
this.baseScore = baseScore;
this.baseSeverity = Cvss3Severity.of(baseScore).name();
this.exploitabilityScore = null;
this.impactScore = null;
}

/**
* Get the value of attackVector.
*
Expand Down
6 changes: 3 additions & 3 deletions core/src/main/resources/templates/htmlReport.vsl
Original file line number Diff line number Diff line change
Expand Up @@ -958,7 +958,7 @@ Getting Help: <a href="https://github.com/jeremylong/DependencyCheck/issues" tar
#if($vuln.getSource().name().equals("NVD"))
<p><b><a target="_blank" href="http://web.nvd.nist.gov/view/vuln/detail?vulnId=$enc.url($vuln.name)">$enc.html($vuln.name)</a></b>&nbsp;&nbsp;<button class="copybutton" title="Generate Suppression XML for this CVE for this file" data-display-name="$enc.html($dependency.DisplayFileName)" data-sha1="$enc.html($dependency.Sha1sum)" data-pkgurl="#if($supressPkgUrl)$enc.html($supressPkgUrl)#end" data-type-to-suppress="cve" data-id-to-suppress="$enc.html($vuln.name)">suppress</button></p>
#elseif($vuln.getSource().name().equals("NPM"))
<p><b><a target="_blank" href="https://www.npmjs.com/advisories/$enc.url($vuln.name)">NPM-$enc.html($vuln.name)</a></b>&nbsp;&nbsp;<button class="copybutton" title="Generate Suppression XML for this vulnerability for this file" data-display-name="$enc.html($dependency.DisplayFileName)" data-sha1="$enc.html($dependency.Sha1sum)" data-pkgurl="#if($supressPkgUrl)$enc.html($supressPkgUrl)#end" data-type-to-suppress="vulnerabilityName" data-id-to-suppress="$enc.html($vuln.name)">suppress</button></p>
<p><b><a target="_blank" href="https://github.com/advisories/$enc.url($vuln.name)">$enc.html($vuln.name) (NPM)</a></b>&nbsp;&nbsp;<button class="copybutton" title="Generate Suppression XML for this vulnerability for this file" data-display-name="$enc.html($dependency.DisplayFileName)" data-sha1="$enc.html($dependency.Sha1sum)" data-pkgurl="#if($supressPkgUrl)$enc.html($supressPkgUrl)#end" data-type-to-suppress="vulnerabilityName" data-id-to-suppress="$enc.html($vuln.name)">suppress</button></p>
#else
<p><span class="underline"><b>$enc.html($vuln.name)</b>&nbsp;($vuln.getSource().name())</span>&nbsp;&nbsp;<button class="copybutton" title="Generate Suppression XML for this vulnerability for this file" data-display-name="$enc.html($dependency.DisplayFileName)" data-sha1="$enc.html($dependency.Sha1sum)" data-pkgurl="#if($supressPkgUrl)$enc.html($supressPkgUrl)#end" data-type-to-suppress="vulnerabilityName" data-id-to-suppress="$enc.html($vuln.name)">suppress</button></p>
#end
Expand Down Expand Up @@ -1159,7 +1159,7 @@ Getting Help: <a href="https://github.com/jeremylong/DependencyCheck/issues" tar
#if($vuln.getSource().name().equals("NVD"))
<p><b><a target="_blank" href="http://web.nvd.nist.gov/view/vuln/detail?vulnId=$enc.url($vuln.name)">$enc.html($vuln.name)</a></b>&nbsp;&nbsp;<span class="suppressedLabel" >suppressed</span></p>
#elseif($vuln.getSource().name().equals("NPM"))
<p><b><a target="_blank" href="https://www.npmjs.com/advisories/$enc.url($vuln.name)">NPM-$enc.html($vuln.name)</a></b>&nbsp;&nbsp;<span class="suppressedLabel" >suppressed</span></p>
<p><b><a target="_blank" href="https://github.com/advisories/$enc.url($vuln.name)">$enc.html($vuln.name) (NPM)</a></b>&nbsp;&nbsp;<span class="suppressedLabel" >suppressed</span></p>
#else
<p><b>$enc.html($vuln.name)</b> ($vuln.getSource().name())&nbsp;&nbsp;<span class="suppressedLabel" >suppressed</span></p>
#end
Expand Down Expand Up @@ -1250,7 +1250,7 @@ Getting Help: <a href="https://github.com/jeremylong/DependencyCheck/issues" tar
<br/>
This report may contain data retrieved from the <a href="https://www.cisa.gov/known-exploited-vulnerabilities-catalog">CISA Known Exploited Vulnerability Catalog</a>.
<br/>
This report may contain data retrieved from the <a href="https://www.npmjs.com/advisories">NPM Public Advisories</a>.
This report may contain data retrieved from the <a href="https://github.com/advisories/">Github Advisory Database (via NPM Audit API)</a>.
<br/>
This report may contain data retrieved from <a href="https://retirejs.github.io/retire.js/">RetireJS</a>.
<br/>
Expand Down
4 changes: 2 additions & 2 deletions core/src/main/resources/templates/jenkinsReport.vsl
Original file line number Diff line number Diff line change
Expand Up @@ -713,7 +713,7 @@ Getting Help: <a href="https://github.com/jeremylong/DependencyCheck/issues" tar
#if($vuln.getSource().name().equals("NVD"))
<p><b><a target="_blank" href="http://web.nvd.nist.gov/view/vuln/detail?vulnId=$enc.url($vuln.name)">$enc.html($vuln.name)</a></b></p>
#elseif($vuln.getSource().name().equals("NPM"))
<p><b><a target="_blank" href="https://www.npmjs.com/advisories/$enc.url($vuln.name)">NPM-$enc.html($vuln.name)</a></b></p>
<p><b><a target="_blank" href="https://github.com/advisories/$enc.url($vuln.name)">$enc.html($vuln.name) (NPM)</a></b></p>
#else
<p><span class="underline"><b>$enc.html($vuln.name)</b>&nbsp;($vuln.getSource().name())</span></p>
#end
Expand Down Expand Up @@ -787,7 +787,7 @@ Getting Help: <a href="https://github.com/jeremylong/DependencyCheck/issues" tar
<br/>
This report may contain data retrieved from the <a href="https://www.cisa.gov/known-exploited-vulnerabilities-catalog">CISA Known Exploited Vulnerability Catalog</a>.
<br/>
This report may contain data retrieved from the <a href="https://www.npmjs.com/advisories">NPM Public Advisories</a>.
This report may contain data retrieved from the <a href="https://github.com/advisories/">Github Advisory Database (via NPM Audit API)</a>.
<br/>
This report may contain data retrieved from <a href="https://retirejs.github.io/retire.js/">RetireJS</a>.
<br/>
Expand Down
2 changes: 1 addition & 1 deletion core/src/main/resources/templates/jsonReport.vsl
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@
"credits": {
"NVD": "This report contains data retrieved from the National Vulnerability Database: http://nvd.nist.gov",
"CISA": "This report may contain data retrieved from the CISA Known Exploited Vulnerability Catalog: https://www.cisa.gov/known-exploited-vulnerabilities-catalog",
"NPM": "This report may contain data retrieved from the NPM Public Advisories: https://www.npmjs.com/advisories",
"NPM": "This report may contain data retrieved from the Github Advisory Database (via NPM Audit API): https://github.com/advisories/",
"RETIREJS": "This report may contain data retrieved from the RetireJS community: https://retirejs.github.io/retire.js/",
"OSSINDEX": "This report may contain data retrieved from the Sonatype OSS Index: https://ossindex.sonatype.org"
}
Expand Down
4 changes: 2 additions & 2 deletions core/src/main/resources/templates/sarifReport.vsl
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@
"help": {
"text": "",
"markdown": "#if($rule.getSource().equals("NVD"))For more information see [$enc.json($rule.name)](http://web.nvd.nist.gov/view/vuln/detail?vulnId=$enc.url($rule.name)).\n
#elseif($rule.getSource().equals("NPM"))For more information see [NPM-$enc.json($rule.name)](https://www.npmjs.com/advisories/$enc.url($rule.name))\n#end
#elseif($rule.getSource().equals("NPM"))For more information see [$enc.json($rule.name)](https://github.com/advisories/$enc.url($rule.name))\n#end
\n\n
If this is a false positive - consider using the HTML report to generate a suppression file.
For more information see [How dependency-check works](https://jeremylong.github.io/DependencyCheck/general/internals.html),
Expand Down Expand Up @@ -63,7 +63,7 @@ For more information see [How dependency-check works](https://jeremylong.github.
"disclaimer": "Dependency-Check is an open source tool performing a best effort analysis of 3rd party dependencies; false positives and false negatives may exist in the analysis performed by the tool. Use of the tool and the reporting provided constitutes acceptance for use in an AS IS condition, and there are NO warranties, implied or otherwise, with regard to the analysis or its use. Any use of the tool and the reporting provided is at the user's risk. In no event shall the copyright holder or OWASP be held liable for any damages whatsoever arising out of or in connection with the use of this tool, the analysis performed, or the resulting report.",
"nvd": "This report contains data retrieved from the National Vulnerability Database: http://nvd.nist.gov",
"cisa": "This report may contain data retrieved from the CISA Known Exploited Vulnerability Catalog: https://www.cisa.gov/known-exploited-vulnerabilities-catalog",
"npm": "This report may contain data retrieved from the NPM Public Advisories: https://www.npmjs.com/advisories",
"npm": "This report may contain data retrieved from the the Github Advisory Database (via NPM Audit API): https://github.com/advisories/",
"retirejs": "This report may contain data retrieved from the RetireJS community: https://retirejs.github.io/retire.js/",
"ossindex": "This report may contain data retrieved from the Sonatype OSS Index: https://ossindex.sonatype.org"
#foreach($prop in $properties.getMetaData().entrySet())
Expand Down
Loading

0 comments on commit 1cb2f20

Please sign in to comment.