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

Update y18n to fix Prototype-Pollution (High) (CVE-2020-7774) #1102 #1106

Closed
thenengah opened this issue May 24, 2021 · 16 comments
Closed

Update y18n to fix Prototype-Pollution (High) (CVE-2020-7774) #1102 #1106

thenengah opened this issue May 24, 2021 · 16 comments

Comments

@thenengah
Copy link
Contributor

High risk security issue with nconf; just need to bump it

https://nodejs.org/en/blog/vulnerability/april-2021-security-releases/#npm-upgrade-update-y18n-to-fix-prototype-pollution-high-cve-2020-7774

#1105

@thenengah
Copy link
Contributor Author

@indexzero Do you think it would be possible to get this fixed?

@thenengah
Copy link
Contributor Author

Ah, looks like you guys are really busy. Unfortunately this breaks our SCA scan for HIGH risk vulnerabilities so we are having to work off my forked repo :(

@kibertoad
Copy link
Contributor

I'll take a look today, sorry for the delay.

@thenengah
Copy link
Contributor Author

@kibertoad thanks for the response. I'm here to help out in whatever way.

@kibertoad
Copy link
Contributor

kibertoad commented May 28, 2021

@thenengah Problem with submitted PR is that it break compatibility with older (but still very widely used) Node versions. Considering that forever is a legacy software primarily used by legacy systems, this is not an ideal solution.

@kibertoad
Copy link
Contributor

@thenengah For yargs dependency one possible approach would be to use https://github.com/jorgebucaran/getopts. Based on what I see in their sources, it should work on Node 6+.
Would you be open to submit a PR which would replace yargs with getopts?

@kibertoad
Copy link
Contributor

kibertoad commented May 28, 2021

@thenengah For nconf, migration to configstore v. 4.0.0 could be an option.

@thenengah
Copy link
Contributor Author

#1110

^ PR to replace yargs with getopts

I'm looking into configstore to replace nconf.

@kibertoad

Do you prefer one MR for both of these updates, or does it not matter?

@kibertoad
Copy link
Contributor

@thenengah Two PRs would be preferable! Thanks.

@kibertoad
Copy link
Contributor

@thenengah If configstore doesn't work out, there should be other alternatives. there is dotenv-json, but it wasn't updated in ages. There should be more.

@thenengah
Copy link
Contributor Author

#1112

^ PR to replace nconf with configstore.

@thenengah
Copy link
Contributor Author

@kibertoad

I've tested #1110 and #1112 in our staging environment. The vulnerability is gone our unit tests pass, e2e tests pass, SCA scan now passes, and app is running in our stage environment.

Hopefully we can get these PRs in and bump a new npm? Let me know if there is anything I can do to help.

@kibertoad
Copy link
Contributor

@thenengah Merged getopts, left a comment for the configstore migration.

@kibertoad
Copy link
Contributor

Thank you! Will release new version today.

@thenengah
Copy link
Contributor Author

@kibertoad 🔓 🏆

Thanks for your response time and help on this! Together we were able to remove the following vulnerability:
https://nodejs.org/en/blog/vulnerability/april-2021-security-releases/#npm-upgrade-update-y18n-to-fix-prototype-pollution-high-cve-2020-7774

@kibertoad
Copy link
Contributor

@thenengah Thank you for your contributions! This is now released as 4.0.0
Note that there is still a known vulnerability coming through transitive dependency of flatiron, which I would also be very happy to remove from dependencies :-/.

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

No branches or pull requests

2 participants