Security warning deluge from 'npm audit' is driving developers to distraction

Dan Abramov, a software engineer at Facebook, this week published a plea to silence a particularly vocal JavaScript security tool – and its creators more or less agreed there's room for improvement. "As of today, npm audit is a stain on the entire npm ecosystem," Abramov declared in a blog post. "The best time to fix it was …

[Filippo]

In one of the last projects I worked on, we used a library to generate random numbers, and the audit tool complained that the random numbers generated by the library were not good enough to be used for cryptography. Fine - but we were using them for simulations, not for cryptography!

We used another library that had many features, including some that went online and depended on a vulnerable SSL implementation. Thanks - but we were not using any online features at all!

Then there was another that had a SQL injection problem. Okay - but in our project we weren't even using a SQL database!

Then we had a whole gaggle of vulnerabilities in a test project. I get it - but it's a test project; it's not going to be Internet-facing at any time!

I understand that any of those issues might suddenly become real issues... but only if some specific sets of other much bigger mistakes happen. It's not worth ripping and replacing an entire library just because some insane person in the future might accidentally decide to remove the localhost-only restriction on the test suite and then put it online. Especially considering that the new library you just spent twenty hours replacing will most likely get flagged for something else in a couple months' time anyway, and it still won't matter because we are still not using it in a way where the vuln is relevant.

And yet, with all of that said, there were a few cases where the tool pointed out stuff that actually needed addressing.

This is definitely a problem, although I'm not sure how it could be solved cleanly and efficiently.

[[e-mail]]

Job done ...

Sounds to me like the warnings worked then. Security depends on context and mitigation. So the tool flagged up warnings, you reviewed those warnings and decided they did not apply in your context. For a few warnings you decided they did apply. Working as intended.

So now you must document all that so that in future if the context changes you have a reference for further review. This is called being professional.

[Gene Cash]

> This is definitely a problem, although I'm not sure how it could be solved cleanly and efficiently.

By defining classes of warnings.

You're not using the network? You need a setting that dykes out all the network-related warnings.

Same for the database related warnings and so on.

Come on. This is nothing new. Turbo Pascal had it in the '80s.

For example, Android gets all hot and bothered about using network functions in the GUI thread (for obvious reasons) - but if I don't care my shitty garage door app freezes for a second while it's connecting to my Raspberry Pi, then I can set a pragma to disable that.

[DevOpsTimothyC]

Elephant in the room ?

Then there was another that had a SQL injection problem. Okay - but in our project we weren't even using a SQL database!

Then why are you including a library with SQL connectivity ? Even if it isn't the library that you're using directly but one of the libraries that are being pulled in as a dependency, or a dependency of a dependency.

This really comes back to some of the issues that https://www.theregister.com/2016/03/23/npm_left_pad_chaos/ highlighted over 5 years ago.

To some extent, the situation is unavoidable given the attack surface in the Node.js ecosystem, where the installation of an average npm package means trusting around 80 other packages due to transitive dependencies

producing numbers often larger than the number of modules in the tree

Both of those quote should (and in other communities would) ring alarm bells. The situation is not unavoidable it's just something that node and npm have decided to live with because it's easier to complain than address the bigger issues

[Filippo]

Re: Elephant in the room ?

Exactly that. The reason we ended up never running the audit was that it took forever to make sure that all the warnings were false positives.

And the reason for that was that most of the warnings were about packages that none of us had ever even heard of, but were pulled as transitive dependencies. That means that, in order to make sure it's a false positive, you need to review the package, figure out what it does and how it does it and how the library you're using is using it in turn, and so on and so forth.

I don't see how it's a sustainable model. Me, I'm just steering clear of JS development any time I can help it, and charging twice as much when I can't.

[oldtaku]

99% false positives is worse than nothing

You need below 5% false positives at the very worst, 1% is better (0% is impossible). Remember when Windows implemented UAC and programs triggered it every 5 minutes (because they weren't UAC-aware at the time) so everyone just turned it off because it was worse than useless? This is like that. You're not even going to notice a legitimate security issue in all the spam.

Eventually, if npm is actually more interested in making it useful rather than revenue enhancement, I think this can be largely mitigated. It's just a retread of what's been done before with compiler errors/warnings and lint errors/warnings though most of the JS people may have no idea what that even means or that this is hardly a new problem. The compiler (checker) gets better at deciding what's a real error and what's just a warning, the package authors get better about adapting their code to the compiler (checker) - which usually always results in better code - and then you get the option to manually disable specific things for specific packages. Just getting rid of the cascading errors (where a single thing generates 20 errors) would be a big start.

[Anonymous Coward]

The difference between good and bad security product

Is the bad one are pushing tons of errors without checking dependencies/applicability in the context.

This is not only true for products for SW, but also for infra/cloud products.

For infra/cloud, there are one or 2 that do elaborated links models between security issues, and rate them according to applicability.

Basically, red means you're already pwned already.

Rest should be analyzed carefully, minus the info that can be ignored.

[Warm Braw]

This is strangely redolent..

... of the present England approach to coronavirus: the data looks bad so let's ignore it.

Frankly, if installation of an average npm package means trusting around 80 other packages there is something very wrong with the packaging structure and the alert overload is a clear warning of that. Given the amount of code you'd have to carefully analyse to claim that there is a 99+ per cent false positive rate, I'm not sure how credible that figure really is.

And this is not the only measure of the apparent fragility of this ecosystem.

[Doctor Syntax]

Re: This is strangely redolent..

Just this. As an innocent bystander there's nothing in this report that would encourage me to ditch NoScript, nor to stop me ignoring sites that simply fail to display anything with with NoScript in operation.

[Anonymous Coward]

Re: This is strangely redolent..

I upvoted this but I was in two minds to downvote it. What it says about javascript is correct, IMHO, but I see no need to have introduced irrelevant political opinion into the comment.

[iron]

Re: This is strangely redolent..

Not to worry, I was also in two minds but downvoted for that very reason. So between us we have covered both our opinions.

[cschneid]

configuration options

By all means, add flexible configuration options to ignore categories of warnings, sub-categories, individual warnings, or any of the preceding in various contexts. Then add the only option anyone in serious need of these warnings will use - ignore all warnings. Kind of like those unit tests that just return true.

[jurassicmonkey]

Can we have some real engineering, please?

Dan Abramov's original blog post claimed that the default npm audit behavior, "... in many situations, leads to a 99%+ false positive rate..."

Hang on a second. Are we not supposed to be engineers? Since when was guessing good enough? I know it's a guess because no research/data was provided (only some examples) and if there was any research you can bet he would have published it.

This preference for guessing-instead-of-knowing (aka engineering) is the problem with the Node.js ecosystem in particular and programming in general! FFS How many times has el reg published a story where some well-intentioned engineer or admin took the guessing-over-knowing route and wound up in the news? No wonder hackers are having a field day - we're doing it wrong!

So, Mr. Abramov, howsabout a little more engineering and a little less guessing in general?

Cheers!

[Falmari]

Re: Can we have some real engineering, please?

@jurassicmonkey ​"default npm audit behavior" default give sthe impression the warning level is adjustable and default is set at the highest level. It would be nice to know how hard it is to set to a level that suit the JS being written.

Coming from C, C++ and C# development if we left the warning level at default with every warning treated as an error we would be in a similar position with false positive rates. We set the level and exclude certain types of warning from being generated.

[BHetrick]

Re: Can we have some real engineering, please?

A long time ago, when I did compilers for a living, I had to explain the difference between an error message, a warning message, and an informational message. An error message is the compiler saying "this code is wrong and I can't figure out what you could have possibly intended." A warning message is the compiler saying "this code is wrong but I can think of something you might have intended." An informational message is the compiler saying "you might want to know this fascinating thing I have observed." A _warning_ means the code is _wrong_. Ignore _warnings_ at your peril.

[Claptrap314]

Fix symptoms, not problems

As mentioned, the issue is the sheer number transitive dependencies. (And as a rubyist, I'm looking at YOU, DHH.) The idea that getting code out the door NOW is functionally the most important thing is what is driving this.

But, as I keep harping, in the end, the customer is king.

Someone posted a mention of James Dean's seatbelt. For those who do know know (such as myself, an hour ago), James Dean died in an automobile accident. Investigators determined that if he had been belted in, he would have survived. At the time, however, very few cars even had an option for seat belts. The article goes on to trace the history of seat belt usage, crediting this incident as getting the ball rolling. I was about 18 when the laws were being passed. I contemplated stopping wearing in protest of the blatantly unconstitutional process by which those laws came about.

I'm afraid that the solution is going to involve legislation. Insurance looks like it is going to fail. (The costs are too high, companies will just ignore the risks.) Liability for directors/board members seems the most likely. The situation with Kaseya is a pretty good demonstration of why.

[Missing Semicolon]

Simple fix

Make the code not be crap, then the tsunami of warnings will disappear. When a new warning appears, we can fix it.

But "too many warnings, let's hide them!" is not a good move... IF THE CODE IS CRAP!