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.

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

fix(security): upgraded flat to version ^5.0.2 #64

Merged
merged 2 commits into from
Oct 1, 2020

Conversation

brandonhawi
Copy link
Contributor

The package flat has deprecated many of its versions due to a known vulnerability regarding prototype pollution. The previously used version, 4.1.0, was one of the many versions that were deprecated. As far as this package is concerned however, upgrading to 5.0.2 should cause no issues as the flatten method did not experience a breaking change in the major version change.

@mceachen
Copy link

mceachen commented Aug 14, 2020

FWIW this is causing all consumers of mocha to get Dependabot security notifications. See https://snyk.io/vuln/SNYK-JS-FLAT-596927

@brandonhawi
Copy link
Contributor Author

Yeah that's exactly what led me here...

@deleonio
Copy link

#65

@rajivshah3
Copy link

The change from var to const might be the reason for the breaking change. According to MDN, modern browsers and Node.js 6.0.0 and later should support it though.

@brandonhawi
Copy link
Contributor Author

The change from var to const might be the reason for the breaking change. According to MDN, modern browsers and Node.js 6.0.0 and later should support it though.

I find that a bit hard to believe that the reason this isn't getting merged is because of the change from var to const considering that yargs-unparser itself uses const. Refer here. I could be wrong, however.

In general I think we'll just have to wait for the maintainers to come and give their feedback to know if there's anything wrong here.

@rajivshah3
Copy link

Yeah, I don't think that should be a problem here. I mainly mentioned that in response to this:

As far as this package is concerned however, upgrading to 5.0.2 should cause no issues as the flatten method did not experience a breaking change in the major version change.

@mastermatt
Copy link

Has anyone in the yargs org been pinged? @bcoe maybe?
I'm seeing this security vun show up on Dependabot and Snyk so far.

@AviVahl
Copy link

AviVahl commented Oct 1, 2020

@bcoe anyone who installs mocha sees the deprecation warning. any chance you can review this?
Also, would you accept PRs updating other deps and dropping Node 6/8? (considering 14 is becoming LTS this month)

@bcoe bcoe changed the title Upgraded flat to version ^5.0.2 fix(security): upgraded flat to version ^5.0.2 Oct 1, 2020
@bcoe bcoe merged commit 9bd7c67 into yargs:master Oct 1, 2020
@bcoe
Copy link
Member

bcoe commented Oct 2, 2020

This wasn't merged because I lost track of the PR, and the vulnerability wasn't listed in npm audit (or on any of my repositories that use mocha). -- otherwise I would have been quickly reminded.

Any ways, sorry for the slow response.

@AviVahl
Copy link

AviVahl commented Oct 2, 2020

Much appreciated. I've opened a PR in mocha which includes the new release.

@bcoe
Copy link
Member

bcoe commented Oct 2, 2020

@AviVahl could you explain to me why I'm not seeing this in npm audit, or on any of my libraries that use mocha? what am missing. I'm feeling pretty frustrated about how fragmented the current state of security is.

@mceachen
Copy link

mceachen commented Oct 2, 2020

@bcoe I saw it from using snyk:

 $ snyk test --dev

Tested 457 dependencies for known issues, found 1 issue, 1 vulnerable path.

Issues with no direct upgrade or patch:
  ✗ Prototype Pollution [Medium Severity][https://snyk.io/vuln/SNYK-JS-FLAT-596927] in [email protected]
    introduced by [email protected] > [email protected] > [email protected]
  This issue was fixed in versions: 5.0.2, 4.1.2, 3.0.1, 2.0.2, 5.0.2, 1.6.2

@rajivshah3
Copy link

I'm feeling pretty frustrated about how fragmented the current state of security is.

I've personally found Snyk to be the most up-to-date recently. It seems like it takes a little while for vulnerabilities to show up in NPM or GitHub's vulnerability databases

@AviVahl
Copy link

AviVahl commented Oct 2, 2020

There are several tools that report audit failures. npm audit appears to have its own database of recorded vulnerabilities. snyk has a different one.

The author of flat also marked the older version with a deprecation message informing about the security risk (using npm deprecate), so it shows to anyone installing mocha without a lock file (or as a new depedency in an existing project).

Both npm and yarn show the deprecation warning. yarn is a bit more explicit.

Currently, mkdir unparser-test && cd unparser-test && npm init -y && npm i mocha shows:

npm WARN deprecated [email protected]: Fixed a prototype pollution security issue in 4.1.0, please upgrade to ^4.1.1 or ^5.0.1.

and mkdir unparser-test && cd unparser-test && yarn init -y && yarn add mocha shows:

warning mocha > yargs-unparser > [email protected]: Fixed a prototype pollution security issue in 4.1.0, please upgrade to ^4.1.1 or ^5.0.1.

Deprecation warnings are only shown when that deprecated version is resolved from the registry. If one uses a lock file (either package-lock.json or yarn.lock), they wouldn't be re-shown until it is being deleted or a new request to (vulnerable) flat is being resolved.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants