npm / cli

the package manager for JavaScript
https://docs.npmjs.com/cli/
Other
8.52k stars 3.19k forks source link

[BUG] CVE-2024-21538 - cross-spawn to 7.0.5+ #7902

Closed crudo closed 4 days ago

crudo commented 2 weeks ago

Is there an existing issue for this?

This issue exists in the latest npm version

Current Behavior

@wraithgar Since we cannot create updates of bundled node_modules, could you please bump the cross-spawn to 7.0.5 or higher? https://security.snyk.io/vuln/SNYK-JS-CROSSSPAWN-8303230

Expected Behavior

No response

Steps To Reproduce

  1. In this environment...
  2. With this config...
  3. Run '...'
  4. See error...

Environment

CraigAtInstrumental commented 1 week ago

Note that this vulnerability is rated as "High".

ljharb commented 1 week ago

indeed, but since you'd have to be attacking yourself to trigger the ReDOS in npm, it's not actually a vulnerability here.

Mayvis commented 1 week ago

Any update on this?

klever34 commented 1 week ago

any update on this issue?

MasterCarl commented 1 week ago

As a workaround, if you're using the Trivy GitHub action, you could exclude the bundled package from the scan by specifying e.g.

uses: aquasecurity/trivy-action
with:
  skip-dirs: node_modules,/usr/local/lib/node_modules/npm/node_modules/cross-spawn

Update: For some reason, this only works with my local trivy CLI, not the GitHub action.

klever34 commented 1 week ago

Another workaround is simply creating a .trivyignore file, and adding CVE-2024-21538 to it.

planv commented 1 week ago

By the way, they have just released a new version - 7.0.6 . https://www.npmjs.com/package/cross-spawn

lempira commented 1 week ago

I am running into the same issue. I first tried to use the override in package.json but that didn't work because cross-spawn is a bundled dependency of another dep.

image

I am using better-npm-audit for the project, so adding the following the .nsprc file allows temporary bypass for that package.

{
  "1100467": {
    "active": true,
    "notes": "Waiting for https://github.com/npm/cli/issues/7902 to be resolved",
    "expiry": "2024-12-31"
  }
}
SpencerWhitehead7 commented 1 week ago

This is preventing my team from deploying because of an automated security scan in google cloud. I know the vulnerability is basically a non-issue in the context of the cli, but the scan does not.

Please bump past the vulnerable version as part of the weekly release. From the lockfile, I think the cli only has it as a transitive dependency, and the transitives have version ranges compatible with the fixed version so it shouldn't be too hard and it's make a huge difference to anyone at the mercy of automated security scans they can't control.

I'd be happy to try and do it myself, but the contribution guidelines explicitly disallow third party fixes for dependencies.

luka-papez commented 6 days ago

For us this was a non-issue because we realized don't really need cross-spawn in the deployed image.

The fix is simple when that is the case. In the Docker image we deploy, simply removed cross-spawn package from node_modules regarding npm, for example:

RUN rm -r /usr/local/lib/node_modules/npm/node_modules/cross-spawn/
RUN rm -r /usr/lib/node_modules_20/npm/node_modules/cross-spawn/
RUN rm -r /usr/local/n/versions/node/18.20.5/lib/node_modules/npm/node_modules/cross-spawn/

This also made me realize that we don't need npm either, and will go even further to remove it too.

I know that some projects need npm in the final image so this solution won't work, but in case you're only using npm to build the app, it's possible you want only the built assets in the final image and don't need npm anyway.

So actually for us this was an improvement in reducing the threat surface of our deployed app :smile_cat:

SpencerWhitehead7 commented 6 days ago

Thank you, I really appreciate your offering a solution. We're trying something similar, but it'd be nice not to have to and I don't know what a project that needed npm in the final image would do.

luka-papez commented 6 days ago

I'm guessing here, but if you need updated cross-spawn in the deployed image couldn't you follow an approach similar to deleting it?

cd /usr/local/n/versions/node/<your version here>/lib/node_modules/npm/
rm -r node_modules/cross-spawn
npm install cross-spawn@7.0.6
harshit-gupta-simplisafe commented 6 days ago

When this will be fix?

draialexis commented 6 days ago

I have a PR open to fix the vuln in foreground-child: https://github.com/tapjs/foreground-child/pull/60

newlinedeveloper commented 6 days ago

Any update on this issue ?

thomasverleye commented 6 days ago

Created a PR but just read that dependency upgrades will be closed and only maintainers can do this, so tagging those maintainers here as this is block loads of people. 🙇🏻

@nlf @wraithgar @lukekarrys @fritzy

juhyunk0820 commented 6 days ago

