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

Transitive dependency to vulnerable node-forge version #2671

Closed
wvanderdeijl opened this issue Oct 5, 2020 · 11 comments
Closed

Transitive dependency to vulnerable node-forge version #2671

wvanderdeijl opened this issue Oct 5, 2020 · 11 comments

Comments

@wvanderdeijl
Copy link
Contributor

wvanderdeijl commented Oct 5, 2020

Using the latest version of firebase-tools (8.11.2) will introduce a vulnerable version of node-forge to your project which throws npm audit warnings about a high severity vulnerability. This raises all sorts of red flags, including internal security monitoring at our enterprise.

This would probably require updating the dependencies @google-cloud/pubsub, google-auth-library and google-gax. This would mean dropping support for node v8 and judging from #2619 you are not enthusiastic to do so. Please reconsider this as node 8 has been end of life since 2019-12-31

[REQUIRED] Environment info

firebase-tools: 8.11.2
Platform: macOs 10.15.6

[REQUIRED] Test case

wilfred@wMac2019 ftools % npm init
...
Is this OK? (yes) 
% npm i firebase-tools
npm WARN deprecated [email protected]: request has been deprecated, see https://github.com/request/request/issues/3142
npm WARN deprecated [email protected]: this library is no longer supported

> [email protected] install /redacted/ftools/node_modules/re2
> install-from-cache --artifact build/Release/re2.node --host-var RE2_DOWNLOAD_MIRROR || npm run rebuild

Trying https://github.com/uhop/node-re2/releases/download/1.15.5/darwin-x64-72.br ...
Writing to build/Release/re2.node ...
Done.

> [email protected] postinstall /redacted/ftools/node_modules/protobufjs
> node scripts/postinstall

npm notice created a lockfile as package-lock.json. You should commit this file.
npm WARN [email protected] requires a peer of bufferutil@^4.0.1 but none is installed. You must install peer dependencies yourself.
npm WARN [email protected] requires a peer of utf-8-validate@^5.0.2 but none is installed. You must install peer dependencies yourself.
npm WARN [email protected] No description
npm WARN [email protected] No repository field.

+ [email protected]
added 603 packages from 379 contributors and audited 603 packages in 17.674s

14 packages are looking for funding
  run `npm fund` for details

found 4 high severity vulnerabilities
  run `npm audit fix` to fix them, or `npm audit` for details
% npm audit
                                                                                
                       === 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 in node-forge                            │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Package       │ node-forge                                                   │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Patched in    │ >= 0.10.0                                                    │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Dependency of │ firebase-tools                                               │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Path          │ firebase-tools > @google-cloud/pubsub > google-auth-library  │
│               │ > gtoken > google-p12-pem > node-forge                       │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ More info     │ https://npmjs.com/advisories/1561                            │
└───────────────┴──────────────────────────────────────────────────────────────┘
┌───────────────┬──────────────────────────────────────────────────────────────┐
│ High          │ Prototype Pollution in node-forge                            │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Package       │ node-forge                                                   │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Patched in    │ >= 0.10.0                                                    │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Dependency of │ firebase-tools                                               │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Path          │ firebase-tools > @google-cloud/pubsub > google-gax >         │
│               │ google-auth-library > gtoken > google-p12-pem > node-forge   │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ More info     │ https://npmjs.com/advisories/1561                            │
└───────────────┴──────────────────────────────────────────────────────────────┘
┌───────────────┬──────────────────────────────────────────────────────────────┐
│ High          │ Prototype Pollution in node-forge                            │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Package       │ node-forge                                                   │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Patched in    │ >= 0.10.0                                                    │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Dependency of │ firebase-tools                                               │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Path          │ firebase-tools > google-auth-library > gtoken >              │
│               │ google-p12-pem > node-forge                                  │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ More info     │ https://npmjs.com/advisories/1561                            │
└───────────────┴──────────────────────────────────────────────────────────────┘
┌───────────────┬──────────────────────────────────────────────────────────────┐
│ High          │ Prototype Pollution in node-forge                            │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Package       │ node-forge                                                   │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Patched in    │ >= 0.10.0                                                    │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Dependency of │ firebase-tools                                               │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Path          │ firebase-tools > google-gax > google-auth-library > gtoken > │
│               │ google-p12-pem > node-forge                                  │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ More info     │ https://npmjs.com/advisories/1561                            │
└───────────────┴──────────────────────────────────────────────────────────────┘
found 4 high severity vulnerabilities in 603 scanned packages
  4 vulnerabilities require manual review. See the full report for details.

