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

build: use dot-prop-legacy patch to resolve CVE-2020-8116 #1682

Closed
wants to merge 1 commit into from
Closed

build: use dot-prop-legacy patch to resolve CVE-2020-8116 #1682

wants to merge 1 commit into from

Conversation

cmdcarini
Copy link

Me again! This updates the version of configstore and dot-prop to resolve the vulnerability present in the existing version's dot-prop dependency. This resolves #1584. This utilizes the changes that @ruyadorno created based on this fork and an update to configstore which also includes this patch. This time while retaining Node v6 compatibility.

References

Closes #1584
https://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2020-8116

@cmdcarini cmdcarini requested a review from a team as a code owner August 16, 2020 13:48
@cmdcarini cmdcarini marked this pull request as draft August 16, 2020 18:52
@cmdcarini
Copy link
Author

Waiting for downstream dependency dot-prop-legacy to be swapped for the official dot-prop

@cmdcarini
Copy link
Author

@ruyadorno looks like just yeoman/update-notifier#187 needs to happen and then npm/cli should be able to be updated to leverage the new dot-prop@legacy

@ruyadorno
Copy link
Contributor

Thanks @cmdcarini! ❤️

Usually updating deps is a part of our regular Release process so I'm sorry I didn't picked up your contribution 😅 normally I try to at least give credit for folks who poke us about updating a dep but in this case the fact that the PR was still in Draft mode made it slip out of my radar.

That said, the update went out yesterday and the dot-prop vulnerability should no longer be a problem in v6.14.8.

Thanks again!

@ruyadorno ruyadorno closed this Aug 18, 2020
@mtrepanier
Copy link

@ruyadorno sorry but in 6.14.8 the faulty dot-prop version is still in that version

https://github.com/npm/cli/blob/v6.14.8/package-lock.json#L1175

@ruyadorno
Copy link
Contributor

@mtrepanier good catch, turns out the v6.14.8 tag wasn't properly published to our upstream repo and thus showing wrong info.

Just fixed that! Thanks for the heads up! 😄

@kamal94
Copy link

kamal94 commented Sep 1, 2020

I still don't see an official release with this fix. Has this code been merged anywhere? On master there are still multiple references to the vulnerable dot-prop package version.

@ruyadorno
Copy link
Contributor

@kamal94 in the latest v6.14.8 we updated it to dot-prop@4.2.1 which contains the fix.

The link you posted to the package-lock.json is also showing up that it's using this fixed 4.2.1 version.

@kamal94
Copy link

kamal94 commented Sep 2, 2020

Thank you for clarifying :) For whatever reason, I had assumed that we needed to upgrade the minor/major version and didn't notice they had backported the fix with a patch release.

Update for clarity:
Looks like we had issues with our installation in the docker image. As of right now, installing npm via apt on ubuntu doesn't download the latest release of npm which includes the fix. So we had to upgrade npm after installing it.

The git diff we needed:

 RUN curl -sL https://deb.nodesource.com/setup_10.x | bash - \
  && apt update \
  && apt install -y nodejs \
+ && npm -g upgrade npm \
  && rm -rf /var/lib/apt/lists \
  && node -v && npm -v

Before using npm -g upgrade npm:

npm --version
6.14.6

After using npm -g upgrade npm:

npm --version
6.14.8

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.

[BUG] Vulnerability present in version of dot-prop used by npm
4 participants