Suggest ignoring a vulnerability by the package maintainer

[naugtur]

In a discussion of this: https://github.com/npm/rfcs/pull/18 @wesleytodd suggested I bring it up here to collect feedback on the feature itself, but mostly to ask one thing:

If this can be leveraged for the package maintainers to declare a vulnerability in their own dependency does not affect the security of the package. And if so - how would you want to indicate that as opposed to ignoring the issue for your internal needs of stopping the CI from failing while there's no fix to update to.

[MarcinHoppe]

I think this would be a very useful option to have to increase the signal to noise ratio for vulnerability reports.

[naugtur]

Any thoughts on how to do it? I see two options

• special option in audit-resolve file
• a field in package.json

Second option has the appeal of it being crawled by package managers when creating the lock file anyway, so the info could be pulled from there.

[ljharb]

I’ve suggested this to npm many times; the challenge/pushback I’ve heard is that, how would you stop a malicious maintainer from hiding a cve report for the purposes of leaving users vulnerable?

[MarcinHoppe]

Malicious maintainer has more direct opportunities for embedding malware in their package, IMO.

[naugtur]

And these would not be ignored by default but provides as hint for decision making. Just a field on audit output.

[mhdawson]

Sounds like a good idea. I saw a tweet today from @lirantal mentioning that Snyk is doing something to try an figure that out automatically.

One way I could see that fitting with that suggestion is that a maintainer could use the info from Synck to populate the data that needs to be provided so that all users of the module benefit.

[wesleytodd]

how would you stop a malicious maintainer from hiding a cve report for the purposes of leaving users vulnerable?

Maybe the question should be phrased as "lazy maintainer". I fully agree that if you have a malicious maintainer you are owned already, but what incentivizes a lazy maintainer to not just ignore all at their level?

That said, I think optimizing for this behavior means we might miss out on the more impactful feature here, which will save "good" maintainers from this burden they have today. So I don't think the answer to this question should block anything as long as we are not making the situation worse.

---

So the format I think this should move forward is as a spec for the json file. I think the implementation of it in npm will be awesome, but to get broad adoption and enable other tooling ecosystems (yarn, snyk, etc) to build on it we would want an official spec to live somewhere. I think this group is a find place for that to live if @naugtur agrees.

[ljharb]

