5 ms·
A regex "denial of service" "vulnerability" could be important, if it shows up in code that processes untrusted input from end users. But NPM Audit has no idea
by JonathonW 5y ago
A regex "denial of service" "vulnerability" could be important, if it shows up in code that processes untrusted input from end users.
But NPM Audit has no idea of context-- a "critical" bug in `browserlist`, which, in this context, is never used outside the development process and never takes input outside of what's in my package.json, gets the same prominence (or more so, since it's early in alphabetical order) as a "critical" bug in Express, potentially allowing my server to be compromised.
I'm not really sure what the solution is here; NPM's just a package manager and doesn't know how you're using a given package. A simple heuristic distinguishing development dependencies and runtime dependencies in NPM Audit might be a start, but that doesn't help with situations like create-react-app's react-scripts where everything, runtime or dev dependency, is a transitive dependency of one package declared as a runtime dependency.
- nkozyra 5y ago> A regex "denial of service" "vulnerability" could be important, if it shows up in code that processes untrusted input from end users. But in this context what's the end result? Chrome locking up on the end user's (attacker's) machine? Again, an "attacker" doesn't have access to the source code for distribution. By inputting bad regexp data they're only DOSin themselves, no?
- esens 5y agoCould be in a service on a server, which in this case a RegEx DOS could lock the server for all users.
- JonathonW 5y agoOr if a JS frontend takes in input that comes from other users-- something like forum post titles or content. That's "just" a browser freeze for end users, but still a potential DOS vulnerability if it's in the application's critical path.
- nkozyra 5y agoRight, but that's why I wrote "context," and seems to be the primary complaint in this article.
- deleted 5y ago[deleted]
- Avamander 5y agoStackOverflow and Cloudflare have both self-DoSed themselves with such flaws, causing downtime.
- twistedpair 5y agoWould be nice if package.json had a flag to indicate the runtime would be either Node.js or a browser. So many of these "bugs" have no bearing in a browser context.
- deleted 5y ago[deleted]
- gforge 5y agoThe package.json should be able to actively ignore vulnerability id's. As id's disappear with audits the npm audit could just remove those, eg a "prune"
- Angius 5y agoIMHO one solution would be to categorize vulnerabilities separately for prod dependencies and dev dependencies, and bubble that categorization up. For example, a RegEx DDoS vulnerability in Express would show up as high severity, while the same would not show in the bundler you use, or any package that your bundler has in its dependency tree.
- IggleSniggle 5y agoAgreed! A “Critical” bug in a dev context should mean something very different from a “Critical” bug in a prod context. A “Critical” devDependency bug should be either a direct threat to the developer’s context, either by infecting the dev machine or by injecting a supply-chain problem, worming it’s way into downstream contexts. npm audit is just not granular OR careful enough to address these issues appropriately.
- pmoleri 5y agoAgree 100%
- lmm 5y agoOther developers have no idea of context either. Unless you have a way of enforcing that certain code is never exposed to user input (and I agree that a build-time-only dependency does solve that), sooner or later it will be. Accepting regexes from user input is a really insidious class of bug that can go undetected for years. I've seen real outages caused by it, so it's absolutely worth doing something proactive about.
- IsaacSchlueter 5y agoTrue story, the npm registry was once taken down (not maliciously, just by accident) by a ReDOS in node-semver. That was extra fun to debug because the failure happened inside of CouchDB.