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

Difficulty with OpenJDK versions #1640

Closed
joshbressers opened this issue Dec 18, 2023 · 9 comments
Closed

Difficulty with OpenJDK versions #1640

joshbressers opened this issue Dec 18, 2023 · 9 comments
Labels
bug Something isn't working changelog-ignore Don't include this issue in the release changelog

Comments

@joshbressers
Copy link
Contributor

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
anchore/syft#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.

@joshbressers joshbressers added the bug Something isn't working label Dec 18, 2023
@willmurphyscode
Copy link
Contributor

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:
    searchVersion := c.Version
    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 OpenJDK CPEs syft#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
Copy link
Contributor

westonsteimel commented Dec 18, 2023

When Grype is using the CPE matcher, it uses the version field for comparison:

searchVersion := c.Version
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
Copy link
Contributor

anchore/grype-db#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
Copy link
Contributor

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
Copy link
Contributor

@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
Copy link
Contributor

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

func TestFuzzyConstraintSatisfaction(t *testing.T) {
, the test fails.

So I think 3 code changes are needed:

  1. Syft makes a better CPE for OpenJDK (described at OpenJDK CPEs syft#2422 (comment))
  2. Grype includes the update field of CPEs in its version construction, at least for some packages (change at
    searchVersion := c.Version
    , discussed above)
  3. Grype has a version comparison mechanism that understands the update field that will be present after change 2.

@willmurphyscode
Copy link
Contributor

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

@wagoodman
Copy link
Contributor

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

@willmurphyscode willmurphyscode moved this to In Review in OSS Oct 14, 2024
@joshbressers
Copy link
Contributor Author

This appears to do what I would expect. Both of my examples are using the same version now, which is what I would expect

Thanks!

@willmurphyscode willmurphyscode added changelog-ignore Don't include this issue in the release changelog and removed needs-investigation labels Nov 21, 2024
@github-project-automation github-project-automation bot moved this from In Review to Done in OSS Nov 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working changelog-ignore Don't include this issue in the release changelog
Projects
Status: Done
Development

No branches or pull requests

5 participants