Security vulnerabilities

[Berkmann18]

After running an audit on dependent packages I found out that both jest and jest-cli were vulnerable to a prototype pollution (due to lodash which doesn't seem to be at the latest version, i.e v4.17.10), explained here: https://nodesecurity.io/advisories/577.
And a cryptographically weak PRNG (due to randomatic) which is explained here: https://nodesecurity.io/advisories/157

I've flagged the respective issues here and here.

[jonschlinkert]

Hi there @Berkmann18, you've been busy creating these issues everywhere. I created responses on other issues you created, but for the edification of anyone who happens to stop by here, the following is a copy of my response on stylelint/stylelint#2888 (comment):

Hey guys, I created both randomatic and micromatch. Unless you're using micromatch to generate passwords or API tokens, I think it's safe to say that this is unlikely to actually be a concern.

Randomatic optionally generates random strings, but the main use of the library has always been to generate strings that follow a certain pattern. For example, it was used in micromatch for simplifying how patterns are expanded in brace patterns. You could also use it to generate a pseudo-random string for mocking out unit tests for things like order numbers that follow a pattern, like ORDER-001-AAAB, then randomatic can generate some strings that follow that pattern. Today, we mention passwords, prior to that, it wasn't used by anyone to do that. The language changed on the readme. As it relates to the usage in micromatch, it's specifically called when a brace pattern is expanded. IMHO, it's pretty unlikely that anyone would use a glob pattern for a password.

That said, I do see the following string a lot in password examples: ****************. My recommendation is that if you can avoid using micromatch for cryptography, you'll be in good shape.

It's also worth mentioning that it's never, ever a good idea to create issues about potential security issues or vulnerabilities, given that anyone in the public can exploit the information before a maintainer is able to act and/or the downstream consumers of the package in question are able to act.

[Berkmann18]

@jonschlinkert Because I would rather use packages not vulnerable to things known.

Also if the audit is wrong as you claimed, then why is still there and relevant to the latest versions of randomatic that you apparently fixed?

[jonschlinkert]

Also if the audit is wrong as you claimed, then why is still there and relevant to the latest versions of randomatic

I spoke with the person who created the warning. It's there still because, according to him, he created it a long time ago and doesn't know how to remove it.

that you apparently fixed

Nothing was "fixed", and there was never anything to fix.

Because I would rather use packages not vulnerable to things known.

You are not helping anyone, or yourself, by irresponsibly disclosing potential vulnerabilities in very public issues. On the other hand, it might seem like an exception to the rule to create an issue about a "known vulnerability", since every developer in the entire Node.js ecosystem sees the same exact console warnings as you. But then... ?

[Berkmann18]

I spoke with the person who created the warning. It's there still because, according to him, he created it a long time ago and doesn't know how to remove it.

How strange, surely that person would have a colleague or know someone who could.

Nothing was "fixed", and there was never anything to fix.

Fair enough.

You are not helping anyone, or yourself, by irresponsibly disclosing potential vulnerabilities in very public issues. On the other hand, it might seem like an exception to the rule to create an issue about a "known vulnerability", since every developer in the entire Node.js ecosystem sees the same exact console warnings as you. But then... ?

Fair point, I admit I did that wrong (so I apologise). The thing is that those vulnerabilities are shown to anyone who install/update a package that depends on a vulnerable one or if the package itself is vulnerable (even through GitHub itself).
So surely, it would be better to alert the relevant people about X or Y issues being present in the latest version of A or B packages?
Of course, some of those vulnerabilities aren't exploitable but it's still some unnecessary noise, don't you agree?

[jonschlinkert]

How strange, surely that person would have a colleague or know someone who could.

I agree, I looked into it for a bit, but admittedly I wasn't as tenacious as I could have been.

Fair point, I admit I did that wrong (so I apologise)

No need to apologize, I think you were doing what seemed right and being on the safe side. If I sounded harsh then allow me to apologize to you instead. It wasn't intended.

The thing is that those vulnerabilities are shown to anyone who install/update a package that depends on a vulnerable one or if the package itself is vulnerable (even through GitHub itself).

I'm going to go out on a limb and say something that I've kept to myself until now. IMHO, the messages are complete nonsense, and I have a hunch that its net impact is costing the ecosystem more in "damages" - in terms of the total time the ecosystem spends on discussing these "vulnerabilities", replacing packages, dealing with issues, etc. - than from the "vulnerabilities" themselves, their risk, or the actual damages caused by them. If a vulnerability is truly severe enough to be threatening to NPM's users, it would be dealt with swiftly in a centralized way. It would be strange for a software company to disclose details publicly about an ongoing vulnerability that could still be exploited.

My theory is that NPM is using those messages as a way of creating a competitive advantage over Yarn. The gravity of both the urgency and importance of the warnings is likely to be also associated with NPM, creating cognitive dissonance in users, and potentially more trust in NPM, etc.

I guess this is a conversation for another time and place lol.

[Berkmann18]

I'm going to go out on a limb and say something that I've kept to myself until now. IMHO, the messages are complete nonsense, and I have a hunch that its net impact is costing the ecosystem more in "damages" - in terms of the total time the ecosystem spends on discussing these "vulnerabilities", replacing packages, dealing with issues, etc. - than from the "vulnerabilities" themselves, their risk, or the actual damages caused by them. If a vulnerability is truly severe enough to be threatening to NPM's users, it would be dealt with swiftly in a centralized way. It would be strange for a software company to disclose details publicly about an ongoing vulnerability that could still be exploited.

I agree I think NPM should only announce those vulnerabilities after emailing or privately notifying the repo owner(s) (maybe a bit like how Github does) and that the vulnerabilities were fixed.
And I do see false positives in security warnings although that's mainly in regards to ESLint's security plugin, I suppose NSP also gives a few false positives too.

Funny enough, those "issues" wouldn't have been so widespread if NSP didn't join NPM (well there's also Snyk out there but ay).

[SampsonCrowley]

@jonschlinkert

It's also worth mentioning that it's never, ever a good idea to create issues about potential security issues or vulnerabilities, given that anyone in the public can exploit the information before a maintainer is able to act and/or the downstream consumers of the package in question are able to act.

I have to completely disagree here. Hiding vulnerabilities doesn't protect anyone. If you can find it, so can someone that is actually trying to exploit it. The difference being when you don't make people aware of the possibility, they can't protect themselves. You are then creating a situation where a malicious user can exploit what they found without risk of people knowing it's possible. this is all open source. If you pretend it doesn't exist, on the idea that it takes time to patch, all you're doing is making sure the people waiting for that patch don't take preventative measures in the meantime.

[gheiler-on]

In any case lodash seemed to think this was a vulnerability concern and they fix it lodash/lodash#4336
So it would be nice to reopen this and update the dependency to the version with the fix.

[SimenB]

Jest does not depend on lodash, so there's nothing for us to do.

And Lodash released their fix in a patch, so just do npm audit fix