Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: Fix several issues on NPM Audit reporting #5546

Merged
merged 5 commits into from Mar 19, 2023
Merged

Conversation

aikebah
Copy link
Collaborator

@aikebah aikebah commented Mar 8, 2023

Fixes Issue #5419

Description of Change

  • Use the GHSA key from the NPM audit API response instead of the id, since as experienced by users the id returned is not stable over time.
  • Improve the reported references by parsing the references of the NPM Audit API back to individual URLs
  • The CWE information present in the NPM Audit report is now also properly extracted and reported on
  • The CVSS v3 information, when present in the NPM Audit report, is now also extracted and reported on. This also improves the reported scores as the score is no longer derived from the textual severity, but from the CVSS v3 score of the report when the NPM Audit report included CVSS v3 information.

Have test cases been added to cover the new functionality?

no

@boring-cyborg boring-cyborg bot added the core changes to core label Mar 8, 2023
@aikebah
Copy link
Collaborator Author

aikebah commented Mar 9, 2023

@jeremylong As this update changes the Advisory it will trigger errors logged by the node-audit cache when a report is found in cache for an npm-package that was stored by a previous release. The cache will in that case return null and a new npm audit report will be requested and stored in the cache, but the first time for each such package the log will contain an exception stacktrace.

Not quite sure whether we should us a minor or a major revision for this case (scan runs fine, but log appears to indicate errors due to the mismatch on serialVersionUid)

I do think it requires an explicit mention in the release-notes to try and avoid bug-reports.

[ERROR] Region [NODEAUDIT] IO Exception, Problem reading object from file
java.io.InvalidClassException: org.owasp.dependencycheck.data.nodeaudit.Advisory; local class incompatible: stream classdesc serialVersionUID = -6157232800626565476, local class serialVersionUID = -6157232800626565475
	at java.base/java.io.ObjectStreamClass.initNonProxy(ObjectStreamClass.java:597)
	at java.base/java.io.ObjectInputStream.readNonProxyDesc(ObjectInputStream.java:2051)
	at java.base/java.io.ObjectInputStream.readClassDesc(ObjectInputStream.java:1898)
	at java.base/java.io.ObjectInputStream.readOrdinaryObject(ObjectInputStream.java:2224)
	at java.base/java.io.ObjectInputStream.readObject0(ObjectInputStream.java:1733)
	at java.base/java.io.ObjectInputStream.readObject(ObjectInputStream.java:509)
	at java.base/java.io.ObjectInputStream.readObject(ObjectInputStream.java:467)
	at java.base/java.util.ArrayList.readObject(ArrayList.java:899)
	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:77)
	at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
	at java.base/java.lang.reflect.Method.invoke(Method.java:568)
	at java.base/java.io.ObjectStreamClass.invokeReadObject(ObjectStreamClass.java:1100)
	at java.base/java.io.ObjectInputStream.readSerialData(ObjectInputStream.java:2423)
	at java.base/java.io.ObjectInputStream.readOrdinaryObject(ObjectInputStream.java:2257)
	at java.base/java.io.ObjectInputStream.readObject0(ObjectInputStream.java:1733)
	at java.base/java.io.ObjectInputStream$FieldValues.<init>(ObjectInputStream.java:2606)
	at java.base/java.io.ObjectInputStream.readSerialData(ObjectInputStream.java:2457)
	at java.base/java.io.ObjectInputStream.readOrdinaryObject(ObjectInputStream.java:2257)
	at java.base/java.io.ObjectInputStream.readObject0(ObjectInputStream.java:1733)
	at java.base/java.io.ObjectInputStream.readObject(ObjectInputStream.java:509)
	at java.base/java.io.ObjectInputStream.readObject(ObjectInputStream.java:467)
	at org.apache.commons.jcs.utils.serialization.StandardSerializer.deSerialize(StandardSerializer.java:83)
	at org.apache.commons.jcs.auxiliary.disk.indexed.IndexedDisk.readObject(IndexedDisk.java:118)
	at org.apache.commons.jcs.auxiliary.disk.indexed.IndexedDiskCache.readElement(IndexedDiskCache.java:702)
	at org.apache.commons.jcs.auxiliary.disk.indexed.IndexedDiskCache.processGet(IndexedDiskCache.java:624)
	at org.apache.commons.jcs.auxiliary.AbstractAuxiliaryCacheEventLogging.getWithEventLogging(AbstractAuxiliaryCacheEventLogging.java:109)
	at org.apache.commons.jcs.auxiliary.disk.AbstractDiskCache.doGet(AbstractDiskCache.java:783)
	at org.apache.commons.jcs.auxiliary.disk.AbstractDiskCache.get(AbstractDiskCache.java:291)
	at org.apache.commons.jcs.engine.control.CompositeCache.get(CompositeCache.java:565)
	at org.apache.commons.jcs.engine.control.CompositeCache.get(CompositeCache.java:476)
	at org.apache.commons.jcs.access.CacheAccess.get(CacheAccess.java:76)
	at org.owasp.dependencycheck.data.cache.DataCache.get(DataCache.java:52)
	at org.owasp.dependencycheck.data.nodeaudit.NodeAuditSearch.submitPackage(NodeAuditSearch.java:127)
	at org.owasp.dependencycheck.analyzer.NodeAuditAnalyzer.analyzePackage(NodeAuditAnalyzer.java:189)
	at org.owasp.dependencycheck.analyzer.NodeAuditAnalyzer.analyzeDependency(NodeAuditAnalyzer.java:146)
	at org.owasp.dependencycheck.analyzer.AbstractAnalyzer.analyze(AbstractAnalyzer.java:131)
	at org.owasp.dependencycheck.AnalysisTask.call(AnalysisTask.java:88)
	at org.owasp.dependencycheck.AnalysisTask.call(AnalysisTask.java:37)
	at java.base/java.util.concurrent.FutureTask.run(FutureTask.java:264)
	at java.base/java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1136)
	at java.base/java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:635)
	at java.base/java.lang.Thread.run(Thread.java:833)
