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

Security Vulnerabilities detected in the depedency packages requires an update #9815

Closed
jissv opened this issue Oct 16, 2020 · 21 comments
Closed

Comments

@jissv
Copy link

jissv commented Oct 16, 2020

Hi, for one of our projects after upgrading react-scripts to the latest version (reacts-scripts@3.4.3), the Veracode static code analysis tool points out that few libraries are vulnerable to uninitialized buffer allocation attacks, prototype pollution,These libraries are given below

ajv@6.11.0 is vulnerable to prototype pollution. By upgrading this to a version >=6.12.4 this issue can be resolved
websocker-driver@0.6.5 is vulnerable to uninitialized buffer allocation attacks. By upgrading this to a version >=0.7.1 this issue can be resolved

Is there any plan to upgrade these packages to improve the security? If yes, could you please update by when these changes could be implemented. Any quick help/support you could provide on this would be much appreciated.

@gaearon
Copy link
Contributor

gaearon commented Oct 16, 2020

As far as I can tell, there are no actual vulnerabilities here. If you link to reports I can confirm that.

@jissv
Copy link
Author

jissv commented Oct 16, 2020

Hi Dan, Thank you for your quick reply. Unfortunatly I would not be able to share the full report. But below are the issues e issues stated in the report

ajv@6.11.0
https://nvd.nist.gov/vuln/detail/CVE-2020-15366
Prototype Pollution: ajv is vulnerable to prototype pollution. The vulnerability exists as it does not prevent the overwrite of proto properties.

websocker-driver@0.6.5
https://sca.analysiscenter.veracode.com/vulnerability-database/security/sca/vulnerability/sid-20396/summary
Uninitialized Buffer Allocation: websocker-driver is vulnerable to uninitialized buffer allocation attacks. The library contains an uninitialized memory allocation when handling a large number, which can allow a malicious user to gain access to sensitive information or crash the application.

image

image

@gaearon
Copy link
Contributor

gaearon commented Oct 16, 2020

Neither of these dependencies is used in the production builds, they're development-only. So they don't have any actual impact on the security of your application.

We're cutting CRA 4.x soon so you can try switching to react-scripts@next betas to resolve this (but watch out for breaking changes).

@nayaks2019
Copy link

nayaks2019 commented Oct 19, 2020

@gaearon -I am also getting Snyk security testing issue with
react-scripts@3.4.3 > resolve-url-loader@3.1.1 > adjust-sourcemap-loader@2.0.0 > object-path@0.11.4
and also with
react-scripts@4.0.0-next.98 > resolve-url-loader@3.1.1 > adjust-sourcemap-loader@2.0.0 > object-path@0.11.4
the issue is with object-path@0.11.4 it will be fixed once we upgrade it to object-path@0.11.5

can you please update the version of object-path@0.11.4. to object-path@0.11.5 inside react-scripts@3.4.3 or react-scripts@4.0.0-next.98. ?

@gaearon
Copy link
Contributor

gaearon commented Oct 19, 2020

These are transitive dependencies, so they're not ours to update. You can file this with adjust-sourcemap-loader and ask them to cut a patch. Again, there is no real security issue here. You are spending your time on false positives.

@nayaks2019
Copy link

@gaearon -Can you please elaborate more on "You can file this with adjust-sourcemap-loader and ask them to cut a patch"

@gaearon
Copy link
Contributor

gaearon commented Oct 19, 2020

