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.

Sign up for GitHub

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

Jump to bottom

Remove minimist dependency via mkdirp (vulnerability CVE-2020-7598) #13050

Closed
davidgilbertson opened this issue Mar 15, 2020 · 12 comments
Closed

Remove minimist dependency via mkdirp (vulnerability CVE-2020-7598) #13050

davidgilbertson opened this issue Mar 15, 2020 · 12 comments
Labels
archived due to age This issue has been archived; please open a new issue for any further discussion core Relates to ESLint's core APIs and features enhancement This change enhances an existing feature of ESLint evaluating The team will evaluate this issue to decide whether it meets the criteria for inclusion

Comments

@davidgilbertson
Copy link

The version of ESLint you are using.
6.8.0

The problem you want to solve.
Remove the warning about the minimist vulnerability detailed here GHSA-7fhm-mqm4-2wp7

This is required via a dependency on the mkdirp package.

Your take on the correct solution to problem.
@mysticatea has already done this in a PR targeting version 7. I suggest making this change in a patch to 6.8.

Are you willing to submit a pull request to implement this change?
Yep.
@davidgilbertson davidgilbertson added core Relates to ESLint's core APIs and features enhancement This change enhances an existing feature of ESLint triage An ESLint team member will look at this issue soon labels Mar 15, 2020
@evanplaice evanplaice mentioned this issue Mar 15, 2020
Closed
@evanplaice
Copy link
Contributor

@davidgilbertson Do you mind if I submit a PR with a fix for this issue?