[ERROR] Region [NODEAUDIT] Failure getting from disk, key = 7340b428314024028b1e43b279a9ac96b09aba39fac53e27aa5eda1b7b445e58
java.io.InvalidClassException: org.owasp.dependencycheck.data.nodeaudit.Advisory; local class incompatible: stream classdesc serialVersionUID = -6157232800626565476, local class serialVersionUID = -6157232800626565475
	at java.base/java.io.ObjectStreamClass.initNonProxy(ObjectStreamClass.java:597)
	at java.base/java.io.ObjectInputStream.readNonProxyDesc(ObjectInputStream.java:2051)
	at java.base/java.io.ObjectInputStream.readClassDesc(ObjectInputStream.java:1898)
	at java.base/java.io.ObjectInputStream.readOrdinaryObject(ObjectInputStream.java:2224)
	at java.base/java.io.ObjectInputStream.readObject0(ObjectInputStream.java:1733)
	at java.base/java.io.ObjectInputStream.readObject(ObjectInputStream.java:509)
	at java.base/java.io.ObjectInputStream.readObject(ObjectInputStream.java:467)
	at java.base/java.util.ArrayList.readObject(ArrayList.java:899)
	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:77)
	at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
	at java.base/java.lang.reflect.Method.invoke(Method.java:568)
	at java.base/java.io.ObjectStreamClass.invokeReadObject(ObjectStreamClass.java:1100)
	at java.base/java.io.ObjectInputStream.readSerialData(ObjectInputStream.java:2423)
	at java.base/java.io.ObjectInputStream.readOrdinaryObject(ObjectInputStream.java:2257)
	at java.base/java.io.ObjectInputStream.readObject0(ObjectInputStream.java:1733)
	at java.base/java.io.ObjectInputStream$FieldValues.<init>(ObjectInputStream.java:2606)
	at java.base/java.io.ObjectInputStream.readSerialData(ObjectInputStream.java:2457)
	at java.base/java.io.ObjectInputStream.readOrdinaryObject(ObjectInputStream.java:2257)
	at java.base/java.io.ObjectInputStream.readObject0(ObjectInputStream.java:1733)
	at java.base/java.io.ObjectInputStream.readObject(ObjectInputStream.java:509)
	at java.base/java.io.ObjectInputStream.readObject(ObjectInputStream.java:467)
	at org.apache.commons.jcs.utils.serialization.StandardSerializer.deSerialize(StandardSerializer.java:83)
	at org.apache.commons.jcs.auxiliary.disk.indexed.IndexedDisk.readObject(IndexedDisk.java:118)
	at org.apache.commons.jcs.auxiliary.disk.indexed.IndexedDiskCache.readElement(IndexedDiskCache.java:702)
	at org.apache.commons.jcs.auxiliary.disk.indexed.IndexedDiskCache.processGet(IndexedDiskCache.java:624)
	at org.apache.commons.jcs.auxiliary.AbstractAuxiliaryCacheEventLogging.getWithEventLogging(AbstractAuxiliaryCacheEventLogging.java:109)
	at org.apache.commons.jcs.auxiliary.disk.AbstractDiskCache.doGet(AbstractDiskCache.java:783)
	at org.apache.commons.jcs.auxiliary.disk.AbstractDiskCache.get(AbstractDiskCache.java:291)
	at org.apache.commons.jcs.engine.control.CompositeCache.get(CompositeCache.java:565)
	at org.apache.commons.jcs.engine.control.CompositeCache.get(CompositeCache.java:476)
	at org.apache.commons.jcs.access.CacheAccess.get(CacheAccess.java:76)
	at org.owasp.dependencycheck.data.cache.DataCache.get(DataCache.java:52)
	at org.owasp.dependencycheck.data.nodeaudit.NodeAuditSearch.submitPackage(NodeAuditSearch.java:127)
	at org.owasp.dependencycheck.analyzer.NodeAuditAnalyzer.analyzePackage(NodeAuditAnalyzer.java:189)
	at org.owasp.dependencycheck.analyzer.NodeAuditAnalyzer.analyzeDependency(NodeAuditAnalyzer.java:146)
	at org.owasp.dependencycheck.analyzer.AbstractAnalyzer.analyze(AbstractAnalyzer.java:131)
	at org.owasp.dependencycheck.AnalysisTask.call(AnalysisTask.java:88)
	at org.owasp.dependencycheck.AnalysisTask.call(AnalysisTask.java:37)
	at java.base/java.util.concurrent.FutureTask.run(FutureTask.java:264)
	at java.base/java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1136)
	at java.base/java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:635)
	at java.base/java.lang.Thread.run(Thread.java:833)
