npm: inconsistent results when dependency has both a dev and non-dev entry in lockfile

[nikpivkin]

Discussed in #5134

^{Originally posted by ngraef September 7, 2023}

Description

Trivy is giving inconsistent results between successive vulnerability scans with the same database version. A sample package-lock.json that induces the behavior is included (collapsed) below. I believe the issue is triggered by the same package ID (name@version) having multiple entries in the lockfile, where one has "dev": true and another doesn't. In this sample, [email protected] meets that condition.

Related: aquasecurity/go-dep-parser#149

Desired Behavior

I expect the scan results to be determinate and include all affected non-dev packages. With the sample lockfile, the expected output is:

package-lock.json (npm)
=======================
Total: 3 (UNKNOWN: 0, LOW: 0, MEDIUM: 2, HIGH: 0, CRITICAL: 1)

┌───────────┬────────────────┬──────────┬────────┬───────────────────┬───────────────┬───────────────────────────────────────────────────────────┐
│  Library  │ Vulnerability  │ Severity │ Status │ Installed Version │ Fixed Version │                           Title                           │
├───────────┼────────────────┼──────────┼────────┼───────────────────┼───────────────┼───────────────────────────────────────────────────────────┤
│ minimist  │ CVE-2021-44906 │ CRITICAL │ fixed  │ 0.0.8             │ 1.2.6, 0.2.4  │ prototype pollution                                       │
│           │                │          │        │                   │               │ https://avd.aquasec.com/nvd/cve-2021-44906                │
│           ├────────────────┼──────────┤        │                   ├───────────────┼───────────────────────────────────────────────────────────┤
│           │ CVE-2020-7598  │ MEDIUM   │        │                   │ 0.2.1, 1.2.3  │ nodejs-minimist: prototype pollution allows adding or     │
│           │                │          │        │                   │               │ modifying properties of Object.prototype using a...       │
│           │                │          │        │                   │               │ https://avd.aquasec.com/nvd/cve-2020-7598                 │
├───────────┼────────────────┤          │        ├───────────────────┼───────────────┼───────────────────────────────────────────────────────────┤
│ validator │ CVE-2021-3765  │          │        │ 7.2.0             │ 13.7.0        │ Inefficient Regular Expression Complexity in Validator.js │
│           │                │          │        │                   │               │ https://avd.aquasec.com/nvd/cve-2021-3765                 │
└───────────┴────────────────┴──────────┴────────┴───────────────────┴───────────────┴───────────────────────────────────────────────────────────┘

Actual Behavior

With the sample lockfile, the output excludes [email protected] in roughly 50% of scans:

package-lock.json (npm)
=======================
Total: 1 (UNKNOWN: 0, LOW: 0, MEDIUM: 1, HIGH: 0, CRITICAL: 0)

┌───────────┬───────────────┬──────────┬────────┬───────────────────┬───────────────┬───────────────────────────────────────────────────────────┐
│  Library  │ Vulnerability │ Severity │ Status │ Installed Version │ Fixed Version │                           Title                           │
├───────────┼───────────────┼──────────┼────────┼───────────────────┼───────────────┼───────────────────────────────────────────────────────────┤
│ validator │ CVE-2021-3765 │ MEDIUM   │ fixed  │ 7.2.0             │ 13.7.0        │ Inefficient Regular Expression Complexity in Validator.js │
│           │               │          │        │                   │               │ https://avd.aquasec.com/nvd/cve-2021-3765                 │
└───────────┴───────────────┴──────────┴────────┴───────────────────┴───────────────┴───────────────────────────────────────────────────────────┘

Reproduction Steps

1. npm init -y && npm install [email protected] [email protected] && npm install -D [email protected]
2. Run `trivy fs --scanners vuln --format table --vuln-type library ./package-lock.json`
3. Repeat step 2 several times

Target

Filesystem

Scanner

Vulnerability

Output Format

Table

Mode

Standalone

Debug Output

2023-09-06T12:04:30.419-0600    DEBUG   Severities: ["UNKNOWN" "LOW" "MEDIUM" "HIGH" "CRITICAL"]
2023-09-06T12:04:30.419-0600    DEBUG   Ignore statuses {"statuses": null}
2023-09-06T12:04:30.432-0600    DEBUG   cache dir:  /Users/me/Library/Caches/trivy
2023-09-06T12:04:30.433-0600    DEBUG   DB update was skipped because the local DB is the latest
2023-09-06T12:04:30.433-0600    DEBUG   DB Schema: 2, UpdatedAt: 2023-09-06 12:13:03.850148455 +0000 UTC, NextUpdate: 2023-09-06 18:13:03.850147555 +0000 UTC, DownloadedAt: 2023-09-06 16:59:07.828888 +0000 UTC
2023-09-06T12:04:30.433-0600    INFO    Vulnerability scanning is enabled
2023-09-06T12:04:30.433-0600    DEBUG   Vulnerability type:  [library]
2023-09-06T12:04:30.434-0600    DEBUG   Walk the file tree rooted at 'package-lock.json' in parallel
2023-09-06T12:04:30.434-0600    INFO    To collect the license information of packages in "package-lock.json", "npm install" needs to be performed beforehand
2023-09-06T12:04:30.454-0600    DEBUG   OS is not detected.
2023-09-06T12:04:30.454-0600    INFO    Number of language-specific files: 1
2023-09-06T12:04:30.454-0600    INFO    Detecting npm vulnerabilities...
2023-09-06T12:04:30.454-0600    DEBUG   Detecting library vulnerabilities, type: npm, path: package-lock.json

Operating System

macOS Ventura 13.5.1

Version

Version: 0.45.0
Vulnerability DB:
  Version: 2
  UpdatedAt: 2023-09-06 12:13:03.850148455 +0000 UTC
  NextUpdate: 2023-09-06 18:13:03.850147555 +0000 UTC
  DownloadedAt: 2023-09-06 16:59:07.828888 +0000 UTC

Checklist

• Run trivy image --reset
• Read the troubleshooting

[ngraef]

@nikpivkin I notice you changed the reproduction steps to include installing the dependencies in a fresh project. That won't work out of the box because newer patch versions of fsevents remove the node-pre-gyp dependency, which is what contributed the [email protected] entry with "dev": true in my example. You can force the older version by adding this override in package.json before installing gulp:

"overrides": {
  "gulp": {
    "fsevents": "1.2.9"
  }
}

Or, probably simpler, change the instructions to npm install -D [email protected] instead of gulp.

[nikpivkin]

@ngraef Good catch, I fixed it

[ngraef]

Repost from #5134 (comment)

This patch in aquasecurity/go-dep-parser fixes my use case:

diff --git a/pkg/nodejs/npm/parse.go b/pkg/nodejs/npm/parse.go
--- a/pkg/nodejs/npm/parse.go
+++ b/pkg/nodejs/npm/parse.go
@@ -116,6 +117,7 @@
 		// There are cases when similar libraries use same dependencies
 		// we need to add location for each these dependencies
 		if savedLib, ok := libs[pkgID]; ok {
+			savedLib.Dev = savedLib.Dev && pkg.Dev
 			savedLib.Locations = append(savedLib.Locations, location)
 			sort.Sort(savedLib.Locations)
 			libs[pkgID] = savedLib

I'm not sure what other implications that has or whether the same issue exists in parsers for other package managers.

Is there anything else I can do to help move this forward?

[faizan12123]

Is there any update on this issue?

[SemProvoost]

Any updates here? Issue seems to be consistently reproducible.

[DmitriyLewen]

Hi all!
Created #6356 for this task.