overreacted by sunshineLixun

npm audit: Broken by Design

07/07/2021

word count:2644

estimated reading time:14minute

Security is important. Nobody wants to be the person advocating for less security. So nobody wants to say it. But somebody has to say it.

So I guess I’ll say it.

The way npm audit works is broken. Its rollout as a default after every npm install was rushed, inconsiderate, and inadequate for the front-end tooling.

Have you heard the story about the boy who cried wolf? Spoiler alert: the wolf eats the sheep. If we don’t want our sheep to be eaten, we need better tools.

As of today, npm audit is a stain on the entire npm ecosystem. The best time to fix it was before rolling it out as a default. The next best time to fix it is now.

In this post, I will briefly outline how it works, why it’s broken, and what changes I’m hoping to see.


Note: this article is written with a critical and somewhat snarky tone. I understand it’s super hard to maintain massive projects like Node.js/npm, and that mistakes may take a while to become apparent. I am frustrated only at the situation, not at the people involved. I kept the snarky tone because the level of my frustration has increased over the years, and I don’t want to pretend that the situation isn’t as dire as it really is. Most of all I am frustrated to see all the people for whom this is the first programming experience, as well as all the people who are blocked from deploying their changes due to irrelevant warnings. I am excited that this issue is being considered and I will do my best to provide input on the proposed solutions! 💜


How does npm audit work?

Skip ahead if you already know how it works.

Your Node.js application has a dependency tree. It might look like this:

your-app
  - view-library@1.0.0
  - design-system@1.0.0
  - model-layer@1.0.0
    - database-layer@1.0.0
      - network-utility@1.0.0

Most likely, it’s a lot deeper.

Now say there’s a vulnerability discovered in network-utility@1.0.0:

your-app
  - view-library@1.0.0
  - design-system@1.0.0
  - model-layer@1.0.0
    - database-layer@1.0.0
      - network-utility@1.0.0 (Vulnerable!)

This gets published in a special registry that npm will access next time you run npm audit. Since npm v6+, you’ll learn about this after every npm install:

1 vulnerabilities (0 moderate, 1 high)

To address issues that do not require attention, run:
  npm audit fix

To address all issues (including breaking changes), run:
  npm audit fix --force

You run npm audit fix, and npm tries to install the latest network-utility@1.0.1 with the fix in it. As long as database-layer specifies that it depends not on exactly on network-utility@1.0.0 but some permissible range that includes 1.0.1, the fix “just works” and you get a working application:

your-app
  - view-library@1.0.0
  - design-system@1.0.0
  - model-layer@1.0.0
    - database-layer@1.0.0
      - network-utility@1.0.1 (Fixed!)

Alternatively, maybe database-layer@1.0.0 depends strictly on network-utility@1.0.0. In that case, the maintainer of database-layer needs to release a new version too, which would allow network-utility@1.0.1 instead:

your-app
  - view-library@1.0.0
  - design-system@1.0.0
  - model-layer@1.0.0
    - database-layer@1.0.1 (Updated to allow the fix.)
      - network-utility@1.0.1 (Fixed!)

Finally, if there is no way to gracefully upgrade the tree, you could try npm audit fix --force. This is supposed to be used if database-layer doesn’t accept the new version of network-utility and also doesn’t release an update to accept it. So you’re kind of taking matters in your own hands, potentially risking breaking changes. Seems like a reasonable option to have.

This is how npm audit is supposed to work in theory.

As someone wise said, in theory there is no difference between theory and practice. But in practice there is. And that’s where all the fun starts.

Why is npm audit broken?

Let’s see how this works in practice. I’ll use Create React App for my testing.

If you’re not familiar with it, it’s an integration facade that combines multiple other tools, including Babel, webpack, TypeScript, ESLint, PostCSS, Terser, and others. Create React App takes your JavaScript source code and converts it into a static HTML+JS+CSS folder. Notably, it does not produce a Node.js app.

Let’s create a new project!

npx create-react-app myapp

Immediately upon creating a project, I see this:

found 5 vulnerabilities (3 moderate, 2 high)
  run `npm audit fix` to fix them, or `npm audit` for details

