facebook / create-react-app

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

Help, `npm audit` says I have a vulnerability in react-scripts! #11174

Open gaearon opened 3 years ago

gaearon commented 3 years ago

npm audit is broken for front-end tooling by design

Bad news, but it's true. See here for a longer explanation.

If you think you found a real vulnerability in react-scripts

If you know that it affects CRA users because you understand what the vulnerability is, report it here as soon as possible.

If you're not sure but your CI is failing or you're worried about what npm audit tells you, keep reading.

Do not file new issues based on npm audit if you don't 100% understand the problem. They will be closed (see why below). If you really need to discuss it, reply in this thread instead.

npm audit says there's a warning about vulnerabilities in my project

Open package.json. You will find this:

  "dependencies": {
    "react": "^17.0.2",
    "react-dom": "^17.0.2",
    "react-scripts": "4.0.3"
  }

Take react-scripts and move it to devDependencies (if you don't have it, create it):

  "dependencies": {
    "react": "^17.0.2",
    "react-dom": "^17.0.2"
  },
  "devDependencies": {
    "react-scripts": "4.0.3"
  },

Then, ensure you run npm audit --production rather than npm audit.

This will fix your warnings.

But isn't this just ignoring the problem?

No.

Create React App is a build tool. In other words, it doesn't produce a running Node application. It runs at the build time during development, and produces static assets.

However, npm audit is designed for Node apps so it flags issues that can occur when you run actual Node code in production. That is categorically not how Create React App works.

This means that the overwhelming amount of "vulnerability" reports we receive for transitive dependencies are false positives. Despite literally a hundred issues with thousands of comments about npm audit warnings in react-scripts, throughout the years not a single one of them (to the best of our knowledge) has ever been a real vulnerability for CRA users.

This is a huge waste of everyone's time. Mostly of yours, but of ours too.

But I still see these warnings when creating a new project or running npm install

Yes, unfortunately that's how npm works since v6. You can bring it up with npm. If enough people complain, maybe they'll rethink this decision. It is unfortunately actively hostile to build tooling.

Note that you can run npm install --no-audit to suppress them.

I know the transitive dependency has a fix, how can I try it?

If you already know that some-library@x.y.z has the fix that you need, but react-scripts hasn't yet updated to it, you can try your luck using that version forcefully. With Yarn, you can do it using resolutions. With npm, you might need to wait for overrides or npm audit fix overrides integration to land first (it's not implemented yet). You can also try npm-force-resolutions.

But can't a build tool have vulnerabilities, too?

Yes, in principle.

The few times there was an actual vulnerability, it was reported separately, and we released patches as soon as it was possible. You can always report real vulnerabilities here, but please do this if you understand the difference between a real vulnerability and a false positive. For example, a "Regex DDOS attack" can never be a real vulnerability for a development-time tool. If you're not sure, you're welcome to ask in this thread, but please keep it brief and to the point so that the thread doesn't become unreadable.

Really, the worst problem is that when there is a real attack poisoning the build toolchain, we won't know about it because it will be buried underneath the 99.9% of false positives.

jacobbroughton commented 3 years ago

Can't blame people for being concerned, big red '96 high risk vulnuerabilities' is sure to get everyone's attention. Thanks for the update though.

gaearon commented 3 years ago

Yeah it's pretty frustrating. And also understandable because many people don't know what things like "regex ddos" means or even how webapps work in general. So I understand that it looks scary.

bcagarwal commented 3 years ago

@gaearon Thanks for the update. However, if we have seen/ are seeing there are hundreds of issues with thousands of comments on those 96 vulnerabilities (as you said 'false positives'), this should have been fixed at the very first place. You must agree that people must have wasted their time as well after seeing those vulnerabilities. Also, there are no documentation to categorize those (at least I am not aware of). So, not everyone would know if they are false positives or real vulnerabilities.

Thank you again for your help. Hope this should be fixed soon so that people would not raise same issue again and again.

gaearon commented 3 years ago

However, if we have seen/ are seeing there are hundreds of issues with thousands of comments on those 96 vulnerabilities (as you said 'false positives'), this should have been fixed at the very first place

