Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

discussion about double-reporting #580

Closed
flavorjones opened this issue Apr 19, 2023 · 7 comments
Closed

discussion about double-reporting #580

flavorjones opened this issue Apr 19, 2023 · 7 comments

Comments

@flavorjones
Copy link
Contributor

Hi all,

Thanks as always for maintaining an incredibly important project. I appreciate you all!

I'm opening this issue to:

  • point out an instance of double-reporting of a CVE,
  • to suggest that it will likely happen again (and may have already happened, I didn't check thoroughly),
  • and ask if I should change my reporting practices as a gem maintainer.

In commit febf9e8, @reedloden added the contents of GHSA-7rrm-v45f-jp64 for nokogiri. This is a complicated report in that it references multiple CVEs in libxml2 that were patched by upgrading the vendored libxml2. One of those CVEs is CVE-2021-3517.

Recently, in commit, 9bfad4b, @reedloden synced with GHSA and added an individual record for CVE-2021-3517. So now the database has two records for the same CVE, and downstream consumers of the db are likely creating double notifications for projects.

How would the maintainers like to handle cases like this? Is what happened totally fine? Please note that I've been creating GHSAs for Nokogiri that reference multiple CVEs when new libxml2 versions drop, so this seems likely to happen again.

It's likely not a big deal to double-report, but given that the CVE was with a packaged library and not Nokogiri, and now there are two records attached to Nokogiri, my urge to shout "someone is wrong on the internet!" is kicking in. 😆 😭

@jasnow
Copy link
Contributor

jasnow commented Apr 19, 2023

Yes, I have another case I am trying to avoid syncing duplicates:

gems/spree_auth_devise/GHSA-6mqr-q86q-6gwr.yml
gems/spree_auth_devise/GHSA-8xfw-5q82-3652.yml
gems/spree_auth_devise/GHSA-gpqc-4pp7-5954.yml

I wrote GHSA issues and they rejected them as duplicates but they were not ready to delete them.

https://github.com/github/advisory-database/pull/2029
https://github.com/github/advisory-database/pull/2028
https://github.com/github/advisory-database/pull/2027

@jasnow
Copy link
Contributor

jasnow commented Apr 19, 2023

Yes, GHSA database has them as duplicates too: https://github.com/advisories?query=CVE-2021-3517

@postmodern
Copy link
Member

I took a stab at this in PR #585. I wrote specs which load all YAML from a gems/*/ or rubies/*/ directory and checks for duplicate ghsa: or cve: values. So far it's caught one: gems/yard/CVE-2019-1020001.yml and gems/yard/GHSA-xfhh-rx56-rxcr.yml have the same GHSA ID. Although my specs do not flag gems/nokogiri/GHSA-7rrm-v45f-jp64.yml, since that advisory doesn't set cve: and gems/nokogiri/CVE-2021-3517.yml sets a different ghsa: ID than 7rrm-v45f-jp64.

As for gems/nokogiri/GHSA-7rrm-v45f-jp64.yml, I say we should split it up into multiple CVE-YYYY-XXXX.yml files and ensure a 1:1 mapping of advisory to vulnerability and not group them together.

@flavorjones
Copy link
Contributor Author

flavorjones commented Apr 21, 2023

For the Nokogiri use case, it's common that libxml2 ships a version with multiple CVE fixes, and since these CVEs are not against Nokogiri itself I have been publishing a single GHSA to cover all the CVEs. A further annoyance is that frequently, CVEs are created for libxml2 after a release is made, and in that case it's easy to go back to the GHSA and update it with the new identifier.

that advisory doesn't set cve:

It's not set because there are multiple CVEs in the GHSA, and none of them are against Nokogiri (they're against libxml2).

gems/nokogiri/CVE-2021-3517.yml sets a different ghsa: ID than 7rrm-v45f-jp64

This is yet another source of anxiety for me: duplicate GHSAs being created years after the original incident.

As for gems/nokogiri/GHSA-7rrm-v45f-jp64.yml, I say we should split it up into multiple CVE-YYYY-XXXX.yml files and ensure a 1:1 mapping of advisory to vulnerability and not group them together.

Allow me to generalize it and say it back to you to see if I'm understanding the decision:

"If a GHSA references multiple CVEs, even if they are against another project, we should prefer creating multiple CVE files that reference the GHSA url (rather than creating a single GHSA file that references multiple CVEs)."

This seems like a pragmatic decision, although it feels like a bit more work than I was hoping for.

@jasnow
Copy link
Contributor

jasnow commented Apr 21, 2023

Could the

related:
  cve:

be of use in this case? (See example at the bottom of this repo's README file).

@flavorjones
Copy link
Contributor Author

@jasnow Thanks for the suggestion, I think in the next GHSA I create I will use that field to report related CVEs and maybe we can make sure dedup checks like #585 look in that field.

I'm happy to close this, but will wait a few days for any final thoughts.

@jasnow
Copy link
Contributor

jasnow commented Jun 30, 2023

@flavorjones and/or @postmodern - If you have any test cases associated with this issue including word descriptions or shell script query(ie. issue 662), feel free to add them as a comment.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants