facebook / create-react-app

Set up a modern web app by running one command.
https://create-react-app.dev
MIT License
102.73k stars 26.85k forks source link

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

Closed jissv closed 4 years ago

jissv commented 4 years ago

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 commented 4 years ago

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

jissv commented 4 years ago

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 commented 4 years ago

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 commented 4 years ago

@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 commented 4 years ago

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 commented 4 years ago

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

gaearon commented 4 years ago

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 commented 4 years ago

FYI. It's already been filed: https://github.com/bholloway/adjust-sourcemap-loader/issues/16

nayaks2019 commented 4 years ago

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

thats great.waiting for this to release

mrwensveen commented 4 years ago

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 commented 4 years ago

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

gaearon commented 4 years ago

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 commented 4 years ago

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

nayaks2019 commented 4 years ago

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 commented 4 years ago

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 commented 4 years ago

@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 commented 4 years ago

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 commented 4 years ago

@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 commented 4 years ago

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 commented 4 years ago

Thanks, this context is super helpful.

gaearon commented 4 years ago

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).