evanplaice added a commit to evanplaice/eslint that referenced this issue Mar 16, 2020
@evanplaice
WIP on issue13050: Fix: Backport removal of mkdirp to 6.x (eslint#13050)
366b2d6
@davidgilbertson
Copy link
Author

Go for it!

evanplaice added a commit to evanplaice/eslint that referenced this issue Mar 16, 2020
@evanplaice
WIP on issue13050: Fix: Backport removal of mkdirp to 6.x (eslint#13050)
cdf04cc 
* the CVE is caused by the mkdirp dependency
* mkdirp is no longer supported
* mkdirp has been removed as of 7.0.0-alpha0
* this back-ports the change to v6.x
@evanplaice evanplaice mentioned this issue Mar 16, 2020
Closed
2 tasks
evanplaice added a commit to evanplaice/eslint that referenced this issue Mar 16, 2020
@evanplaice
Fix: Backport removal of mkdirp to 6.x (fixes eslint#13050)
555f033 
* the CVE is caused by the mkdirp dependency
* mkdirp is no longer supported
* mkdirp has been removed as of 7.0.0-alpha0
* this back-ports the change to v6.x
@evanplaice
Copy link
Contributor

The PR is in, all tests are passing. That's as far as I can go for now.

@au5ton
Copy link

au5ton commented Mar 16, 2020

Thank you, I've been seeing this for days

Closed
@rzr
Copy link

rzr commented Mar 16, 2020

Release to npm welcome or tell which git branch should be used ?

@vcastro-DDS vcastro-DDS mentioned this issue Mar 16, 2020
Closed
2 tasks
@WPMGPRoSToTeMa WPMGPRoSToTeMa mentioned this issue Mar 16, 2020
Closed
@wu-lee wu-lee mentioned this issue Mar 16, 2020
Merged
@miketalley miketalley mentioned this issue Mar 16, 2020
Closed
@Rugvip Rugvip mentioned this issue Mar 16, 2020
Closed
@Vektrat Vektrat mentioned this issue Mar 17, 2020
Closed
@littlewhywhat littlewhywhat mentioned this issue Mar 17, 2020
Closed
2 tasks
@evanplaice
Copy link
Contributor

Update: mkdirp published a new version that bumps minimist to a safe version.

Patching ESLint is no longer unnecessary.

Re-creating package-lock.json should fix the issue.

@celosauro
Copy link

Update: mkdirp published a new version that bumps minimist to a safe version.

Patching ESLint is no longer unnecessary.

Re-creating package-lock.json should fix the issue.

Still failing and not updated:

> npm ls minimist   
├─┬ eslint@6.8.0
│ └─┬ mkdirp@0.5.1
│   └── minimist@0.0.8

@willkjackson
Copy link

Wiping package-lock/minimist from mode_modules and reinstalling packages updated the tree:

> npm ls minimist
├─┬ eslint@6.8.0
│ └─┬ mkdirp@0.5.3
│   └── minimist@1.2.5

@KEINOS KEINOS mentioned this issue Mar 19, 2020
Merged
@yumetodo
Copy link

yumetodo commented Mar 19, 2020 •
edited

I decided to ignore this security warning.
mkdirp dependency was already removed by #12753 however, eslint v7 is alpha release. And backport PR was rejected #13051 .
The way described by @willkjackson has a question "that is really safe? major version change should be doing more carefully, isn't it?"
So I will wait eslint v7 stable release.

I missed that in mkdirp@0.5.2, isaacs/node-mkdirp@c5b97d1 , major version was updated.

@freben freben mentioned this issue Mar 19, 2020
Merged
@phish108 phish108 mentioned this issue Mar 19, 2020
Closed
@tvvignesh tvvignesh mentioned this issue Mar 19, 2020
Closed
@bcoe bcoe mentioned this issue Mar 19, 2020
Closed
@bcoe bcoe mentioned this issue Mar 19, 2020
Closed
@kaicataldo kaicataldo added evaluating The team will evaluate this issue to decide whether it meets the criteria for inclusion and removed triage An ESLint team member will look at this issue soon labels Mar 20, 2020
@kaicataldo
Copy link
Member

Given the workaround listed here, we have made the decision not to backport this change to the 6.x line.

wu-lee pushed a commit to DigitalCommons/mykomap that referenced this issue Mar 20, 2020
package-lock.json, node_modules/ - remove and regenerate
460c8b5 
This fixes the problems with ESLint, as described here:

eslint/eslint#13050 (comment)

The problematic dependency `mkdirp` published a new version that bumps
`minimist` to a safe version.
miketalley added a commit to HubSpot/hubspot-cli that referenced this issue Mar 20, 2020
@miketalley
recreated yarn.lock to fix dependency vulnerabilities according to es…
7ef07c8 
…lint/eslint#13050 (comment)
@miketalley miketalley mentioned this issue Mar 20, 2020
Merged
Amphiluke added a commit to Amphiluke/lindsvg that referenced this issue Mar 21, 2020
@Amphiluke
Re-install eslint to fix a security warning
afe5330 
eslint/eslint#13050
calebHankins added a commit to calebHankins/node-playground that referenced this issue Mar 24, 2020
@calebHankins
Regenerating package-lock to address [CVE-2020-7598] as per guidance …
e5e9df6 
…found here: eslint/eslint#13050 (comment)
@RyanZim RyanZim mentioned this issue Mar 25, 2020
Closed
@yudai-nkt yudai-nkt mentioned this issue Mar 27, 2020
Closed
@kaicataldo kaicataldo mentioned this issue Mar 27, 2020
Closed
wincent added a commit to liferay/liferay-frontend-guidelines that referenced this issue Apr 6, 2020
@wincent
chore: regenerate lockfile
f9e5603 
As per:

eslint/eslint#13050

This gives us a clean `yarn audit`, getting rid of:

    ┌───────────────┬──────────────────────────────────────────────────────────────┐
    │ low           │ Prototype Pollution                                          │
    ├───────────────┼──────────────────────────────────────────────────────────────┤
    │ Package       │ minimist                                                     │
    ├───────────────┼──────────────────────────────────────────────────────────────┤
    │ Patched in    │ >=0.2.1 <1.0.0 || >=1.2.3                                    │
    ├───────────────┼──────────────────────────────────────────────────────────────┤
    │ Dependency of │ eslint                                                       │
    ├───────────────┼──────────────────────────────────────────────────────────────┤
    │ Path          │ eslint > file-entry-cache > flat-cache > write > mkdirp >    │
    │               │ minimist                                                     │
    ├───────────────┼──────────────────────────────────────────────────────────────┤
    │ More info     │ https://www.npmjs.com/advisories/1179                        │
    └───────────────┴──────────────────────────────────────────────────────────────┘
    ┌───────────────┬──────────────────────────────────────────────────────────────┐
    │ low           │ Prototype Pollution                                          │
    ├───────────────┼──────────────────────────────────────────────────────────────┤
    │ Package       │ minimist                                                     │
    ├───────────────┼──────────────────────────────────────────────────────────────┤
    │ Patched in    │ >=0.2.1 <1.0.0 || >=1.2.3                                    │
    ├───────────────┼──────────────────────────────────────────────────────────────┤
    │ Dependency of │ eslint                                                       │
    ├───────────────┼──────────────────────────────────────────────────────────────┤
    │ Path          │ eslint > mkdirp > minimist                                   │
    ├───────────────┼──────────────────────────────────────────────────────────────┤
    │ More info     │ https://www.npmjs.com/advisories/1179                        │
    └───────────────┴──────────────────────────────────────────────────────────────┘
    2 vulnerabilities found - Packages audited: 596
@wincent wincent mentioned this issue Apr 6, 2020
Merged
@laurenbarker laurenbarker mentioned this issue Apr 7, 2020
Merged
@Zodiase Zodiase mentioned this issue May 11, 2020
Closed
@oliikit
Copy link

oliikit commented May 18, 2020

For those with really old package-lock.json and not a tightly controlled package.json, completing wiping the package-lock.json to reinstall dependencies can break their application. I'd recommend the following option:

 npm uninstall eslint; npm install -D eslint@VERSION

That will update all the subdependencies in eslint without you be in dependency hell.

@evanplaice
Copy link
Contributor

@oliikit The latest SemVer major release (ie v6.x) of ESLint removes the need for this change altogether.

Id recommend upgrading if possible

@eslint-deprecated eslint-deprecated bot locked and limited conversation to collaborators Sep 18, 2020
@eslint-deprecated eslint-deprecated bot added the archived due to age This issue has been archived; please open a new issue for any further discussion label Sep 18, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
archived due to age This issue has been archived; please open a new issue for any further discussion core Relates to ESLint's core APIs and features enhancement This change enhances an existing feature of ESLint evaluating The team will evaluate this issue to decide whether it meets the criteria for inclusion
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
9 participants
@rzr @evanplaice @willkjackson @au5ton @davidgilbertson @kaicataldo @yumetodo @oliikit @celosauro