I'm using Node.js 22.9 & npm 10.8.2 version. i updated the node&npm version(nodejs 20.18 -> 22.11, npm: 10.8.3 -> 10.9.0)and problem occurred. So I downgraded the node&npm version and vulnerability disappeared. But I think it's a temporary measure, So I want to know resolution. (I am Korean. Please understand if I am not good at English.)

nilesh-sutar4755 commented 6 days ago

I'm using Node.js 22.9 & npm 10.8.2 version. i updated the node&npm version(nodejs 20.18 -> 22.11, npm: 10.8.3 -> 10.9.0)and problem occurred. So I downgraded the node&npm version and vulnerability disappeared. But I think it's a temporary measure, So I want to know resolution. (I am Korean. Please understand if I am not good at English.)

Hey @juhyunk0820 which version of node is not vulnerable according to you ?

gidsola commented 6 days ago

honestly, the wide use of npm alone should be cause enough to keep it updated. To let it go a week?? srsly, professionalism is gone these days.

juhyunk0820 commented 6 days ago

@nilesh-sutar4755 I'm using nodejs@22.9.0 & npm@10.8.2. it's not velnerable in my case.

Juraj-Masiar commented 6 days ago

I'm gonna suggest here a strange looking fix - some of you may actually not need @swc/cli. If you are using Webpack with swc-loader and @swc/core (and maybe also @swc/css), you don't need the command line cli package at all.

draialexis commented 6 days ago

honestly, the wide use of npm alone should be cause enough to keep it updated. To let it go a week?? srsly, professionalism is gone these days.

@gidsola Give them a break, we're talking about open-source code here, you know how these things go -- they're maintained by overworked volunteers who could probably use a break from aggressive remarks like these

draialexis commented 6 days ago

Their release process says they do releases on Wednesdays. Out-of-band releases could happen for critical vulns, but they don't say that about high vulns. I'm hopeful that we'll see a resolution tomorrow.

Source: https://github.com/npm/cli/wiki/Release-Process#overview

ljharb commented 6 days ago

also, it's not actually a vulnerability based on its usage in npm, so it shouldn't be blocking anyone unless their security policies are incorrectly over-rigid.

gidsola commented 6 days ago

@gidsola Give them a break, we're talking about open-source code here, you know how these things go -- they're maintained by overworked volunteers who could probably use a break from aggressive remarks like these

You are right. I apologize. bad morning, no excuse.

wraithgar commented 5 days ago

As has been pointed out already, this is not a critical vulnerability, it is a false positive. The package itself has a vulnerability but it does not affect npm. This will be updated w/ the next npm release which should be soon.

warnerjc commented 5 days ago

@wraithgar is this confirmed fixed in https://nodejs.org/en/blog/release/v20.18.1? I could have sworn I saw the release notes for 20.18.1 on 11/13/2024.

warnerjc commented 4 days ago

@wraithgar and other maintainers-

I'm following these:

It does not appear that Node v20.18.1, shipping with npm v10.8.2, addresses this CVE. Checking the package-lock for npm v10.8.2, cross-spawn is still at a vulnerable level of v7.0.3. So, it's still unclear to me that uplifting from v20.18.0 to v20.18.1 will remediate this issue.

Will there be another Node v20.18.1+ release prior to next Wednesday?

Tofandel commented 4 days ago

The last few versions of node 20 always shipped with npm 10.8.2, and I doubt this will be changed soon given that there has been 10.8.3 and 10.9.0 for a while but only node 22 gets shipped with npm 10.9.0

Node is not npm and vice versa, even if npm is bundled when installing node. You can, and should, upgrade your npm independently from node and always do so after a fresh install of node

Npm hasn't released 10.9.1 with the fix yet, so I don't see how node would have addressed this in 20.18.1. It's much more likely that node 22 will get that upgrade before node 20

humam-nameer-10p commented 4 days ago

For us this was a non-issue because we realized don't really need cross-spawn in the deployed image.

The fix is simple when that is the case. In the Docker image we deploy, simply removed cross-spawn package from node_modules regarding npm, for example:

RUN rm -r /usr/local/lib/node_modules/npm/node_modules/cross-spawn/
RUN rm -r /usr/lib/node_modules_20/npm/node_modules/cross-spawn/
RUN rm -r /usr/local/n/versions/node/18.20.5/lib/node_modules/npm/node_modules/cross-spawn/

This also made me realize that we don't need npm either, and will go even further to remove it too.

I know that some projects need npm in the final image so this solution won't work, but in case you're only using npm to build the app, it's possible you want only the built assets in the final image and don't need npm anyway.

So actually for us this was an improvement in reducing the threat surface of our deployed app 😸

I can confirm this worked for my gcloud build. thanks!

wraithgar commented 4 days ago

Fixed in npm 10.9.1 (and 9.9.4)