If the "vulnerable" (and again, there's no actual vulnerability — you are chasing a ghost) package is object-path, the package that needs to update its version is the one that uses it. In this case, according to your comment, it is adjust-sourcemap-loader (and one step further, resolve-url-loader). So these are the packages that you need to find on GitHub and ask their maintainers to cut a patch release. Then your app can automatically update to them.

@WayneEllery
Copy link

FYI. It's already been filed: bholloway/adjust-sourcemap-loader#16

@nayaks2019
Copy link

FYI. It's already been filed: bholloway/adjust-sourcemap-loader#16

thats great.waiting for this to release

@mrwensveen
Copy link

mrwensveen commented Oct 20, 2020

If the "vulnerable" (and again, there's no actual vulnerability — you are chasing a ghost) package is object-path, the package that needs to update its version is the one that uses it. In this case, according to your comment, it is adjust-sourcemap-loader (and one step further, resolve-url-loader). So these are the packages that you need to find on GitHub and ask their maintainers to cut a patch release. Then your app can automatically update to them.

False positives like this are arguably more dangerous. When we tell people to ignore reported vulnerabilities, actually dangerous vulnerabilities get ignored as well. That's not a bug of create-react-app, obviously, but something I wanted to react to nonetheless.

@nayaks2019
Copy link

nayaks2019 commented Oct 20, 2020

@gaearon adjust-sourcemap-loader is upgraded to 3.0.0 with security fix seems.Can you please upgrade react-scripts ?

@gaearon
Copy link
Contributor

gaearon commented Oct 20, 2020

This is not how npm works. We can’t “update” to some transitive dependency’s bump. What needs to happen here is a patch release (not a major!) for the affected dependency. Then things would sort themselves out automatically thanks to semver.

@gaearon
Copy link
Contributor

gaearon commented Oct 20, 2020

And again, let’s not call this a “security fix” because there is no actual security problem in this case.

@nayaks2019
Copy link

I am not a security expert.but we are getting High Severity
✗ Prototype Pollution [High Severity][https://snyk.io/vuln/SNYK-JS-OBJECTPATH-1017036] in object-path@0.11.4
introduced by react-scripts@3.4.3 > resolve-url-loader@3.1.1 > adjust-sourcemap-loader@2.0.0 > object-path@0.11.4
This issue was fixed in versions: 0.11.5

without react-scripts update we cannot move forward

@lirantal
Copy link

Snyk by default doesn't report on security issues in devDependencies to reduce the noise to signal ratio, just as @gaearon is saying - some times these vulnerabilities in devDependencies don't make a lot of sense since these aren't making it into production code. Good examples of these are security issues in packages like webpack, jest, etc.

@nayaks2019 perhaps you can move those dependencies added by CRA to devDependencies? such as @testing-library/*, react-scripts, etc. @gaearon I think this will be a good default in the future for CRA. Would such a change be ok or is this going to break anything in the tooling for you?

@nayaks2019 another thing to consider is that Snyk allows you to ignore vulnerabilities (I think other tools, maybe like npm audit but not sure, take a look at their docs). See Snyk CLI for ignore or the Snyk UI instructions.

@gaearon
Copy link
Contributor

gaearon commented Oct 20, 2020

@gaearon I think this will be a good default in the future for CRA. Would such a change be ok or is this going to break anything in the tooling for you?

Sounds totally fine. I had no idea there is an actual semantic meaning to it from Snyk's perspective!

@lirantal
Copy link

Sounds totally fine. I had no idea there is an actual semantic meaning to it from Snyk's perspective!

Cool! I think this will really help a lot to bring the noise down for CRA for such issues being opened, at least for those using Snyk or making sure their tooling is ignoring devDeps by default as the snyk CLI does.

@gaearon
Copy link
Contributor

gaearon commented Oct 20, 2020

@lirantal

Snyk by default doesn't report on security issues in devDependencies to reduce the noise to signal ratio,

Is npm audit a different thing from Snyk? I don't understand the exact relationships here but I do get npm audit errors for devDependencies. I suspect that's where people see them primarily.

@lirantal
Copy link

Yes it is :-)

Snyk is a security platform, CLI tool, and maintains its own vulnerability database at https://snyk.io/vuln?type=npm which has a broader coverage than npm. And so, when users get alerts in the form of the CLI tooling reports, or via a GitHub pull request this is all based on Snyk's own database. There is some overlap with npm and others since all of these tooling works by tracking the open and public database sets like NVD.

npm audit is a tool maintained by the npm team (its part of the npm cli), and has its own security advisories database.

An out of the box CRA project which I just scaffolded provides the following security report from Snyk, indeed showin that there are many issues that don't have a direct upgrade, and can't be easily addressed by their maintainers:

image

However, I applied the change I suggested to move the relevant packages into devDeps

image

and a follow-up snyk test now shows 0 vulnerabilities:

image

If someone really wanted to, they can issue a snyk test --dev and it'll report on devDependencies as well.
Unfortunately, reporting devDeps is a default for npm (maybe it changed with the new npm 7 release but I haven't yet followed up on what's there) and so this is a bit noisy and users end up opening such issues.

I'm happy to support where I can. Don't be shy to tag me in this or future issues if I can help.

P.S.
Full disclosure in case this wasn't apparent before, I'm wearing two hats: I'm a developer advocate at Snyk, as well as a member of the Node.js foundation's security working group.

@gaearon
Copy link
Contributor

gaearon commented Oct 20, 2020

Thanks, this context is super helpful.

@gaearon
Copy link
Contributor

gaearon commented Oct 20, 2020

I bumped just resolve-url-loader alone in react-scripts@3.4.4.

For the other issues, please move react-scripts to devDependencies (if you use Snyk).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants