jQuery 3.4.0 reported as having vulnerability, but current Snyk DB shows that it's fine

[tmb-github]

Running Lighthouse 4.3.0 on Chrome Canary Version 75.0.3769.0 (Official Build) canary (64-bit).
OS: Windows 8.1

Site tested: https://ronsanfordproductions.com

Lighthouse complains:

Includes front-end JavaScript libraries with known security vulnerabilities:
[email protected]

However, according to the Snyk vulnerability DB, which is referenced as the source for the vulnerability check, [email protected] is free of vulnerabilities. See:

https://snyk.io/vuln/npm:jquery

So, Lighthouse isn't reading the current info from Snyk when disapproving of jQuery 3.4.0, it seems. This seems to be a bug.

[paulirish]

Yup everything you're saying looks correct.

Snyk originally said this vuln affected ALL versions of jQuery. (The vulnerability in question)
But after we shipped LH 4.3.0, snyk updated their data to say that the vuln only applies to jQuery 3.4.0.

Team, we may have to ship a LH 4.3.1 to fix this. :/

[paulirish]

@lirantal do you know why the jquery prototype pollution bug was originally marked as affecting all versions?

Also do you know why we only got the revised vuln range 2 weeks later?

[connorjclark]

3.4.0 is the newest jquery, so I guess snyk publishes vulns for newest versions of libs as * to catch future releases w/o the bug fix?

eb0cc68#diff-7413e898991773eb5c8e74ee5458818bR60 was 15 days ago, pre jquery 4.3.0, so "*" was technically correct. But it doesn't play well with how we use snyk has a dependency.

Any way we could change how we consume snyk to live-update the vuln database (w/o a release)?

[lirantal]

@hoten @paulirish 👋
there was no fix for that vulnerability for quite a bit but the vulnerability was public and known, hence affecting all jquery versions. When the fix was available we updated the vulnerability version range accordingly.

does that explain it?
happy to hear how we could've improved that process

[paulirish]

Thanks for the explanation.

When the fix was available we updated the vulnerability version range accordingly.

Looking at the dates, new jquery was shipped april 10th.
We didn't get a PR with the new dump until april 17th.

I guess we just got unlucky with the bot's (apparent) weekly schedule?

At the very least we'll be a lot more careful when we see vuln ranges that are "*" or ">=X".

[brendankenny]

Since we ship to places we can't necessarily quickly update (e.g. DevTools), I wonder if we should have a LH test that fails if the database has * or >=X, and require it be corrected to <= some hardcoded version (presumably the latest available release), even if that version is still vulnerable.

The theory would be: if a new, still vulnerable version of the library comes out after that LH release, then it's better to have a false negative (given that there's no fix available anyways and there are other sources (github, npm, greenkeeper, etc) that will warn folks) than it is to have up to 12 weeks of a false positive (if a new, non-vulnerable version comes out) which would annoy folks and encourage them to ignore Lighthouse's warnings in the future.

[patrickhulce]

Big +1 to @brendankenny's solution.

I'd much prefer a few temporary false negatives to some way to dynamically update LH from a remote URL we don't really have much control over.

[lirantal]

Indeed the weekly scheduled has missed it by a bit, and I also agree with @brendankenny as this is something we also do in the Node.js Security WG (use a <= upper boundary of last known version vs a * although tbh maybe for Node.js we should change that). Another thing to consider is perhaps to bump the update time to happen less than every week but I'm not sure what are the constraints for that (cc @aviadatsnyk)

[aviadatsnyk]

I don't know what the full considerations are, but we're happy to move to a model where users download the vulnerability snapshot from us (our CDN, obviously) on a regular basis, instead of getting it bundled. That way the info used would be more recent.

[aviadatsnyk]

Another option is just for us to open the vulnerabilities update PRs daily instead of weekly, if that is something that you think might work better.
@paulirish @patrickhulce @brendankenny @hoten

[patrickhulce]

Thanks for the ideas @aviadatsnyk! The problem is we have several environments where it's not really an option to dynamically update from a remote URL, so even more frequent PRs wouldn't address this specific issue.

[connorjclark]

We fixed this with 4.3.1. Preventing a similar situation is tracked in #8748.