SEMVER ranges could be flattened

[oliverchang]

Title

SEMVER ranges could be flattened

What is the problem this feature will solve?

Currently, OSV entries with multiple SEMVER ranges are provided separately:

e.g. https://github.com/bitnami/vulndb/blob/87b00414368ba008df70dae5141ed0c4ca63e6eb/data/wordpress/BIT-2023-39999.json

        {
          "type": "SEMVER",
          "events": [
            {
              "introduced": "4.1.0"
            },
            {
              "fixed": "4.1.38"
            }
          ]
        },
        {
          "type": "SEMVER",
          "events": [
            {
              "introduced": "4.2.0"
            },
            {
              "fixed": "4.2.35"
            }
          ]
        },
        {
          "type": "SEMVER",
          "events": [
            {
              "introduced": "4.3.0"
            },
            {
              "fixed": "4.3.31"
            }
          ]
        },

Instead, it's more concise (and preferred) to flatten these into:

        {
          "type": "SEMVER",
          "events": [
            {
              "introduced": "4.1.0"
            },
            {
              "fixed": "4.1.38"
            },
            {
              "introduced": "4.2.0"
            },
            {
              "fixed": "4.2.35"
            },
           ...
          ]
        },

What is the feature you are proposing to solve the problem?

Flatten the version ranges.

[gongomgra]

Hi @oliverchang,

Thanks for your suggestion. We have been testing the changes to solve this, but we are facing one of the conditions not allowed in the osv/schema.json file: there are mixed fixed and last_affected entries

$ check-jsonschema --verbose --color never --schemafile /vulndb/config/validation/schema.json /vulndb/data/wordpress/BIT-2023-2745.json

  /vulndb/data/wordpress/BIT-2023-2745.json::$.affected[0].ranges[0]: {'type': 'SEMVER', 'events': [{'introduced': '0'}, {'fixed': '4.1.38'}, {'introduced': '4.2.0'}, {'fixed': '4.2.35'}, {'introduced': '4.3.0'}, {'fixed': '4.3.31'}, {'introduced': '4.4.0'}, {'fixed': '4.4.30'}, {'introduced': '4.5.0'}, {'fixed': '4.5.29'}, {'introduced': '4.6.0'}, {'fixed': '4.6.26'}, {'introduced': '4.7.0'}, {'fixed': '4.7.26'}, {'introduced': '4.8.0'}, {'fixed': '4.8.22'}, {'introduced': '4.9.0'}, {'fixed': '4.9.23'}, {'introduced': '5.0.0'}, {'fixed': '5.0.19'}, {'introduced': '5.1.0'}, {'fixed': '5.1.16'}, {'introduced': '5.2.0'}, {'fixed': '5.2.18'}, {'introduced': '5.3.0'}, {'fixed': '5.3.15'}, {'introduced': '5.4.0'}, {'fixed': '5.4.13'}, {'introduced': '5.5.0'}, {'fixed': '5.5.12'}, {'introduced': '5.6.0'}, {'fixed': '5.6.11'}, {'introduced': '5.7.0'}, {'fixed': '5.7.9'}, {'introduced': '5.8.0'}, {'fixed': '5.8.7'}, {'introduced': '5.9.0'}, {'fixed': '5.9.6'}, {'introduced': '6.0.0'}, {'fixed': '6.0.4'}, {'introduced': '6.1.0'}, {'fixed': '6.1.2'}, {'introduced': '6.2.0'}, {'last_affected': '6.2.0'}]} should not be valid under {'properties': {'events': {'contains': {'required': ['fixed']}}}}

How should we handle these situations in our OSV files? Should we add the (introduced + last_affected) events in a different affected.ranges[] entry? You mentioned this is kind of an aesthetic suggestion, and I wonder if having to distinguish between events would make the resulting file more complex than simply having multiple entries by default as we currently have. What do you think?

[oliverchang]

there are mixed fixed and last_affected entries
Hmm, our reasoning there is that last_affected is redundant when there is fixed. last_affected is only there to serve as a best effort stop gap for when the fixed versions is unknown (or not available at the time).

Is there some reason that this information is important to express for you?

How should we handle these situations in our OSV files? Should we add the (introduced + last_affected) events in a different affected.ranges“[] entry? You mentioned this is kind of an aesthetic suggestion, and I wonder if having to distinguish between events would make the resulting file more complex than simply having multiple entries by default as we currently have. What do you think?

There are some cases where this can cause issues:

For instance:

{
  "type": "SEMVER",
  "events": [
    { "introduced": "0" },
  ]
},
{
  "type": "SEMVER",
  "events": [
    { "introduced": "2.0"
    { "fixed": "2.4" },
  ]
}

means a very different thing from:

{
  "type": "SEMVER",
  "events": [
    { "introduced": "0" },
    { "introduced": "2.0"
    { "fixed": "2.4" },
  ]
}

The former means all versions are always going to be affected, because the unclosed "introduced": "0" will take end up marking the entire version space as vulnerable, no matter what else is specified.

The latter would mean that versions after and including 2.4 are marked as unaffected.

[gongomgra]

Hi @oliverchang,

Thanks for your inputs. We chose to express all affected versions in the ranges.affected array to avoid redundancy, and for the case of an specific single version affected, we decided to include a new (introduced + last_affected) pair of events in a new affected entry. Do you think that's the correct way of handling these situations? Would it work if we group all the introduced and fixed events in an entry (flattening much of the current ranges.affected arrays), and the introduced and last_affected in a different one?

[oliverchang]

Hi @oliverchang,

Thanks for your inputs. We chose to express all affected versions in the ranges.affected array to avoid redundancy, and for the case of an specific single version affected, we decided to include a new (introduced + last_affected) pair of events in a new affected entry. Do you think that's the correct way of handling these situations? Would it work if we group all the introduced and fixed events in an entry (flattening much of the current ranges.affected arrays), and the introduced and last_affected in a different one?

For the single version case, it may be better to use the versions array instead. e.g. similar to https://github.com/github/advisory-database/blob/cec562bf92f76b05fe37b2953a3a2ae6ffab6647/advisories/github-reviewed/2022/05/GHSA-vrrc-3wwh-frgx/GHSA-vrrc-3wwh-frgx.json#L36

or

      "package": {
        "ecosystem": "Maven",
        "name": "org.jenkins-ci.plugins:mercurial"
      },
      "versions": [
        "2.11"
      ]

You can specify individual versions without encoding them as part of any range.

Having to mix last_affected and fixed in the same events array for the reason of encoding different branches is a good point though. This might actually be a place in the schema where we should loosen the rules a bit, and allow this (while encouraging less use of last_affected).

Would it work if we group all the introduced and fixed events in an entry (flattening much of the current ranges.affected arrays), and the introduced and last_affected in a different one?

Yes, this sounds like this would work, but the other two approaches (using versions instead, and/or loosening our schema validation) might be preferred.

[gongomgra]

Hi @oliverchang,

Thanks for your message. We will go with the two arrays approach for now as it solves the issue on our side and also simplifies the affected.ranges[] length. Do not hesitate to open a new request if the validating schema gets relaxed on this point so we can simplify this even more

[gongomgra]

We have updated the vulndb with the changes mentioned in this ticket (see pr/215). I'm closing this ticket.