[REQUIRED] Steps to reproduce

see test case

[REQUIRED] Expected behavior

No vulnerabilities reported by npm audit

[REQUIRED] Actual behavior

npm audit reports vulnerabilities after a fresh install of firebase-tools

@google-oss-bot
Copy link
Contributor

This issue does not seem to follow the issue template. Make sure you provide all the required information.

@samtstern
Copy link
Contributor

@wvanderdeijl thank you for all of the detail here. As you noted, we'd have to drop Node 8 support to fix this issue (via google-auth-library). We do plan to do this, but only when Firebase developers can no longer deploy Cloud Functions in the Node 8 environment. That will be in 2021.

@bkendall do you have a canonical "Drop Node 8" issue or milestone where we could collect issues like this?

@wvanderdeijl
Copy link
Contributor Author

The official firebase functions documentation states that deployment of node 8 functions hasn’t been allowed since February 2020:

Deployment of Node.js 8 functions will no longer be allowed after February 15, 2020. Then, executions of already-deployed Node.js 8 functions will stop after March 15, 2021

This seems to indicate you can no longer deploy node8, but existing functions would continue to run until March 2021. If deployment is no longer allowed, what is the reason to keep node8 compatibility for Firebase-tools ?

@wvanderdeijl
Copy link
Contributor Author

After reading the docs at https://firebase.google.com/docs/functions/manage-functions#upgrade-node10 again I feel that the statement about not allowing node8 deployments should have mentioned February 2021, not February 2020:

Node.js 8 (deprecated as of June 8, 2020) Deployment of Node.js 8 functions will no longer be allowed after February 15, 2020. Then, executions of already-deployed Node.js 8 functions will stop after March 15, 2021. If you have deployed functions to the Node.js 8 runtime, you're recommended to upgrade to the Node.js 10 runtime.

If deployments are allowed until February 2021 I understand the desire to keep node8 compatibility. But it is a shame we will run into (possibly many) security warnings until then. I guess you don’t want to go the route of jumping a major version of firebase-tools and keep both major versions supported for now; the existing major version for node8 and the higher major version for node10+ ?

@samtstern
Copy link
Contributor

@wvanderdeijl thanks for pointing out that docs error, you're right it should be 2021 and that's why we're keeping compatibility.

We considered having two simultaneous major versions but in the end it's just too much effort and confusion. Freezing the Node 8 CLI isn't really fair for non-Functions users and backporting features to the Node 8 tree would become more and more difficult over time. Plus our docs about how to install the CLI would be very confusing.

The security warnings are definitely scary however the vast majority of them do not affect firebase-tools when used from the command line (usage as a Node module is another story).

We're keeping an open mind on this one though. If any specific security warning means we're putting Firebase developers at risk we could revisit this decision and cut off CLI support for Node 8 early.

@samtstern
Copy link
Contributor

For anyone following along here this blog post to be a good explanation of what a prototype pollution vulnerability is.

It should be extremely hard (maybe impossible) for anyone to exploit this type of vulnerability when using the firebase CLI but please take extra caution around user input when using firebase-tools as a Node dependency in your code.

@MrAndrew
Copy link

Any updates on this? As of firebase-tools 9.14.0 npm audit still throws this warning. I see above that Firebase should have retired node 8 support as of this past February.

@samtstern
Copy link
Contributor

@MrAndrew have you tried removing your package-lock.json and reinstalling?

@MrAndrew
Copy link

Oh yea, that did the trick. Thanks so much.

@dotdoom
Copy link

dotdoom commented Feb 13, 2022

That didn't help me, I can still reproduce 4 high severity vulnerabilities as stated in the 1st comment in this issue.

@bkendall
Copy link
Contributor

I'm cleaning up the current audit issues for our dependencies, but I'm going to end up closing this issue. We get several variations on audit issues on a regular basis, and since this particular one is so old, I'm going to let it be closed. I do think that all the original issues in the 1st comment have been taken care of

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

7 participants