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

High severity security vulnerability in dot-prop #26128

Closed
derykmarl opened this issue Jul 30, 2020 · 17 comments · Fixed by #26142 or #26566
Closed

High severity security vulnerability in dot-prop #26128

derykmarl opened this issue Jul 30, 2020 · 17 comments · Fixed by #26142 or #26566
Assignees
Labels
good first issue Issue that doesn't require previous experience with Gatsby help wanted Issue with a clear description that the community can help with. type: bug An issue or pull request relating to a bug in Gatsby type: upstream Issues outside of Gatsby's control, caused by dependencies

Comments

@derykmarl
Copy link

Seeing the following:

                       === npm audit security report ===


                                 Manual Review

             Some vulnerabilities require your attention to resolve

          Visit https://go.npm.me/audit-guide for additional guidance
High            Prototype Pollution

  Package         dot-prop


  Patched in      >=5.1.1

  Dependency of   gatsby

  Path            gatsby > devcert > configstore > dot-prop

  More info       https://npmjs.com/advisories/1213

  High            Prototype Pollution

  Package         dot-prop

  Patched in      >=5.1.1

  Dependency of   gatsby

  Path            gatsby > gatsby-cli > update-notifier > configstore >
                  dot-prop

  More info       https://npmjs.com/advisories/1213

How do we go about getting dot-prop updated for these components of gatsby?

@derykmarl derykmarl added the type: bug An issue or pull request relating to a bug in Gatsby label Jul 30, 2020
@gatsbot gatsbot bot added the status: triage needed Issue or pull request that need to be triaged and assigned to a reviewer label Jul 30, 2020
@ascorbic
Copy link
Contributor

ascorbic commented Jul 30, 2020

This is, ironically, going to require an update from update-notifier. configstore already has a release that depends on dot-prop 5.2.0, but update-notifier hasn't been updated in a while. I'm going to close this, because dependabot will ensure we update once it's available. You might want to open an issue on the update-notifier repo, as it doesn't seem like anyone has. In the meantime you can use yarn resolutions in your own site, or one of the hacks that does similar for npm.

@ascorbic ascorbic added type: upstream Issues outside of Gatsby's control, caused by dependencies and removed status: triage needed Issue or pull request that need to be triaged and assigned to a reviewer labels Jul 30, 2020
@mobidev111
Copy link

actually, I think that gatsby-cli needs to upgrade to [email protected]

currently:
"update-notifier": "^3.0.1",
https://github.com/gatsbyjs/gatsby/blob/master/packages/gatsby-cli/package.json#L48

[email protected] references already the fixed [email protected] dependency.

@mobidev111
Copy link

and the renovate bot leaves the "update-notifier" dependency untouched - incorrectly:
https://github.com/gatsbyjs/gatsby/pull/25546/files#diff-fad0dd845cf69afbd0d9c70239e6fff9R48

@ascorbic
Copy link
Contributor

OK, you are correct. I misread that. I guess Renovate won't update it because it's a major version bump. I'll reopen and mark as help wanted. If somebody would like to upgrade it and test the new major version then we'd welcome a PR. I don't know how much the API has changed.

@ascorbic ascorbic reopened this Jul 30, 2020
@ascorbic ascorbic added help wanted Issue with a clear description that the community can help with. good first issue Issue that doesn't require previous experience with Gatsby labels Jul 30, 2020
@mobidev111
Copy link

Fixing this is important & urgent.

  • right now most gatsby-cli versions fail 'npm audit' (nam security audit)
  • this immediately breaks the CI pipelines of everybody using gatsby-cli
  • there are workarounds, but exceptions from the security audit are seldomly/hard to administer for obvious reasons

@herecydev
Copy link
Contributor

I'll raise a PR

@herecydev
Copy link
Contributor

There's an additional issue with devcert as it depends on an even older version of update-notifier. I'll see if I can resolve that too.

@nmastroianni
Copy link

Not sure if this helps, but I found this searching around. I'm new to this type of thing, so "grain-of-salt" is worth taking. I added the following code to my package.json and then ran yarn install. dot-prop fixed:
"resolutions": { "gatsby/**/dot-prop": ">=5.2.0" },

@herecydev
Copy link
Contributor

It is a fix by forcing dot-prop to a higher version but it can introduce issues if the consuming library isn't prepared for a later version.

With that said, for dot-prop I don't think you'll have any issues with forcing a higher version. Also you shouldn't need to wildcard it. I think the following will work better;

"resolutions": { "dot-prop": ">=5.2.0" }

@MatthewCushing
Copy link

@herecydev Unfortunately did not work for me. I'm using npm rather than yarn but I don't see why that would make a difference here.

@nmastroianni
Copy link

nmastroianni commented Aug 4, 2020

@cushmatt Unfortunately did not work for me. I'm using npm rather than yarn but I don't see why that would make a difference here.

I do believe that if you use Yarn, you have a different lock file than if you use npm. Yarn allows you to use the "resolutions" in your package.json, while npm does not. This might explain why you're not getting the same result with npm. Not sure how to do this with npm.

@sakihayashi
Copy link

yeah, I see Gatsby CLI does not update update-notifier to the latest version. I have 13 update-notifiers on package-lock.json and others have the latest version. is it a good idea to do 'npm audit fix --force' ?

@herecydev
Copy link
Contributor

gatsbybot got a little hasty there, still needs devcert to be patched

@samgermain
Copy link

Unfortunately did not work for me. I'm using npm rather than yarn but I don't see why that would make a difference here.

Same

@waleedsadek-panx
Copy link

I am also using NPM instead of Yarn, are there any workarounds for this issue?
What's stranger is both update-notifier and dot-prop latest versions are already in the package.json for devcert and gatsby-cli. Still same issue persists.
Ps. I only get this issue when I port a React app to Gatsby.

@alexbeattie42
Copy link

alexbeattie42 commented Aug 13, 2020

If you're on npm you can use this (https://github.com/rogeriochaves/npm-force-resolutions) to mimic yarn's resolutions and then use
"resolutions": { "dot-prop": ">=5.2.0" } in package.json

@waleedsadek-panx
Copy link

If you're on npm you can use this (https://github.com/rogeriochaves/npm-force-resolutions) to mimic yarn's resolutions and then use
"resolutions": { "dot-prop": ">=5.2.0" } in package.json

Thanks Alex! This solved my problem. I was stuck on this for days. You're a lifesaver!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Issue that doesn't require previous experience with Gatsby help wanted Issue with a clear description that the community can help with. type: bug An issue or pull request relating to a bug in Gatsby type: upstream Issues outside of Gatsby's control, caused by dependencies
Projects
None yet
Development

Successfully merging a pull request may close this issue.

10 participants