Incorrect handling of product versions from OSV source

[gluesmith2021]

I found two issues with version numbers handling in OSV source parsing. As they are somewhat related, I decided to put them in a single issue here:

• bug: duplicated ranges instead of multiple ranges
  • DB is populated with identical rows, whereas different affected version ranges are expected for them
• probably incorrect: using "ecosystem" version for the product version.
  • Current behavior: ecosystem "versions" (that can be any string as well) show up in the database instead of product versions, and this can be misleading whenever a product version has to be checked against affected version ranges.
  • Expected behavior: unreliable versions or "non-version" strings do not appear in cve_range table for a product (perhaps not inserting the row at all). Even if the product is the ecosystem itself, "version" strings would then make sense but still be unusable programmatically.

Duplicated Ranges

According to OSV format, affected[].ranges[].events does not restrict child fields to be unique. For instance, there can be many "introduced" and "fixed" objects, as in https://osv-vulnerabilities.storage.googleapis.com/Android/ASB-A-219942275.json

However, event loop in actual code reuses the same affected object for each introduced/fixed occurrence. When parsing the above CVE, this same object is appended four times to the affected_data list. Then, when the DB is populated, it has four identical rows, all with the last version range encountered, instead of each version ranges.

A new object should be used each time instead.

ECOSYSTEM Versions

Many versions numbers should probably not be parsed to start with, because they are ecosystem versions, but that's open to interpretation.

According to the above OSV format again, if affected[].ranges[].type is ECOSYSTEM:

The versions introduced and fixed are arbitrary, uninterpreted strings specific to the package ecosystem, which does not conform to SemVer 2.0’s version ordering.

Basically, this is probably not the product version, and it can be anything, like a name. From the linked OSV sample, 12L:2022-09-01 is pushed to the DB as if it was a version of expat product.

So, in the same function as above, should such events be ignored as they are for "GIT" event type? affected[].versions would then be used instead.

But here's another problem: affected[].versions can be anything as well and is not even required to be specified when "ECOSYSTEM" type is used. Furthermore, as the documentations says:

The infrastructure and tooling provided by https://osv.dev also provides automation for auto-populating the versions list based on supported ECOSYSTEM ranges as part of the ingestion process.

In short, for "ECOSYSTEM" type, affected[].versions can be just as bad as affected[].ranges[].type to extract product version numbers. It might be best to ignore them rather than include them in a "best effort".

[terriko]

This sounds like a good set of ideas.

I don't think any of our current active contributors has time to put this on our plates before next release (our GSoC contributors, for example, have already started school for the year and are just finishing up their projects). So I just want to put it out there that I'd really love it if a new contributor wanted to tackle writing actual PRs for the improvements described here!

[terriko]

Update: @rhythmrx9 actually took a deeper look at this and thinks he might have time for a PR to fix the most obvious mistakes, so if he's able to that we might actually be able to fix it for the 3.3 release (estimated: mid to late sept)

[smclinden]

Is that why I am seeing the following error with the latest Git checkout.

│ /opt/hostedtoolcache/Python/3.11.4/x64/lib/python3.11/site-packages/cve_bin_ │
│ tool/data_sources/osv_source.py:323 in format_data                           │
│                                                                              │
│   320 │   │   │   │   │   │   events = ranges["events"]                      │
│   321 │   │   │   │                                                          │
│   322 │   │   │   │   if events is None:                                     │
│ ❱ 323 │   │   │   │   │   versions = package["versions"]                     │
│   324 │   │   │   │   │                                                      │
│   325 │   │   │   │   │   if versions == []:                                 │
│   326 │   │   │   │   │   │   continue                                       │
╰──────────────────────────────────────────────────────────────────────────────╯
KeyError: 'versions'
Error: Process completed with exit code 1.

[terriko]

Looks like we need a better check on line 323.

[terriko]

Looks like this was fixed some time ago, but if there's something that still needs doing feel free to re-open or file a new issue. I feel like we've got some issues with OSV at the moment and would be happy to get feedback on where else we can improve.

[jloehel]

@terriko Can you please re-open this issue. The second point of @gluesmith2021 is not fixed, right? In case of the mentioned example at https://osv-vulnerabilities.storage.googleapis.com/Android/ASB-A-219942275.json, the Android version is used and not the version of expat. For Kernel Patches like https://osv-vulnerabilities.storage.googleapis.com/Android/ASB-A-111893654.json, Kernel gets detected as Version:

Maybe the best choice is to ignore Android Security Bulletins till you have an extra table for those bulletins. But the bad thing is that the corresponding CVE at https://storage.googleapis.com/osv-test-cve-osv-conversion/osv-output/CVE-2022-22822.json will have no affected_data.

@anthonyharrison Has a good point in #2448 the ecosystem needs to be considered and saved. Parse the version ranges depending on the ecosystem. Would be cool to use the commit ranges feature from OSV (https://osv.dev/blog/posts/introducing-broad-c-c++-support/).