I'm not sure what you're suggesting. These are not issues with Create React App, but with low-level dependencies of transitive packages. Like I said, they are almost always irrelevant because they don't make sense in the context of a build tool. So there is nothing to "fix". Eventually the low-level dependencies update, and we pull in the updates in the next update. But it's a lot of churn and unnecessary release work just to work around the warnings which are not relevant.

You must agree that people must have wasted their time as well after seeing those vulnerabilities.

I am referring to people's time, not to my time. It is a waste of time for our users. This is why I made this issue for a centralized explanation. We'll also move react-scripts to devDependencies so that these warnings aren't reported by default.

It wasn't our idea to show these warnings. npm added this without considering the ecosystem impact on build tools.

Also, there are no documentation to categorize those (at least I am not aware of).

I don't know what kind of documentation we could provide here. Each CVE is annotated with an explanation of the type of the mistake (e.g. "prototype pollution" or "regex ddos"). These are generally well-described if you look for information about those. But we can't provide you some automated way to understand which ones affect a build tool. To understand this, you need to have an idea of how build tools work, and how the dependency is used. This isn't something we can teach in a day, but if you research each issue yourself for a little bit, you will be able to figure it out. If not, we can help in this thread.

Hope this should be fixed soon so that people would not raise same issue again and again.

I don't know what you want to be fixed. The way npm audit works is fundamentally at odds with the way build tools work. So we'll keep having this issue. But hopefully the move to devDependencies (as this thread suggests) will make it less prominent.

frankthoeny commented 3 years ago

What if the low-level dependencies of transitive packages are deprecated and there is no fix until those low-level dependencies are updated?

gaearon commented 3 years ago

What if the low-level dependencies of transitive packages are deprecated and there is no fix until those low-level dependencies are updated?

My question is what are you trying to fix, precisely? If the issue is real and affects CRA users, then we'll need to work with the packages up the tree to find who can solve the issue. If the issue does not affect CRA users, then it's up to you how you want to approach it. I don't think it makes sense for the CRA maintainers to solve issues that are out of scope of CRA's usage.

If all you want to fix are npm audit warnings, then the first post in this thread explains how. (Move react-scripts to devDependencies.)

ako-v commented 3 years ago

Hi. I moved react-scripts to devDependencies as you said, but it does not solve the reporting problem, and I still get npm audit warnings. thank you for your help.

ptamarit commented 3 years ago

Could react-scripts use caret ranges for all its dependencies, or is it too idealistic to assume that the maintainers of these dependencies respect semantic versioning?

gaearon commented 3 years ago

I moved react-scripts to devDependencies as you said, but it does not solve the reporting problem, and I still get npm audit warnings.

Yes, you're right. It appears that it's also necessary to use npm audit --production rather than npm audit. I amended the original post.

