Node: Update npm on all supported release lines to address CVE scored 9.8 in minimist package

Created on 16 Mar 2020 · 28Comments · Source: nodejs/node

Is your feature request related to a problem? Please describe.
The package mkdir 0.5.1 contains a dependency to minimist 0.0.8, which has the CVE-2020-7598, scored 9.8

Describe the solution you'd like
Remove the package mkdirp or find a maintained alternative.

Others

node -v
v12.16.1

npm -v
6.13.4

list mkdirp
[email protected] /usr/lib/node_modules/npm
+-- [email protected]
| `-- [email protected]  deduped
+-- [email protected]
| `-- [email protected]  deduped
+-- [email protected]
| `-- [email protected]  deduped
+-- [email protected]
| `-- [email protected]  deduped
+-- [email protected]
+-- [email protected]
| +-- [email protected]
| | `-- [email protected]  deduped
| `-- [email protected]  deduped
+-- [email protected]
| `-- [email protected]  deduped
+-- [email protected]
| `-- [email protected]  deduped
`-- [email protected]
  `-- [email protected]  deduped

npm

Source

mleneveut

Most helpful comment

What actual vulnerability is being addressed here? The CVE itself seems to indicate that the attack vector is "you can craft a malicious command line argument to attack yourself", which doesn't seem like something particularly urgent.

My opinion is that there is no vulnerability. I __strongly disagree__ on considering this a vulnerability on minimist in all form or fashion. If somebody can manipulate the commands passed to a CLI, they are definitely outside the Node.js threat model (and therefore the one of every app for Node.js). My opinion is that the security process failed the community in this specific case. However, the topic is not if minimist is vulnerable or not, it is to avoid Node.js including a vulnerable piece of code.

The problem is that every vulnerability scanner is going to pinpoint Node.js as vulnerable because those files are on disk. This is causing disruption to all enterprise deployments: most companies have a very strict rule of _no_ known vulnerabilities. As a result, we have to __ship releases asap to all lines, without waiting for the 2 weeks period__.

cc @nodejs/releasers

mcollina on 18 Mar 2020

👍8

All 28 comments

seems to have been forked and released in v1.0.3 without the minimalist deps : https://github.com/isaacs/node-mkdirp

mleneveut on 16 Mar 2020

This should be posted to the npm issue tracker instead.

mscdex on 16 Mar 2020

I did, thx

mleneveut on 16 Mar 2020

I think we should keep this open because we'll need to issue new releases on all LTS line.

mcollina on 16 Mar 2020

Can the subject be changed to something more specific, is this the plan?

Update npm on all supported release lines to address CVE

Right now, subjects suggests we'll be updating a package deep inside npm's deps, which I assume/hope is not the intention.

sam-github on 16 Mar 2020

The intention should be for npm to do releases for all those lines and we'll just backport those to all lts lines

mcollina on 16 Mar 2020

cc @nodejs/tsc this is important.

mcollina on 16 Mar 2020

I think this is not important because mkdirp doesn't use minimist in its API (only in the CLI, which is never used by npm or any of its dependencies).

targos on 16 Mar 2020

It is because most vulnerability scanners are going to detect this automatically.

mcollina on 16 Mar 2020

If we want to quickly fix this on our side, we can probably just rm -rf deps/npm/node_modules/minimist

targos on 16 Mar 2020

I think hacking deps/npm sets a bad precedent, but given a 10.x is going out tomorrow, maybe it can update npm to the latest (assuming latest fixes this).

sam-github on 16 Mar 2020

I think hacking deps/npm sets a bad precedent, but given a 10.x is going out tomorrow, maybe it can update npm to the latest (assuming latest fixes this).

AFAIK there isn't a fixed version of npm yet. v6.14.2 (the latest npm release) still has [email protected]: https://github.com/npm/cli/blob/v6.14.2/node_modules/minimist/package.json

Tracking bug: https://github.com/npm/cli/issues/1027

richardlau on 16 Mar 2020

We've floated patches to npm in the past, fwiw. I would be a bit more comfortable with patching the tree to squelch any dependency warnings than shipping with a version of npm that hasn't gone out in any other release lines.

MylesBorins on 17 Mar 2020

I asked the npm folks in the openjs slack and Darcy confirmed they will be shipping an npm release today so if this can wait out a bit for that maybe that can work

lirantal on 17 Mar 2020

👍3

There's an OpenSSL update due out today (https://github.com/nodejs/node/issues/32210). That could potentially go out in the same release as the npm update.

richardlau on 17 Mar 2020

I will wait a tiny bit with the next v13 release to get a fix into that release.

BridgeAR on 17 Mar 2020

I don't have an active line to any npm folks these days but if they want to co-ordinate on a node-gyp release then I'd like to hear about it. We have a flagged minimist in our dep tree via mkdirp too but we try and keep our dep ranges roughly in line with npm's too. So for them to ship a "safe" npm will require a "safe" node-gyp.
Can someone pass this on to them: https://github.com/nodejs/node-gyp/issues/2074
Or just contact me directly, my email address is all over the place, or IRC.

(Also, this whole minimist issue is beyond bogus, I hate this binary security culture we have that incentivises certain companies to make package maintainers lives hard).

rvagg on 17 Mar 2020

👍2

Actually, we (node-gyp) probably don't need to do anything to synchronize, we ship with "mkdirp": "^0.5.1" and Isaac has released an 0.5.3 to update the dependency. We don't ship a package-lock.json, so new installs should take care of this, npm just needs to update their deps and it should be taken care of.

rvagg on 18 Mar 2020

👍1

FYI, as Isaac released a 0.5.3 of mkdirp, a simple npm update (actually two) fixes the CVE in a node 12.x :

cd /usr/lib/node_modules/npm/node_modules/rc && npm update
cd /usr/lib/node_modules/npm && npm update

mleneveut on 18 Mar 2020

🎉1

What actual vulnerability is being addressed here? The CVE itself seems to indicate that the attack vector is "you can craft a malicious command line argument to attack yourself", which doesn't seem like something particularly urgent.

Additionally, it seems like mkdirp can be updated to v0.5.3 trivially, even as a floating patch. Can that be pulled in and shipped in v13 ASAP, especially if that has to start a two-week clock?

ljharb on 18 Mar 2020

What actual vulnerability is being addressed here? The CVE itself seems to indicate that the attack vector is "you can craft a malicious command line argument to attack yourself", which doesn't seem like something particularly urgent.

cc @nodejs/releasers

mcollina on 18 Mar 2020

👍8

I agree 100% with @mcollina. This is unfortunately forcing us to make a release when it should not have. Let's just do it and move on.

jasnell on 18 Mar 2020

👍3

Thanks for clarifying, that position makes sense to me.

ljharb on 18 Mar 2020

👍1

v12.x (https://github.com/nodejs/node/pull/32313) and v10.x (https://github.com/nodejs/node/pull/31984) have releases due Tuesday 24th March that the necessary patch/update could be pulled into. Is that timeframe sufficient?

It's possible v13.x could be sooner (@BridgeAR https://github.com/nodejs/Release/issues/487#issuecomment-600256245)

Are we still waiting on a new version of npm or do we just need to float a patch?

BethGriggs on 19 Mar 2020

v12.x (#32313) and v10.x (#31984) have releases due Tuesday 24th March that the necessary patch/update could be pulled into. Is that timeframe sufficient?

I think that's sufficient.