anchore / grype

A vulnerability scanner for container images and filesystems
Apache License 2.0
8.71k stars 571 forks source link

Difficulty with OpenJDK versions #1640

Open joshbressers opened 10 months ago

joshbressers commented 10 months ago

OpenJDK version 8 CPEs tend to have very odd version identifiers, Grype does not appear to be doing the right thing (I'm not entirely sure what the right thing is). There is also a Syft issue that is dealing with how to capture the OpenJDK versions https://github.com/anchore/syft/issues/2422

I am attaching a doctored SBOM (please do not read too much into these, they are modified to show my point)

The first is openjdk-8.json. This has an openjdk CPE

      "cpes": [
        "cpe:2.3:a:oracle:openjdk:8:update392:*:*:*:*:*:*"
      ],

openjdk-8.json

The second is openjdk-8-alt.json, the CPE is

      "cpes": [
        "cpe:2.3:a:oracle:openjdk:8-update392:*:*:*:*:*:*:*"
      ],

openjdk-8-alt.json

The difference is how the version is specified in the CPE. The first is 8:update392, the second is 8-update392. The reasoning for this will make sense in a bit

If we scan the openjdk-8.json file with Grype, we get 44 vulnerabilities. If we scan openjdk-8-alt.json we get 6. This was done to show the way we match on the versions seems to prefer the 8-update392 format with the data in Grype's database (I'm unsure which should be more right, I just leave this here to show one of the odd problems)

I also think focusing on CVE-2023-21930 is useful because it shows up in both scans, but shouldn't be in them. If we look at the NVD CPEs https://nvd.nist.gov/vuln/detail/CVE-2023-21930

update392 is not listed. This means we are matching on something else. If we look at the data in GrypeDB, this CPE is listed cpe:2.3:a:oracle:openjdk:*:*:*:*:*:*:*:* which probably shouldn't be in there (I don't know why it's there, that CPE isn't listed in NVD. I could be mistaken, but that seems to be the culprit.

This all appears to partially be a problem in openjdk versions in the SBOM, partially a problem in the NVD data itself (it is very obtuse), and partially how we turn the NVD data into Grype data

I'm unsure if this is missing anything important. Please let me know if anything isn't clear.

willmurphyscode commented 10 months ago

This means we are matching on something else. If we look at the data in GrypeDB, this CPE is listed cpe:2.3:a:oracle:openjdk:*:*:*:*:*:*:*:* which probably shouldn't be in there (I don't know why it's there, that CPE isn't listed in NVD.)

This isn't quite correct. The CPE cpe:2.3:a:oracle:openjdk:*:*:*:*:*:*:*:* is listed in NVD. It appears 3 different cpeMatch objects on the node in the fourth configuration (source: https://services.nvd.nist.gov/rest/json/cves/2.0?cveId=CVE-2023-21930 / https://nvd.nist.gov/vuln/detail/CVE-2023-21930):

                  {
                    "vulnerable": true,
                    "criteria": "cpe:2.3:a:oracle:openjdk:*:*:*:*:*:*:*:*",
                    "versionEndExcluding": "8",
                    "matchCriteriaId": "111E81BB-7D96-44EB-ACFA-415C3F3EA62A"
                  },
                  {
                    "vulnerable": true,
                    "criteria": "cpe:2.3:a:oracle:openjdk:*:*:*:*:*:*:*:*",
                    "versionStartIncluding": "11",
                    "versionEndIncluding": "11.0.18",
                    "matchCriteriaId": "90F6CEC5-2FD9-4ADB-9D86-B741C0ABCD7B"
                  },
                  {
                    "vulnerable": true,
                    "criteria": "cpe:2.3:a:oracle:openjdk:*:*:*:*:*:*:*:*",
                    "versionStartIncluding": "17",
                    "versionEndIncluding": "17.0.6",
                    "matchCriteriaId": "83395182-E46E-47FF-A781-4EF235BC83B6"
                  }

All of these have some additional version constraints specified outside the CPE criteria string.