[INFO] Finished Node Audit Analyzer (0 seconds)

@aikebah
Copy link
Collaborator Author

aikebah commented Mar 9, 2023

And given this required serialization mismatch issue I'm wondering whether we should obtain more info from the NPM audit report into the advisory as well.... I see the CVEs, CWEs and a CVSS v3.1 score in there... maybe take the opportunity to improve on those as well before we integrate this?

@aikebah
Copy link
Collaborator Author

aikebah commented Mar 9, 2023

e.g.

  
  "cves": [
    "CVE-2020-7788"
  ],
  "cwe": [
    "CWE-1321"
  ],



  "cvss": {
    "score": 7.3,
    "vectorString": "CVSS:3.1\/AV:N\/AC:L\/PR:N\/UI:N\/S:U\/C:L\/I:L\/A:L"
  },

In the case of pkg:npm/ini@1.3.5

Copy link
Owner

@jeremylong jeremylong left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@aikebah aikebah linked an issue Mar 10, 2023 that may be closed by this pull request
@aikebah aikebah changed the title fix: Use the stable github_advisory_id instead of the now unstable id in NPM audit results fix: Fix several issues on NPM Audit parsing Mar 10, 2023
@aikebah aikebah changed the title fix: Fix several issues on NPM Audit parsing fix: Fix several issues on NPM Audit reporting Mar 10, 2023
The existing code tried to retrieve cwe as a string, but in the NPM Audit report it is an array. This commit parses the array and adds each CWE
Fixes issue #5551
@jeremylong
Copy link
Owner

I'll definitely add notes about the exception linking back to this entry. I'm fine either way if you want to incorporate more data from the API. I was planning on releasing the next version this weekend.

@aikebah
Copy link
Collaborator Author

aikebah commented Mar 16, 2023

@jeremylong that schedule matches nicely with my polishing up the final modifications - addition of the CVSS v3 score to NPM that I hope to finish tonight

@aikebah aikebah marked this pull request as ready for review March 16, 2023 20:12
@aikebah aikebah requested a review from jeremylong March 16, 2023 20:30
@aikebah aikebah added this to the 8.2.0 milestone Mar 16, 2023
@aikebah aikebah merged commit 1cb2f20 into main Mar 19, 2023
7 of 8 checks passed
@aikebah aikebah deleted the use-GHSA-for-NPMAudit branch March 19, 2023 15:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
core changes to core
Projects
None yet
2 participants