Unfortunately, that probably means that even changing the default won't fix the warnings that people see creating a new project. This is something we (probably?) can't fix without npm or hijacking console output. (Edit: https://github.com/facebook/create-react-app/pull/11176 may help.)

Could react-scripts use caret ranges for all its dependencies, or is it too idealistic to assume that the maintainers of these dependencies respect semantic versioning?

The problem is usually with deeply transitive dependencies, so carets in the middle of the tree usually take care of this anyway. It is pretty risky to use carets at the react-scripts level because it's an integration package, and so it is much easier for something to break accidentally even if a dependency released a patch. It's also not going to help realistically in cases where the transitive dependency fix comes with a major bump, and everything in the middle has to be upgraded.

bcagarwal commented 3 years ago

To understand this, you need to have an idea of how build tools work, and how the dependency is used. This isn't something we can teach in a day, but if you research each issue yourself for a little bit, you will be able to figure it out. If not, we can help in this thread.

Not everyone is an expert on how the build tools work. Would definitely love to know how CRA build tool works. Appreciate any help.

gaearon commented 3 years ago

@bcagarwal I empathize with this but I really don't know what we should be doing here. I feel out of my depth. npm added these warnings without consulting or working with the build tool ecosystem, and now an untold number of person-years is being spent chasing this security theater. I am beyond frustrated by this, as I imagine you are, but I don't know who and how can solve this.

gaearon commented 3 years ago

As a first step, this might (if it works) at least reduce the confusion for newly created projects: https://github.com/facebook/create-react-app/pull/11176.

gaearon commented 3 years ago

I'll also be reaching out to our contacts at Node/npm to see how we can solve this at the ecosystem level.

rhalaly commented 3 years ago

I think you should consider putting the react-scripts in the devDependencies automatically for new projects as part of create-react-app templates (JS + TS)

rhalaly commented 3 years ago

Create React App is a build tool. In other words, it doesn't produce a running Node application. It runs at the build time during development, and produces static assets.

At least, for me, this is the reason that I'm too sensitive for vulnerabilities in react-scripts...

I understand what you mean, that Create React App is a build tool so most of the audit vulnerabilities are irrelevant since this package is used in build time only.

But there's no way that vulnerability in one of the react-scripts dependencies going to cause a vulnerability in the "produced static assets"?

For example, in the list of closed referenced issues, there are some vulnerabilities related to css-what. For people like me, which are not deeply familiar with how react-scripts works, it sounds suspicious: "react-scripts is a build tool, so it not using any CSS for itself. Is this vulnerability affects the static assets that it produces for me?"

By turning off the audit for react-scripts (devDependencies + npm audit --production) we will not receive any audit vulnerabilities related to react-scripts and any other devDependencies packages. Again, I understand your point and I think that most of the audit vulnerabilities for react-scripts are irrelevant. But, if there is some chance (even the tiniest one) that vulnerability in one of the react-scripts dependencies going to cause a vulnerability in the "produced static assets", we will miss it! Can you confirm that the chance of that is totally zero?

gaearon commented 3 years ago

But there's no way that vulnerability in one of the react-scripts dependencies going to cause a vulnerability in the "produced static assets"?

In theory, yes, of course. In practice, if an audit produces 99.9% false positives, I think it’s fair to say that it doesn’t do its job. When there’s a real issue it gets reported through other channels because each parent package learns about it from users who flagged it. So it would reach us anyway and then we would release a fix. As already happened the couple of times there was an actual vulnerability.

gaearon commented 3 years ago

Can you confirm that the chance of that is totally zero?

That is not what I’m saying. I’m not saying the risk of using software is zero. If you don’t want the risk of using third-party code, you shouldn’t use third-party code at all. You can write all the code of your application (including all tooling) yourself.

When you use code written by other people you assume some of that risk. My point isn’t that using build tools is risk-free. My point is that npm audit is an inadequate mechanism to surface these issues. If I may use an analogy, a person might have a high temperature, but you don’t want to measure it with a faulty thermometer that always reports high temperature. That’s the situation we’re in now.

rhalaly commented 3 years ago

But there's no way that vulnerability in one of the react-scripts dependencies going to cause a vulnerability in the "produced static assets"?

In theory, yes, of course. In practice, if an audit produces 99.9% false positives, I think it’s fair to say that it doesn’t do its job. When there’s a real issue it gets reported through other channels because each parent package learns about it from users who flagged it. So it would reach us anyway and then we would release a fix. As already happened the couple of times there was an actual vulnerability.

Thank you!

But I'm just wondering loudly, putting the react-scripts under devDependencies and use npm audit --production hides all the vulnerabilities that audit finds, which, as you said, "99.9% false positives", and I agree with that.

Again, I'm totally with you about the false positive of npm audit. npm audit just reports the packages with vulnerabilities, but it doesn't know how you use that package - if the vulnerability has even been used in your source code. For example, we are using jest for unit testing - with jest I'm totally fine to move it to devDependencies and use npm audit --production since it is 100% false positive - audit doesn't know that jest is not been used in production and the vulnerability is irrelevant...

But, react-scripts is not jest since it generates the production assets, so it has a responsibility. If you have a real vulnerability in react-scripts (directly or in any of your sub-dependencies) that affects the produced static assets for production, that, as you said, already happened in the past, we will not see it... So, it hides the 99.9% false positive, which is excellent, but it also hides the 0.01% true positive - and that's the problem I see.

I don't want to miss any real vulnerability in react-scripts, even if it just has "0.01% chance" for that. I prefer to stop my CI/CD pipelines and hold down the releases for a while, and not publish assets with vulnerabilities in production - better be safe than sorry :)

How can we react ASAP in case that react-scripts will have a real vulnerability that affects the produced assets if we totally turn the audit off for this package?

That is not what I’m saying. I’m not saying the risk of using software is zero. If you don’t want the risk of using third-party code, you shouldn’t use third-party code at all. You can write all the code of your application (including all tooling) yourself.

When you use code written by other people you assume some of that risk. My point isn’t that using build tools is risk-free. My point is that npm audit is an inadequate mechanism to surface these issues. If I may use an analogy, a person might have a high temperature, but you don’t want to measure it with a faulty thermometer that always reports high temperature. That’s the situation we’re in now.

Hmm, that is not what I meant. Your previous message was a satisfactory answer for me :)

