Mocha: Replace vulnerable mkdirp with fs.mkdir

Created on 12 Mar 2020  路  7Comments  路  Source: mochajs/mocha

  • [x] Checked that your issue hasn't already been filed by cross-referencing issues with the faq label
  • [x] Checked next-gen ES issues and syntax problems by using the same environment and/or transpiler configuration without Mocha to ensure it isn't just a feature that actually isn't supported in the environment in question or a bug in your code.
  • [x] 'Smoke tested' the code to be tested by running it outside the real test suite to get a better sense of whether the problem is in the code under test, your usage of Mocha, or Mocha itself
  • [x] Ensured that there is no discrepancy between the locally and globally installed versions of Mocha. You can find them with: node node_modules/.bin/mocha --version(Local) and mocha --version(Global). We recommend that you _not_ install Mocha globally.

Description

Mocha does depend on mkdirp, which depends on an old version of minimist, which has a known Prototype pollution vulnerability: https://snyk.io/test/github/mochajs/mocha

It does not seem that minimist is getting updated soon, and also, fs.mkdir(path[, options], callback) can be run recursively, effectively obsoleting mkdirp. Furthermore, packages depending on any version of mocha currently exhibit this vulnerability (cf. any package on snyk.io), and are shown to the user as vulnerable, too.

  • The output of mocha --version and node node_modules/.bin/mocha --version: 7.1.0
  • The output of node --version: v13.6.0
  • Your operating system

    • name and version: Windows 10

    • architecture (32 or 64-bit): 64-bit

  • Your shell (e.g., bash, zsh, PowerShell, cmd): PS
  • Any code transpiler (e.g., TypeScript, CoffeeScript, Babel) being used (and its version): no
good-first-issue node.js security

Most helpful comment

It should be enough to add caret (^) to mkdirp dependency, because 0.5.3 has minimist updated:

% npm info [email protected] dependencies 
{ minimist: '^1.2.5' }

All 7 comments

please note that fs.mkdir with recursive: true does _not_ completely obsolete mkdirp. that said, I think we can move away from it once Node.js v8 support is dropped from Mocha. the recursive flag arrived in Node.js v10.12.0.

We will drop Node.js v8 support in mocha v8.0.0, see https://github.com/mochajs/mocha/pull/4164

It should be enough to add caret (^) to mkdirp dependency, because 0.5.3 has minimist updated:

% npm info [email protected] dependencies 
{ minimist: '^1.2.5' }

If someone else gets here because of minimist vulnerability, do

  • force update mocha (remove from package.json and $: npm install mocha with optional
    --save-dev) Note: $: npm update might work as well. Didn't for me.
  • delete node_modules and package-lock.json
  • $: npm install to get all fresh and updated dependencies. Check with $: npm audit that all is fixed.

@ejke: The key is to use the depth flag with npm update. Even just npm update --depth 10 is typically enough (the docs recommend 9999, but besides often hanging, it doesn't seem to be typically necessary to check to such a high depth).

Getting this too in the log when installing -> npm WARN deprecated [email protected]: Legacy versions of mkdirp are no longer supported. Please update to mkdirp 1.x.

@JimmyBjorklund : That's an improvement (and separate issue) though (the message actually means you got the vulnerability fixed or avoided). While 0.5.3 is deprecated in favor of the new 1.0 series, 0.5.3 includes a fix for a security vulnerability present in 0.5.1 (which some of us still had in our node_modules or package-lock.json).

But it would indeed be good to start using the 1.0 series of mkdirp too (or remove it) as it is presumably more attractive to use internally given its use of Promises, and because it is the version currently being maintained. But as a 1.0 version, it introduces breaking changes, so there'd need to be some code refactoring to get it working.

Was this page helpful?
0 / 5 - 0 ratings