fix: improve handling of triage data

[r-vdp]

Description

Before this change, there were two issues when using an SBOM file
together with a VEX file for triage:

1. new CVEs for a product for which there were existing CVEs in the
   triage file, were not added to the triage file
2. triage info recorded in the triage file was overwritten when
   cve-bin-tool was executed with both the --triage-file and vex
   options.

This commit fixes both issues by:

1. still scanning for CVEs even if the product was already present in
   the triage file. Before we would not scan for CVEs as soon as we
   found that the product was already present, but by doing so we might
   miss new CVEs since the last time we did a scan.
2. merge recorded triage info into the CVEs that we found for the SBOM
   components.

In order to properly identify products, I implemented the hash and eq
methods on the ProductInfo type to not consider the location field
as this field does not seem to be populated in a consistent manner.

Fixes #4158

Checklist

• Add checker
• Add test
• Make sure long tests are passing
• Add condensed downloads to the commit
• Make sure all tests are passing
• Make sure black/isort tests are passing
• Run a manual test with a vulnerable version of the product
• Update the template for this checker
• Update the reference links

[mastersans]

@r-vdp It seems like the test are failing due to difference in expected and actual severity of test_exploit.py I looked into the changes not sure what exactly is causing it, also we use gitlint as one of the linter so "fix: handling of triage data" both in commit message and PR title should fix gitlint fail and black should be fixed by running black filename.py.

[r-vdp]

I pushed a commit that should fix the tests. There are others that fail locally for me, but I think that might be an environment issue, let's see.

I think we'll need to refactor the scanner a bit though because the current state is quite complex, and I feel that we don't have the best data model to track the CVE data obtained from different sources and from the triage. I think that it would be better to make this more explicit in the data model so that we can make more informed decisions on which source should get priority when they don't agree.

We probably also want to add a test case for the issue that this PR resolves, so that it doesn't get broken again.

I'm not sure that I'll have time to do either of these in the short term.

[terriko]

@r-vdp Thanks for working on this!

I think it's valuable to have this as it is even if we probably need a refactor and a test later. Did you want to file an issue with some notes about what you think we'll need in the refactor? I can file an issue about the test when this merges if you want.

Did you want to tidy up the linter issues to get this into a mergeable state, or were you hoping one of us could handle that? I'm happy to do it if you're too busy, but I don't want to push to your branch if you're still working on this!

[r-vdp]

Hi @terriko, thanks for your feedback!

Yeah I do think that we can merge before the refactor indeed, but the scanning loop does feel rather brittle right now, so I wouldn't be surprised that we find other bugs still.
I can think a bit about a proposal for the refactor and write it down in an issue, but probably not before next week.

For this PR, I have been looking at the remaining test that's failing, but I don't yet understand what's the issue. If you have some time, maybe it would be clearer for you what's going on there? Otherwise I can spend some more time on that next week.

The linting issues are easy to fix, I think, we can do that once the tests are passing, I just didn't give that priority so far because I wanted to focus on the failing test, and I need to check how to run the linters locally so that I can verify that it's ok. Probably also next week.

[terriko]

Marking this as needed changes so I won't keep glancing at it.

Also, you should have an invite for a cve-bin-tool-read team . Accepting it will allow CI on your pull requests to run automatically so you can iterate faster without having to wait for me to approve CI on the PR every time you make a change. I realize that's not entirely obvious from the name, but that's what it's about!

[terriko]

Not sure what's happening with the cve scan here. Our current version of plotly.js is v2.27.1 and the latest CVE I see for that component is https://www.cvedetails.com/cve/CVE-2023-46308/ that's for versions < 2.25.2. So, the good news is it's probably not a component that actually needs an upgrade, but the bad news is that I'm not sure why this would be showing up at all.

I've got to run to a meeting, but I'll try to do some investigation later today. If you just want it to work for now, feel free to throw a triage file in there to make the test pass while we figure it out.

[mastersans]

@terriko the plotly.js is coming from the:

cve-bin-tool/triage.json

Line 76 in 2941eef

"ref": "urn:cbt:1/plotly#plotly.js-2.13.2"

triage.json being used in test_requirement.py as triage-input since the code mentioned tries to scan for the cves even if the triage data is present, apologies for not interpreting the changes in greater depth , cve_scanner is a little complex to read but this particular line is what i suspect is causing error hope it helps!

[mastersans]

so i mostly got it, firstly the changes are useful as it is in my opinion and can be used exactly, secondly the issue is arising due to the new found vulnerability in the plotly since this pr change s to scan both vex and the other data for cves, earlier the vex was not being scanned for cves as mentioned so the triage.json file was commited in 2022 and the cve is reported for CVE-2023-46308 was published 4 Jan 2024, also the version of plotly in triage file is quite older then the current version being used. the test passes when we also add the data for CVE-2023-46308 in the triage file with not affected i mean almost pass since it is not affected with space in between.

either of two things would fix the error:

• Adding the data for the plotly.js field for CVE-2023-46308 in triage file
• Or removing the plotly.js entirely since the the version of plotly is very old(1.3.2) in triage file and current being used is 5.22.0

cc @terriko @anthonyharrison @r-vdp

[terriko]

Okay, @mastersans has removed the problematic triage entry in #4267 so i think this is safe to merge. We'll likely want to handle old triage more seamlessly (we're still debating what sort of message and what config options we need for ignoring) but that can come in a future PR.

Thanks again for working on this @r-vdp and sorry it took so long to get it merged!

[terriko]

Darn, won't let me merge without re-running tests, so I'll be back when they're done (or more likely, tomorrow)

[r-vdp]

Okay, @mastersans has removed the problematic triage entry in #4267 so i think this is safe to merge. We'll likely want to handle old triage more seamlessly (we're still debating what sort of message and what config options we need for ignoring) but that can come in a future PR.

Thanks again for working on this @r-vdp and sorry it took so long to get it merged!

That's great!

No worries at all, I also got swamped with other things, so thanks a lot to both of you for fixing this up!