gaearon commented 3 years ago

I think we're in agreement here. Of course we'd like to have a mechanism for reliable security flags. One thing we've been doing historically is to tag vulnerable releases of react-scripts with npm deprecate so people notice. But yes, I agree ideally there would be a proper solution that balances the tradeoffs well. It doesn't exist now but we'll be talking to Node and npm to see if we can work something out.

rhalaly commented 3 years ago

Yes, we are! I'm just raising the concern that the "0.01%" of true positive vulnerabilities will be off the radar and people should be aware of that.

When Dan Abramov posts a post like this one, people will just do it 😄. But I think it is important to also let people know and be aware that with these instructions the true positive vulnerabilities will be turned off as well.

In my system, I will try to do what you've suggested but also catch deprecation messages for react-scripts (or even use npm outdated react-scripts) as a CI/CD gate to be sure that I didn't miss a security fix (implicit assumption: your reaction is fast 😄). I think it is a good balance for the current npm limitation. Please let me know if you have an even better suggestion. Thank you!

gaearon commented 3 years ago

One thing we can consider is to bundle dependencies at publish time. This way, they won't show up in dep trees. Startup would be faster too. I'm seeing both Next.js and Vite doing it. This would break "customization" hacks but those were never first-class supported.

alamenai commented 3 years ago

I've done what you recommended :

  1. Move react-scripts to `devDependencies.
  2. Run npm audit --production.
  3. I got found 0 vulnerabilities

Thank you @gaearon ❤️ .

DesignByOnyx commented 3 years ago

Are we concerned at all about people who deploy their dev code? Whether it be for a quick POC, intimidation with managing build pipelines and artifacts, or general laziness? NPM audit is more paranoid than it is flawed, and I think it's an injustice to arm inexperienced developers with the sentiment "Dan Abramov says it's useless".

It seems that the maintainers of vulnerable projects are doing their part to fix the vulnerabilities - the least we could do is keep pressure on downstream projects to do the same - not matter how benign the vulnerability. I mean, the person-hours spent complaining about this the past week could have made a substantial dent in the upgrade.

Lastly, aren't most security exploits the result of some project maintainer being like "I see no way possible to exploit my code" and some hacker being like "hold my beer". I am very wary of someone declaring "this is not a real vulnerability" if they don't have substantial street cred in the hacker community.

compulim commented 3 years ago

@DesignByOnyx resonate with me.

Say, a package has vulnerability and CVE published. But the dev tool isn't hitting that code path. How could we count this as false positive reliably? Should someone scope it out in CVE and say, "if you hit this code path, this CVE applies to you. Otherwise, you are safe."

Not perfect, but I would suggest that we look at individual CVEs, and list out which CVE does not apply to react-scripts. And in short future, they should be fixed eventually. Or come back and fix them in the next 6 months.

npm audit --production seems a bit too automatic and could skip severe CVE that we have before. Not to mention, in an enterprise, a corporate governance policy may be used to inject npm audit for both production and development dependencies.

jindong-zhang-git commented 3 years ago

Hi there, npm audit --production works well for me, but yarn audit --production still found vulnerabilities...

jindong-zhang-git commented 3 years ago

By the way, how to avoid Github dependabot alerts?

lil5 commented 3 years ago

Could someone explain to me why the maintainers do not just update their dependencies?

Den-dp commented 3 years ago

@lil5 TLDR: sometimes "just update dependencies" is not an option because of the support policy.

If we take webpack as an example of a big package with dependencies that should be always up-to-date (your statement), webpack is restricted with introducing any breaking change into minor releases (since there is a concept of support policy and LTS), meaning the tool must not break its API in any way and must work with specified versions of node.js.

At the same time, webpack dependencies may not adhere to the concept of LTS and for example, they are free to release a newer version that may drop old node.js support. So at this point, webpack can't even consume such a package since this is a breaking change (even if API is the same).

And it can get worse when maintainers of such small packages are fixing security vulnerabilities only for the latest version of their package, which leads us to another problem of why webpack can't just update their dependencies all the time.

I believe create-react-app is in the same situation.

cronon commented 3 years ago

@Den-dp

And it can get worse when maintainers of such small packages are fixing security vulnerabilities only for the latest version of their package, which leads us to another problem of why webpack can't just update their dependencies all the time.

Then you can talk with author of the small package. In the worst case you fork that small package and do that fix by yourself. Either it is done by CRA or I'm forking CRA and do the same thing with CRA.

We have packages and modules and functions to abstract low-level details and therefore to alleviate the burden of micro management. That also includes low-level details about npm audit warnings.

When I use something I want to spend as little time as possible on learning how to use it. And npm warnings are very expensive to learn about. Not only I myself need to go deep into the details to find out if it is real or not. I will also have to convince all my customers that it is not real.

So at this point it is a question about what is cheaper: for me to talk to every customer, or to fork CRA and fix those warnings there or not to use CRA at all.

In the very case with CRA I can use it since my customers are fine if there are only low- and moderate-level vulnerabilities and none of the high and critical ones. I rather used it as an example on how people do their judgement.

lil5 commented 3 years ago

Currently vite is a reasonable alternative without audit messages

mcandre commented 3 years ago

--production is a stopgap, it does not actually remove any vulnerabilities that may be present on the developer's machine.

seanblonien commented 3 years ago

@lil5

Currently vite is a reasonable alternative without audit messages

As Dan mentioned in his full article, this is only because Vite bundles their dependencies in the library itself, so as to not use any node_modules at all, largely defeating the purpose of the module system.

If all packages started doing this, we would get rid of the audit warnings, but immediately lose protections provided by npm audit on client-side code (like actual vulnerabilities), there would be no deduping, and package sizes and bundle sizes could explode. Vite can do this because they are a dev dependency, but most libraries don't have that luxury, and it's a terrible precedent to set for the npm ecosystem.

pooyaEst commented 3 years ago

i'm curious to know why react-scripts isn't in devDependencies by default ?

seanblonien commented 3 years ago

Since it doesn't affect the bundle size when building, and the difference between dependencies/devDependencies is semantics at that point, the team chose to leave it in. See Dan's reason explaining it.

paolotormon commented 3 years ago

I have 2519 high vulnerabilities. Is this the same issue?

image

kp2017-zz commented 3 years ago

I completely disagree with this analysis. NPM audit's purpose is to help package users identify safe packages. The issue mentioned and the justification behind it doesn't hold water. The same rational could be used to justify exposing SQL injection attacks (I want to do it, I can do it, I am doing it). This project provides not warrantee or protections for its code and therefore is up to the user to take on the blunt of the responsibility. If you use this package based on the justifications described with in this issue, you will have to convince others that your rational was sound if a security breach happens. I don't think you will be successful. I would suggest using another JS library or Web Components. Currently Vue.JS have no vulnerabilities and offers much of the same functionality as React JS.

kp2017-zz commented 3 years ago

Angular also has 0 vulnerabilities

icecream17 commented 3 years ago

VueJS does have vulnerabilities: (e.g.: Webpack 4.28.4 https://github.com/vuejs/vue/blob/dev/package.json https://snyk.io/test/npm/webpack/4.28.4) but everything's bundled so npm doesn't detect any dependencies. (https://snyk.io/test/npm/vue?tab=dependencies) And same with angular: https://snyk.io/test/npm/angular?tab=dependencies

SherylHohman commented 3 years ago

In addition to the npm command line warnings, GitHub Dependabot is also adding to the pipeline. "Vulnerabilities" highlighted at the top of our pages as persistent warnings really grabs my attention! While this format can make it easier for us tend to actual vulnerability issues on our schedule (especially since they include links to "fix" the), it could also add stress, seeing them in the first place. Which may also increase the number of people reaching out to you.
Thank you for your attempts to address this issue which is outside your control, and the great explanation. This puts my mind at ease, and I can wasting my time quit trying find fixes for these myself, and worry that if I to update a "vulnerable" package that I haven't broken CRA itself. I'll leave everything as is! Thanks for everything you do !! :-)

jap99 commented 3 years ago

For anyone who missed it, report your complaint here in hopes of this being fixed:

https://github.com/npm/cli/issues

I have made one here; however, and you can add on to it instead of creating a new, duplicate issue:

https://github.com/npm/cli/issues/3930

clintandrewhall commented 3 years ago

@gaearon Sorry to ping you directly, but assuming I haven't made a mistake somewhere, it's not just npm. Github dependabot still highlights the "vulnerabilities", even when moved to devDependencies. So I don't think this is limited to npm audit.

I just noticed this in a snack project (https://github.com/clintandrewhall/bunnyghp) and found this issue when searching for a resolution. So even the simplest of projects gives a lot of noise in even the Github GUI.

Screen Shot 2021-10-22 at 1 28 35 AM Screen Shot 2021-10-22 at 1 28 55 AM

I empathize... but this is going to get noisier without a solution?

EDIT: I see @SherylHohman also called this out, above.

alexandra-urberg commented 2 years ago

Thank you!!!

hai0611 commented 2 years ago

I still can't fix the error when npx create-react-app myapp , still showing the message npm audit , I really don't know what to do can anyone guide me in detail how to create create-react-app when I get this error. Thanks so much

Samuel-Therrien-Beslogic commented 2 years ago

Here's the workaround I'm using: Add audit=false to your .npmrc, and add "scripts": {"postinstall": "npm audit --production --audit-level=none"} to your package.json !

icest99 commented 2 years ago

I can't believe I got stuck in this for hours because of an npm bugs. jeez, thanks you so much.

hai0611 commented 2 years ago

Here's the workaround I'm using: Add audit=false to your .npmrc, and add "scripts": {"postinstall": "npm audit --production"} to your package.json !

After running that error, it will stop and I did the same as you and then still don't know how to continue the installation with the command

dyedwiper commented 2 years ago

Thanks for the helpful explanation! Just a side note: If you deploy your react app to Heroku, you can break your deployment by moving react-scripts to devDependencies. For an explanation refer to this stackoverflow answer.

@gaearon I don't know how common Heroku is for react apps but maybe you wanna add a hint in your advice.

Samuel-Therrien-Beslogic commented 2 years ago

Here's the workaround I'm using: Add audit=false to your .npmrc, and add "scripts": {"postinstall": "npm audit --production"} to your package.json !

After running that error, it will stop and I did the same as you and then still don't know how to continue the installation with the command

@hai0611 @twocs @pzrq I've updated my answer, make sure to have --audit-level=none

asitsar1 commented 2 years ago

Hi, We received a report of vulnerability in our security testing.

The normalize-url package before 4.5.1, 5.x before 5.3.1, and 6.x before 6.0.1 for Node.js has a ReDoS (regular expression denial of service) issue because it has exponential performance for data: URLs.

As we expect 4.5.1 , when the library will be available?

benjaminpeto commented 2 years ago

Hi,

Same issue here, as well. After CRA, uninstall react-scripts from dependencies, reinstall to devDependencies, run npm audit -production, still getting a bunch of vulnerabilities. 27 vulnerabilities (16 moderate, 9 high, 2 critical)

Coming from :

ansi-html * Severity: high

ansi-regex >2.1.1 <5.0.1 Severity: moderate

browserslist 4.0.0 - 4.16.4 Severity: moderate

glob-parent <5.1.2 Severity: high

immer <9.0.6 Severity: critical

nth-check <2.0.1 Severity: moderate

Node version: 16.13.1 Npm version: 8.1.2

I don't know how to move forward with it anymore, or should I just ignore and build the app anyway.