Oh no, that seems bad! My just-created app is already vulnerable!

Or so npm tells me.

Let’s run npm audit to see what’s up.

First “vulnerability”

Here is the first problem reported by npm audit:

┌───────────────┬──────────────────────────────────────────────────────────────┐
│ Moderate      │ Regular Expression Denial of Service                         │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Package       │ browserslist                                                 │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Patched in    │ >=4.16.5                                                     │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Dependency of │ react-scripts                                                │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Path          │ react-scripts > react-dev-utils > browserslist               │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ More info     │ https://npmjs.com/advisories/1747                            │
└───────────────┴──────────────────────────────────────────────────────────────┘

Apparently, browserslist is vulnerable. What’s that and how is it used? Create React App generates CSS files optimized for the browsers you target. For example, you can say you only target modern browsers in your package.json:

  "browserslist": {
    "production": [
      ">0.2%",
      "not dead",
      "not op_mini all"
    ],
    "development": [
      "last 1 chrome version",
      "last 1 firefox version",
      "last 1 safari version"
    ]
  }

Then it won’t include outdated flexbox hacks in the output. Since multiple tools rely on the same configuration format for the browsers you target, Create React App uses the shared browserslist package to parse the configuration file.

So what’s the vulnerability here? “Regular Expression Denial of Service” means that there is a regex in browserslist that, with malicious input, could become very slow. So an attacker can craft a special configuration string that, when passed to browserslist, could slow it down exponentially. This sounds bad…

Wait, what?! Let’s remember how your app works. You have a configuration file on your machine. You build your project. You get static HTML+CSS+JS in a folder. You put it on static hosting. There is simply no way for your application user to affect your package.json configuration. This doesn’t make any sense. If the attacker already has access to your machine and can change your configuration files, you have a much bigger problem than slow regular expressions!

Okay, so I guess this “Moderate” “vulnerability” was neither moderate nor a vulnerability in the context of a project. Let’s keep going.

Verdict: this “vulnerability” is absurd in this context.

Second “vulnerability”

Here is the next issue npm audit has helpfully reported:

┌───────────────┬──────────────────────────────────────────────────────────────┐
│ Moderate      │ Regular expression denial of service                         │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Package       │ glob-parent                                                  │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Patched in    │ >=5.1.2                                                      │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Dependency of │ react-scripts                                                │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Path          │ react-scripts > webpack-dev-server > chokidar > glob-parent  │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ More info     │ https://npmjs.com/advisories/1751                            │
└───────────────┴──────────────────────────────────────────────────────────────┘

Let’s look at the webpack-dev-server > chokidar > glob-parent dependency chain. Here, webpack-dev-server is a development-only server that’s used to quickly serve your app locally. It uses chokidar to watch your filesystem for changes (such as when you save a file in your editor). And it uses glob-parent in order to extract a part of the filesystem path from a filesystem watch pattern.

Unfortunately, glob-parent is vulnerable! If an attacker supplies a specially crafted filepath, it could make this function exponentially slow, which would…

Wait, what?! The development server is on your computer. The files are on your computer. The file watcher is using the configuration that you have specified. None of this logic leaves your computer. If your attacker is sophisticated enough to log into your machine during local development, the last thing they’ll want to do is to craft special long filepaths to slow down your development. They’ll want to steal your secrets instead. So this whole threat is absurd.

Looks like this “Moderate” “vulnerability” was neither moderate nor a vulnerability in the context of a project.

Verdict: this “vulnerability” is absurd in this context.

Third “vulnerability”

Let’s have a look at this one:

┌───────────────┬──────────────────────────────────────────────────────────────┐
│ Moderate      │ Regular expression denial of service                         │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Package       │ glob-parent                                                  │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Patched in    │ >=5.1.2                                                      │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Dependency of │ react-scripts                                                │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Path          │ react-scripts > webpack > watchpack > watchpack-chokidar2 >  │
│               │ chokidar > glob-parent                                       │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ More info     │ https://npmjs.com/advisories/1751                            │
└───────────────┴──────────────────────────────────────────────────────────────┘

Wait, it’s the same thing as above, but through a different dependency path.

Verdict: this “vulnerability” is absurd in this context.

Fourth “vulnerability”

Oof, this one looks really bad! npm audit has the nerve to show it in red color:

┌───────────────┬──────────────────────────────────────────────────────────────┐
│ High          │ Denial of Service                                            │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Package       │ css-what                                                     │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Patched in    │ >=5.0.1                                                      │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Dependency of │ react-scripts                                                │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Path          │ react-scripts > @svgr/webpack > @svgr/plugin-svgo > svgo >   │
│               │ css-select > css-what                                        │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ More info     │ https://npmjs.com/advisories/1754                            │
└───────────────┴──────────────────────────────────────────────────────────────┘

What is this “high” severity issue? “Denial of service.” I don’t want service to be denied! That would be really bad… Unless…

Let’s look at the issue. Apparently css-what, which is a parser for CSS selectors, can be slow with specially crafted input. This parser is used by a plugin that generates React components from SVG files.

So what this means is that if the attacker takes control of my development machine or my source code, they put a special SVG file that will have a specially crafted CSS selector in it, which will make my build slow. That checks out…

Wait, what?! If the attacker can modify my app’s source code, they’ll probably just put a bitcoin miner in it. Why would they add SVG files into my app, unless you can mine bitcoins with SVG? Again, this doesn’t make any sense.

Verdict: this “vulnerability” is absurd in this context.

So much for the “high” severity.

Fifth “vulnerability”

┌───────────────┬──────────────────────────────────────────────────────────────┐
│ High          │ Denial of Service                                            │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Package       │ css-what                                                     │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Patched in    │ >=5.0.1                                                      │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Dependency of │ react-scripts                                                │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Path          │ react-scripts > optimize-css-assets-webpack-plugin > cssnano │
│               │ > cssnano-preset-default > postcss-svgo > svgo > css-select  │
│               │ > css-what                                                   │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ More info     │ https://npmjs.com/advisories/1754                            │

└───────────────┴──────────────────────────────────────────────────────────────┘

This is just the same exact thing as above.

Verdict: this “vulnerability” is absurd in this context.

Shall we keep going?

So far the boy has cried wolf five times. Two of them are duplicates. The rest are absurd non-issues in the context of how these dependencies are used.

Five false alarms wouldn’t be too bad.

Unfortunately, there are hundreds.

Here are a few typical threads, but there are many more linked from here:

Screenshot of many GH threads

I’ve spent several hours looking through every npm audit issue reported to us over the last several months, and they all appear to be false positives in the context of a build tool dependency like Create React App.

Of course, they are possible to fix. We could relax some of the top-level dependencies to not be exact (leading to bugs in patches slipping in more often). We could make more releases just to stay ahead of this security theater.

But this is inadequate. Imagine if your tests failed 99% of the times for bogus reasons! This wastes person-decades of effort and makes everyone miserable:

It doesn’t help that npm audit fix (which the tool suggests using) is buggy. I ran npm audit fix --force today and it downgraded the main dependency to a three-year-old version with actual real vulnerabilities. Thanks, npm, great job.

What next?

I don’t know how to solve this. But I didn’t add this system in the first place, so I’m probably not the best person to solve it. All I know is it’s horribly broken.

There are a few possible solutions that I have seen.

The root of the issue is that npm added a default behavior that, in many situations, leads to a 99%+ false positive rate, creates an incredibly confusing first programming experience, makes people fight with security departments, makes maintainers never want to deal with Node.js ecosystem ever again, and at some point will lead to actually bad vulnerabilities slipping in unnnoticed.

Something has to be done.

In the meantime, I am planning to close all GitHub issues from npm audit that I see going forward that don’t correspond to a real vulnerability that can affect the project. I invite other maintainers to adopt the same policy. This will create frustration for our users, but the core of the issue is with npm. I am done with this security theater. Node.js/npm have all the power to fix the problem. I am in contact with them, and I hope to see this problem prioritized.

Today, npm audit is broken by design.

Beginners, experienced developers, maintainers, security departments, and, most importantly — our users — deserve better.