However, if npm and github (the tools who we'd need to respect this ignore setting) don't consider that a worthy tradeoff, then it's not really up to us :-/

[naugtur]

There was a suggestion on today's RFC meeting to donate the package for embedders with the JSON schema within to an org. I'm on board with that.

If there's a place to put the tiny schema for the file, I'd love to do it. Being in control of it was fun and all, but that's not the point.
Looking forward to suggestions.

[naugtur]

However, if npm and github (the tools who we'd need to respect this ignore setting) don't consider that a worthy tradeoff, then it's not really up to us :-/

It's up to us to come up with something for them to consider and I believe it's likely to succeed coming from the fine folks here ;)

[ljharb]

I certainly hope so; however the reasons I explained are why npm has said no to me on multiple occasions for this very request, so I'm not sure why they'd have a different opinion now.

[naugtur]

Both the people involved and the NPM Inc. are different. If we come up with something reasonable, I'm pretty sure it'll get introduced into audit.

There's also an idea in the RFCs to audit licenses, which will require going to package.jsons in the tree and checking something, I think gathering vulnerability resolution recommendations should be doable in the same pass.

[ljharb]

npm, inc. has 100% full control over npm audit, so i'm not sure what you mean.

We can try to persuade, and we should, but it's entirely their decision.

[naugtur]

I never said we're the decision makers here. It's not something that'd stop me. I came here to collect some opinions on what'd work and maybe put some of it in my RFC that's older than my now fluently speaking kid. I'm doing it in my rare spare time. I've got other unusual hobbies too :wink:

Would you mind sharing the details of what you proposed before and what you think about it nowadays?

[ljharb]

I didn't come up with a concrete proposal; I've just had multiple conversations where I wanted, as a maintainer, to be able to say "CVE XXX does not apply to my package", and then ensure that "my package" is never the reason a user sees a warning about that CVE.

[lirantal]

Glad to join this discussion and indeed @ljharb and me spoke about this as part of his feedback for me about a year back.

@ljharb your feedback from past discussions with npm is weird to me (on their part). I'm referring to:

how would you stop a malicious maintainer from hiding a cve report for the purposes of leaving users vulnerable?

It's a weird statement in the sense that a maintainer could argue the same about a direct vulnerability in their package and claim "it doesnt reproduce/not relevant/etc" but that doesn't hold because the triage process validates the vulnerability and in the same way I expect that if a maintainer wants to "waive off" an indirect vulnerability, they'd need to follow the same and show evidence that there's no code paths/reproducing that works in order to deem a vulnerability as irrelevant.

What I'm trying to say here is that the burden of proof is going to be here on the maintainer, which to me sounds reasonable but also may add more complexity/effort on their part in some occassions.

About this:

However, if npm and github (the tools who we'd need to respect this ignore setting) don't consider that a worthy tradeoff, then it's not really up to us :-/

Last year we had a heated discussion on twitter on this with a bunch of folks and this is essentially what I was saying that this is a bigger ecosystem problem with the way that CVEs work. Meaning to say - even if today a maintainer would've reached out to Snyk to say an indirect CVE is not reachable in their package and Snyk would indeed stop showing it for its users, this solution would still be contained to only those who use Snyk. If you use GitHub or npm you'd still receive the alerts. That's why I have claimed several times that this is a broader issue with the way CVEs work and the complexity they create for indirect packages. You can't dismiss the CVE and you'd need to have the proper tooling/database that says a CVE isn't reachable to dismiss the vuln.

Generally speaking, with regards to what can be done - I think that even if for the Node.js Security WG for example we wanted to allow maintainers to opt-in and dismiss a vulnerability in their indirect package, I see the following complexities:

1. it wouldn't really help anyone since the actual tooling would need to align: npm audit, github, snyk, etc
2. who is going to assume the accountability for dismissing based on a maintainer's word? they could be wrong or simply not understanding the involved attack vectors and we'd be putting users in risk. maybe there are super obvious cases but I think those are the 20%, so what do we do about the 80% of cases where deeper security expertise is required?

My feel is that if 20% of the cases are very obvious and straight-forward to maintain and also account for the 80% of the ecosystem noise that will happen then it's still a good investment to focus there as it will ease maintainers and users a lot. Next would be to base on some sort of SAST tool to validate the findings of unreachability for the 80% of cases which are more complex.

[ljharb]

I totally agree the real issue is with CVEs and the security industry built up around them, and not with any individual tool.

npm, github, and snyk, at least, would surely be able to align (if they all agreed it's a problem worth solving), and the rest would follow suit. The second point, however, is exactly why my many suggestions over the years to npm for this have not resulted in any changes.

[naugtur]

It's midnight and I was catching up on the discussion. A midnight crazy thought: what if instead of putting the ignore recommendation in the package itself we did something along the model of definitely typed repository with a collection of reports from people or even a voting/rating system where you could form a web of trust and decide you're going to ignore vulns that certain people also ignore?

It'd be fairly easy to add to an audit resolver tool - "3 people you like ignored this vulnerability"

[mhdawson]

Discussed in the package-maintenance team meeting, we thought it would be good to invite people from npm/github, snyk to our next meeting.

Maybe @MylesBorins, @darcyclarke, @lirantal to see if there is any opportunity to work together on the issue.

[naugtur]

maybe @isaacs and @wesleytodd too as they were the ones suggesting to involve this group.

[darcyclarke]

Apologize I couldn't make it today. I'll make sure I can attend next week.

[dominykas]

Would it be enough (or at least a good starting point) if the tooling gave a "Developer response" link as part of the audit report printed in the console?

Not sure whether the package.json or the audit.json would be the right place to store that link (I suspect not, as that requires an npm publish), but advisories usually already link back to discussions/PRs, etc - so bubbling that up and making it more prominent could be a step in the right direction?

[naugtur]

Audit currently only reads package-lock, installation process involves going through the tree of package.json files, so at least there's battle tested code that does that...

I'd see it as a section in package.json if it's supposed to affect what audit displays

[dominykas]

If we're talking about maintainers providing information on vulnerabilities in their package, then audit would have to fetch the package tarball to get the audit.json? And it would have to fetch the packument to read the vuln info from there? Not sure the lock files have space for this information?

Either way, I would think authors should have write access to add this info in the vulnerability db, which the audit tool fetches?

[dominykas]

(sorry, fat finger on the phone)

[mhdawson]

We should have Darcy at the next meeting to represent npm. I talked to Adam Baldwin who can't make it but he'll catch up with Darcy between now and the next meeting. Still hoping we'll get @lirantal but piing here as I'll be out most of the time befor the next meeting and won't be able to follow up.

[Eomm]

Just to recap if I understood the case:

1. there is a CVE disclosure for package broken used by package bar
2. bar package's users receive a security warning
3. the bar's user may:
   1. run npm audit fix if a fix has been released ✔
   2. open an issue if the command doesn't solve
4. the bar maintainer may:
   1. if possible, release a new version with a fix if broken has not been released jet ✔
   2. ignore the CVE because the bar package doesn't expose the users to the issue ✨
   3. ignore the CVE hoping the broken package will release a fix and npm audit fix will do the job ⏳

If we put the ignored CVEs in the package.json, would not mean that the maintainer should bump a new release? Is this necessary? I think this would work if npm would implement the @ljharb's idea when we discussed the support.json file: let the maintainer updates some metadata of the package.json without bumping the version

My simple idea would improve the security tab in the GH repository (sorry gitlab..): as we have https://github.com/<org>/<package>/issue/123 we could have https://github.com/<org>/<package>/security/CVE-2020-7608 that the maintainer may close as "not an issue". The tools should check the security issue state in the package repo when the audit command finds a warning

[ljharb]

Yes, currently the maintainer has to bump a release, and since lockfiles aren’t published, it can only be done serially, one package at a time up the tree (when semver doesn’t cover it).

Note that even if semver does cover it, packages in between with a lockfile will still get noise reports, even if the vuln doesn’t actually apply.

[boneskull]

I want to know if my lib/app is actually hitting vulnerable code, and cannot envision being satisfied with any solution that falls short of this. squelching warnings for every vuln that comes up is still extra work. I don't want extra work.

also: I want to emphasize the difficulty of solving the above problem. it is not as simple as code coverage or static analysis. (but either of those things would get the false positive rate down!)

yet, if we were able to reduce the number of times a maintainer (or downstream consumer) must step in to make a change, then we have made progress. personally, I'd rather see a move in this direction. it's less disruptive to spend 25 minutes resolving one problem than to spend 5 minutes each resolving 5 problems.

take this with a grain of salt. npm, snyk, etc will decide if it is worth throwing resources at. (also: excuse me; I am a curmudgeon. it is good to see us trying to work together on this.)

[naugtur]

Not an ad and I'm not affiliated with snyk beyond having talked to two people working there. And I got magic wands with their logo.

Snyk has already published a feature that detects if the vulnerability is reachable from the app. For some other languages :) I know they're working on doing it in JS too.

Does that affect your opinion?
How does that change the case for people not using tools beyond package manager?

[boneskull]

It does not affect my opinion until it lands for JS, is open source (we need everybody using naive security checks to use something smarter, and they are not all Snyk users), and works as advertised.

[boneskull]

Are there "next steps" on this?

[mhdawson]

I'm talking with a few people to see if they will champion a "collaboration space" focused on agreeing/documenting the path forward. Have not quite closed on that, but hoping that we spin that up as the next step.

[mhdawson]

• Collaboration space has kicked off here.
• First meeting was held: https://github.com/openjs-foundation/pkg-vuln-collab-space
• Closing issue as expect discussion to move over to that repo

Please let us know if you think closing this was not the right thing to do.