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 vulnerability found in fsevents >node-pre-gyp >tar #6790

Closed
kaiwen-zhang-ck opened this issue Apr 10, 2019 · 23 comments
Closed
Assignees

Comments

@kaiwen-zhang-ck
Copy link

kaiwen-zhang-ck commented Apr 10, 2019

Using Synk, I found security vulnerability from package node-pre-gyp in the package tar. This was discovered on Synk April 4 2019.
Please see Synk screenshots
Screen Shot 2019-04-10 at 9 55 58 AM

You can see details about this vulnerability here
https://snyk.io/vuln/SNYK-JS-TAR-174125

Screen Shot 2019-04-10 at 10 00 30 AM

Because, I imagine many teams will not be able to use React Scripts. What is the best way to go about this? I filed an issue with the node-pre-gyp team but it seems like to fix this we will need fsevents to be updated as well. Any ideas?

@kaiwen-zhang-ck kaiwen-zhang-ck changed the title High severity vulnerability found in tar High severity vulnerability found in fsevents -> node-pre-gyp -> tar Apr 10, 2019
@kaiwen-zhang-ck kaiwen-zhang-ck changed the title High severity vulnerability found in fsevents -> node-pre-gyp -> tar High severity vulnerability found in fsevents >node-pre-gyp >tar Apr 10, 2019
@amdp
Copy link

amdp commented Apr 11, 2019

npm i node-pre-gyp

@iansu
Copy link
Contributor

iansu commented Apr 11, 2019

There isn't really anything we can do to fix this, other than updating these dependencies when new versions are released.

@amdp
Copy link

amdp commented Apr 11, 2019

If you just install node-gyp, you'll have that issue. Installing node-pre-gyp too removes the problem.
The tar dependency is a problem of node-pre-gyp, not of node-gyp.

kswenson added a commit to concord-consortium/codap-shared-table-plugin that referenced this issue Apr 11, 2019
@cruiserkernan
Copy link

Apparently there is a version 2.0 of fsevents that does not have a dependency to node-pre-gyp. Maybe bumping the version of fsevents could fix this. I'm not sure if that would cause a breaking change to react-scripts.

@leepowelldev
Copy link

If you just install node-gyp, you'll have that issue. Installing node-pre-gyp too removes the problem.
The tar dependency is a problem of node-pre-gyp, not of node-gyp.

Sadly this doesn't work for me - still get the audit errors

@amdp
Copy link

amdp commented Apr 12, 2019

Then another option I read about is to play with package-lock.json (set all tar to updated version), and use npm ci instead of i

source: https://stackoverflow.com/questions/15806152/how-do-i-override-nested-npm-dependency-versions

hope this helps more than my previous

@levimeahan
Copy link

There isn't really anything we can do to fix this, other than updating these dependencies when new versions are released.

Given that there is a high security risk vulnerability, would it be possible to push out a minor/patch update for react-scripts that does just that?

@verde79
Copy link

verde79 commented Apr 13, 2019

@kaiwen-zhang-ck managed to get a fix https://github.com/mapbox/node-pre-gyp/issues/446

I've just updated the tar version in the package-lock.json, saved and ran npm audit again, and yeah it sorts it

@vaibhav-kona
Copy link

Is it a good practice to change package-lock.json directly? I thought we should not

@verde79
Copy link

verde79 commented Apr 15, 2019

@adred8 I'm not sure if it's good practice, seen plenty of people saying to do it, but I don't know for sure.

At any rate, I've noticed that after updating package-lock and doing an npm install, the package lock is overwritten to the vulnerable version again. My current thinking on this is that as npm audit only looks at package.json and package-lock and not node_modules, updating package-lock to a secure version of tar(>4.4.2) doesn't eliminate the insecure version in node_modules.
fsevents is an optional dependency, requiring a mac. As I'm on windows, can anyone with a mac confirm fsevents is in node_modules still?
Also unless I'm wrong because the dependency is only installed on mac, windows users could ignore the warning?

@amdp
Copy link

amdp commented Apr 15, 2019

In the article I cited they say that if you use npm ci (clean install) it shouldn't overwrite the changes you did in the .lock.json
Give it a try. And no, I don't think it is a nice practice, you are just doing it as a quick fix.

@erwanriou
Copy link

Same issue here on ubuntu.. I would be happy of this small fix in the react-script since we are now dealing with high vulnerabilities...

@verde79
Copy link

verde79 commented Apr 16, 2019

@amdp I shall give it a try. I wasn't doing it as a quick fix, I'm pretty new to dealing with dependency trees and trying to get my head around doing things well and understanding why

@amdp
Copy link

amdp commented Apr 16, 2019

Keep in mind I am a noob as well so I am -maybe- just good at explaining what other people were showing as a solution

@verde79
Copy link

verde79 commented Apr 16, 2019

kaiwen had posted the answer in another thread, which I shared above. I was trying to point people with the same issue to his solution, and confirm that it worked for me (even though it later didn't seem to be. Hopefully get time tomorrow to look at it again!

@HarisSpahija
Copy link

Updating all "tar" locations in package-lock.json to use "tar": "^4.4.8" manually solved my issue.

@glrodasz
Copy link

@HarisSpahija But how did you do that if node-gyp requires ^2.2?

@sridevs
Copy link

sridevs commented Apr 19, 2019

Updating all "tar" locations in package-lock.json to use "tar": "^4.4.8" manually solved my issue.

That solves the issue, but its not a good practice to directly update dependencies in package-lock.json.

@sridevs
Copy link

sridevs commented Apr 19, 2019

There isn't really anything we can do to fix this, other than updating these dependencies when new versions are released.

Exactly, you need to update the versions, especially all the tar version under react-scripts package to 4.4.2 and above to remediate Arbitrary File Overwrite vulnerability.

@iansu
Copy link
Contributor

iansu commented Apr 19, 2019

Yes, like I said, we will update these packages. A new version of fsevents has been released and there is already a PR updating our dependencies.

@ereztdev
Copy link

we there yet? ;P Just came across it now.

@ianschmitz
Copy link
Contributor

This should be fixed by our 3.0 release. Can you confirm?

@svenlaater
Copy link

@ianschmitz yes, 3.0 is good.

@lock lock bot locked and limited conversation to collaborators Apr 29, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests