Skip to content

fix #4837 Upgrade glob dependency to 8.0.3 #4941

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.

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

Closed
wants to merge 1 commit into from

Conversation

joewagner
Copy link

@joewagner joewagner commented Nov 2, 2022

Description of the Change

The minimatch npm package version <=3.0.4 has a security vulnerability explained here: GHSA-f8q6-p94x-37v3

Mocha has the glob package 7.2.0 as a dependency, which in turn has minimatch ^3.0.4 as a dependency

This results in a high security risk warning for consumers of mocha. See #4937 for an example.

This PR changes the glob dependency to 8.0.3 which no longer depends on the insecure version of minimatch.

Alternate Designs

The current glob version is a fixed value (7.2.0), I thought about using the ^ to set it as ^8.0.3 but the other dependencies here are all fixed versions so I'm following suit.

Why should this be in core?

It's not possible to fix #4937 without updating core.

Benefits

Fix security concerns raised in GHSA-f8q6-p94x-37v3

Possible Drawbacks

All tests pass for me, and I see no drawbacks.

Applicable issues

Applicable Issue is #4937
This should be suitable for a patch release.

@linux-foundation-easycla
Copy link

linux-foundation-easycla bot commented Nov 2, 2022

CLA Signed

The committers listed above are authorized under a signed CLA.

  • ✅ login: joewagner / name: Joe Wagner (5629d61)

@juergba
Copy link
Contributor

juergba commented Nov 3, 2022

@joewagner thank you.

I don't agree with your description and conclusions.

$ npm list minimatch -all --production
[email protected] D:\mocha-fork\mocha
├─┬ [email protected]
│ └── [email protected]
└── [email protected]

While installing mocha correctly, minimatch ^3.0.4 resolves to v3.1.2 which - as per your description - should not print any vulnerability report.

@juergba juergba added the dependencies Pull requests that update a dependency file label Nov 3, 2022
@joewagner
Copy link
Author

joewagner commented Nov 3, 2022

@juergba Thanks for checking this out.
I'm not reporting a security vulnerability, since as you point out module resolution lands on secure versions.
But since ^3.04 is inclusive of the 3.04 version of minimatch, other packages will end up getting security alerts.
Here's a dependabot example: https://github.com/tablelandnetwork/local-tableland/security/dependabot/2
I figured that since other people were getting the same type of alerts I'd try to make a fix. Are there any drawbacks to upgrading?

@coveralls
Copy link

Coverage Status

Coverage remained the same at 94.326% when pulling 5629d61 on joewagner:joewagner/upgrade-glob into 8f3c37b on mochajs:master.

@juergba
Copy link
Contributor

juergba commented Nov 3, 2022

Are there any drawbacks to upgrading?

If you can fix the failing tests on Windows, then I don't see any drawbacks.

@joewagner
Copy link
Author

If you can fix the failing tests on Windows, then I don't see any drawbacks.

I don't use windows. I can try to set up a vm and look at this, but it may take me a while to find the time.

@mochajs mochajs deleted a comment from phitattoo Dec 25, 2022
@jb2311
Copy link
Contributor

jb2311 commented Mar 8, 2023

@joewagner @juergba Is it okay for me to take over this PR/branch?

@juergba
Copy link
Contributor

juergba commented Mar 8, 2023

@jb2311 yes, it's probably better to open a new PR. I would close this one then.

@joewagner
Copy link
Author

@joewagner @juergba Is it okay for me to take over this PR/branch?

Definitely! Thanks for taking this on, and sorry I didn't find time to setup a Windows env.

@jb2311
Copy link
Contributor

jb2311 commented Mar 8, 2023

created this PR: #4970

All the tests passed on my local windows machine. Only difference from this PR is a newer version of glob

@juergba juergba closed this Mar 10, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dependencies Pull requests that update a dependency file
Projects
None yet
Development

Successfully merging this pull request may close these issues.

🔒 Security: Relies on [email protected] version with minimatch ReDoS vulnerability
4 participants