There are a couple of different issues with syft/grype/grype-db contributing to this false positive I think:

  1. When Grype is using the CPE matcher, it uses the version field for comparison: https://github.com/anchore/grype/blob/844711285b22c0e6ac55fa7c3a20ecd9650aaf71/grype/search/cpe.go#L91 which means that a CPE like cpe:2.3:a:oracle:openjdk:8:update392:*:*:*:*:*:* will be version "8" in grype's CPE matcher. (Maybe it should use the "version" field and the "update" field together?)
  2. Grype DB itself collapses these CPEs and version constraints into a single row, which breaks the association between different CPEs strings and different version constraints (see below)
  3. OpenJDK has weird version numbers. (This last problem is probably really part of https://github.com/anchore/syft/issues/2422)

Here's what I mean about collapsing the CPE and version constraint:

(obtained by running qlite3 --json ~/Library/Caches/grype/db/5/vulnerability.db 'select * from vulnerability where id like "CVE-2023-21930" and namespace like "nvd:cpe" and package_name like "openjdk";' | jq .)

[
  {
    "pk": 265857,
    "id": "CVE-2023-21930",
    "package_name": "openjdk",
    "namespace": "nvd:cpe",
    "package_qualifiers": null,
    "version_constraint": "< 8 || >= 11, <= 11.0.18 || >= 17, <= 17.0.6 || = 8 || = 8-milestone1 || = 8-milestone2 || = 8-milestone3 || = 8-milestone4 || = 8-milestone5 || = 8-milestone6 || = 8-milestone7 ... snip ...",
    "version_format": "unknown",
    "cpes": "[\"cpe:2.3:a:oracle:openjdk:*:*:*:*:*:*:*:*\",\"cpe:2.3:a:oracle:openjdk:20:*:*:*:*:*:*:*\",\"cpe:2.3:a:oracle:openjdk:8:-:*:*:*:*:*:*\",\"cpe:2.3:a:oracle:openjdk:8:milestone1:*:*:*:*:*:*\",\"cpe:2.3:a:oracle:openjdk:8:milestone2:*:*:*:*:*:*\",\"cpe:2.3:a:oracle:openjdk:8:milestone3:*:*:*:*:*:*... snip ...."]",
    "related_vulnerabilities": null,
    "fixed_in_versions": null,
    "fix_state": "unknown",
    "advisories": null
  }
]

As you can see, there are a bunch of version constraint clauses joined by || (that is, but OR), and there are a bunch of CPE criteria strings like cpe:2.3:a:oracle:openjdk:*:*:*:*:*:*:*:*. The problem is that in the NVD data (see JSON above), the CPE cpe:2.3:a:oracle:openjdk:*:*:*:*:*:*:*:* is only supposed to be a match if the version is less than 8. But specific clauses of the version constraint are no longer associated with specific CPEs, so for example, something with version 8, because of || = 8 and the CPE cpe:2.3:a:oracle:openjdk:*:*:*:*:*:*:*:*, would match, even though there isn't a cpeMatch object in a configuration node on the CVE that results in a match. In other words, because the CPE match fields and version constraint fields are collapsed to a single row, a package matching the version number from one CPE node and the CPE expression from another node might be considered vulnerable, even though it doesn't match any specific node.

I'd be curious to hear if @westonsteimel has any more context here.

westonsteimel commented 10 months ago

When Grype is using the CPE matcher, it uses the version field for comparison: https://github.com/anchore/grype/blob/844711285b22c0e6ac55fa7c3a20ecd9650aaf71/grype/search/cpe.go#L91 which means that a CPE like cpe:2.3:a:oracle:openjdk:8:update392:::::: will be version "8" in grype's CPE matcher. (Maybe it should use the "version" field and the "update" field together?)

@willmurphyscode this is the main fix needed on the grype side here I think. In the past I adjusted the compiled version constraint expressions in grype-db to add on the cpe version update component to the version (with a -), but missed that grype also ignores the update component on CPE comparisons. I suspect this will help with quite a few of these instances. It should at least make the two cases that Josh has above produce the same output

westonsteimel commented 10 months ago

https://github.com/anchore/grype-db/pull/145 was the grype-db change for adding update component, so I think making the same change you mentioned in grype will make these matches significantly better overall

westonsteimel commented 10 months ago

The built constraint in the db seems correct given the data and you can basically ignore the CPEs in the db since they are largely unused apart from vendor/product

westonsteimel commented 10 months ago

@joshbressers , so in this case it is matching on the = 8 clause of the grype-db version_constraint because grype is currently dropping the update392 component when doing the comparison. Grype should be updated per @willmurphyscode suggestion above, so in this case should build the version 8-update392 which should no longer match any of the constraints

willmurphyscode commented 10 months ago

Thanks @westonsteimel! I think we may also need a special version comparator for OpenJDK versions (or enhance the fuzzy comparator in some way).

If add add a test like this:

        {
            name:       "openjdk 8 with updates, not vulnerable",
            version:    "8-update392",
            constraint: "< 8 || = 8",
            satisfied:  false,
        },

To the fuzzy version constraint tests at https://github.com/anchore/grype/blob/377ea3e0c628ad5404a1ed40b615894309d1a7a7/grype/version/fuzzy_constraint_test.go#L60, the test fails.

So I think 3 code changes are needed:

  1. Syft makes a better CPE for OpenJDK (described at https://github.com/anchore/syft/issues/2422#issuecomment-1863011080)
  2. Grype includes the update field of CPEs in its version construction, at least for some packages (change at https://github.com/anchore/grype/blob/844711285b22c0e6ac55fa7c3a20ecd9650aaf71/grype/search/cpe.go#L91, discussed above)
  3. Grype has a version comparison mechanism that understands the update field that will be present after change 2.
willmurphyscode commented 1 month ago

@wagoodman @joshbressers was this fixed by #1718 / #2114? Or what's left?

wagoodman commented 2 weeks ago

I think so! @joshbressers I'll let you have the final say here