Change compareVersion behaviour

[florentulve]

#1832 NA in VulnSoftware only match when target version is ANY

[nscuro]

Thanks for the PR @florentulve! Makes sense to me at first glance.

@officerNordberg, as you dabble with CPEs a lot, may I ask for you opinion on this?

[officerNordberg]

I'm torn by this. The test case is an exact problem that we've wrestled with. I chose to handle this with a strategy similar to @nscuro 's project for managing audit results via code and policies. I like the results this code change gives but it does not match the CPE matching spec as I read it. https://nvlpubs.nist.gov/nistpubs/Legacy/IR/nistir7696.pdf So either I'm misreading it or we'd be choosing to go against the spec because NIST has terrible data for older CVEs.

[ChaosInTheCRD]

Hey :) I am probably misunderstanding, but shouldn't

        if (LogicalValue.NA.getAbbreviation().equals(vs.getVersion()) && LogicalValue.ANY.equals(targetVersion) ) {

instead be

        if (LogicalValue.NA.getAbbreviation().equals(targetVersion) && LogicalValue.ANY.equals(targetVersion) ) {

Reason being is that in this instance, the first if argument doesn't compare at all to the target version...

[ChaosInTheCRD]

Oh of course, I think I get it. Is the first argument in the if saying "if there is an NA in the VulnSoftware", then let me know of the vulnerability.

Correct me if I am wrong, but do I not want to be given back ALL of the VulnSoftwares if the targetVersion is ANY? By definition, I am saying "my target version could be anything", and therefore Dependency Track could not possibly be able to distinguish for me which vulns I do and don't care about

[florentulve]

Could we simply drop the first lines of compareVersion which handles logical value NA cause I've noticed CPE matching seems to be handled correctly lower by compareAttributes ?

[syalioune]

Following discussion #2188, I think that an agreement should be reached @nscuro @stevespringett.

In any case, there is indeed another bug as comparing cpe:2.3:a:xiph:speex:1.2:-:*:*:*:*:*:* to cpe:2.3:a:xiph:speex:1.2:-:*:*:*:*:*:* lead to a no match because

dependency-track/src/main/java/org/dependencytrack/tasks/scanners/AbstractVulnerableSoftwareAnalysisTask.java

Lines 201 to 203 in 836da60

	if (LogicalValue.NA.getAbbreviation().equals(vs.getUpdate())) {
	return false;
	}

happens before

dependency-track/src/main/java/org/dependencytrack/tasks/scanners/AbstractVulnerableSoftwareAnalysisTask.java

Line 216 in 836da60

return compareAttributes(vs.getUpdate(), targetUpdate);

can detect the exact match of target between vulnerable software and component.

[syalioune]

My understanding as per https://nvlpubs.nist.gov/nistpubs/Legacy/IR/nistir7696.pdf is that cpe:2.3:a:apache:http_server:-:*:*:*:*:*:*:* should not match cpe:2.3:a:apache:http_server:2.4.53:*:*:*:*:*:*:*

[melba-lopez]

@syalioune @florentulve This PR has been inactive for some time. Can I proceed with closing this? otherwise, can you update your branch?

[syalioune]

Hello @melba-lopez
Sorry for the late answer. I think the blocking point on this PR is not a technical one but a functional one.

With :

• Source (NVD) : cpe:2.3:a:apache:http_server:-:*:*:*:*:*:*:*
• Target (Component CPE) : cpe:2.3:a:apache:http_server:*:*:*:*:*:*:*:*

we fall under the fifth case of NISTIR 7696 Table 6-2

Component CPE is therefore a superset of NVD vulnerable software and thus it should not be a match.

If the above is correct and strict spec enforcement is what we want to achieve, then that PR should be updated/merged. Otherwise, it should be closed.

[melba-lopez]

@syalioune thanks for this. I've not dived deep into CPE before, so this is a good nugget for me!

In the same file you altered, I see the following quote from @stevespringett:

 * This does not follow the spec precisely because ANY compared to NA is
     * classified as undefined by the spec; however, in this implementation ANY
     * will match NA and return true.
     *
     * This will compare the left value to the right value and return true if
     * the left matches the right. Note that it is possible that the right would
     * not match the left value.

Based on the pdf and your screenshot, (if i understand this comment correctly) ANY compared to NA should be a superset. Even if i reverse the options it results in a subset and does not say "undefined"; only 4,8,11,17 say undefined.
It is not clear to me which use cases steve intended to comment on, since its not matching (based on my interpretation).

Perhaps we do need to take a look at the code a little bit more to make sure all use cases are covered and also identify it via comments in the code. Thoughts @nscuro ?

[Jasper-Ben]

Hi all,

maybe a bit OT for this merge request, but @syalioune directed me here.

I have been doing a deep dive into the CPE Name Matching specification, due to incomplete component CPE information leading to false negatives (attributes containing "ANY" values), as (for example) described at #2580. While component SHOULD have a information-complete CPE, it struck me odd, that a "ANY" value in an target attribute should lead to a no-match.

For example: Why would a CVE CPE with attribute TargetHW=x86 not be a match for an otherwise identical component CPE which has TargetHW="ANY"? Logically, the CVE SHOULD be a match for the component since any other decision would lead to false negatives.

It then struck me: The root cause for false negatives is the decision that DT does not consider CPE name relations being a SUBSET as a match. If we would consider SUBSET relations a match (that is cases 5, 13 and 15), the problem of false negatives due to incomplete data will go away, as described in #2988.

The flip-side however is, that inaccurate component CPEs will lead to false positives. This is to be expected. If your component reports TargetHW as "ANY" but in reality only runs on x86, then it will logically cause false positives for a CVE CPE with TargetHW set as arm64. That is why accurate component CPEs matters.

[Jasper-Ben]

I guess what I'm trying to say is that we should maybe take a step back and re-evaluate (and potentially re-align with NIST standard) the current CPE matching behavior as a whole.

(and that I disagree with @syalioune on #1929 (comment) based on my case made in #2988 😉)

[Jasper-Ben]

I might be stating the obvious here, but are we aware that there is CPE Name Matching Pseudocode in the NIST standard itself?

[nscuro]

Thanks all for the research and critical looks at this area!

The dilemma that I personally have with altering the existing logic is that it's very hard to estimate the impact it will have on users. Will they be flooded by false positives due to bad CPE data in both the SBOMs they ingest and the NVD? The fact that we have not nearly enough tests around this (#2243) just makes this worse.

The NVD's CPE data is also notoriously bad, which to my understanding is what resulted in DT's "non-standard" behavior as it is today. I am very much in favor of revisiting our matching logic, and harden it with silly amounts of tests. And personally, I'd love it to be aligned with the specification as much as possible.

As a side note, the fact that we have lots of different interpretations (including mine, which I haven't posted) of something that is considered to be an official specification in this thread alone is slightly amusing.

[Jasper-Ben]

Hi @nscuro,

The dilemma that I personally have with altering the existing logic is that it's very hard to estimate the impact it will have on users. Will they be flooded by false positives due to bad CPE data in both the SBOMs they ingest and the NVD?

I understand your concern, however I for one (and of course I can only speak for myself) would rather have false positives to deal with than false negatives lulling me into a false sense of security. And as it currently stands there definitely are loads of false negatives the way matching is implemented right now.

The NVD's CPE data is also notoriously bad, which to my understanding is what resulted in DT's "non-standard" behavior as it is today.

I obviously haven't had a lot of dealing with CPE so far, is this really the case? How do other projects deal with this? Are we sure it isn't just a case of e.g. interpreting old CPE 2.2 data (wrong)?

As it currently stands, the CPE matching mechanism is a hard blocker for our use-case (Yocto embedded linux), due to a lack of complete CPE data. I have also raised this issue with them, but even if this is resolved on Yocto's site (which will be quite some pain), I fear this is a huge issue for other use-cases as well.

[Jasper-Ben]

If we cannot agree on how SUBSETs are handled, maybe we could introduce that as a configurable option (similar to fuzzy)?

Since the subset case only occurs when the component (target) CPE is incomplete, aka uses "ANY" where CVE (source) CPE doesn't, the subset question boils down to "How do I treat incomplete component CPEs" with two options:

a) no-match with the risk of having false negatives
b) match with the risk of having false positives

So this can be put behind a switch that is easily understood by the user.

[Jasper-Ben]

@nscuro should we maybe organize a call regarding the general topic of "CPE matching"? I feel like the current situation leaves much to be desired and we (i.e. the company I work for) might be able to commit resources into getting this back on track. But as it stands I am not confident in getting the ball rolling, if we cannot agree on a desired output and end up spending resources on something that will end up not be merged back into DT.

[Jasper-Ben]

👋 @nscuro Thanks again for looking into this!

However, the NVD search results include CVEs where the Linux Kernel is merely mentioned as "Running On" configuration, and the vulnerable components is completely different [...]. I'd argue that DT is doing the right thing in not reporting those.

Completely agree! DT should only report on components actually present.

So what is happening is, we first mirror the correct, current data, only to override it with stale data a bit later.

Nice catch!

A short-term "fix" would be to simply mirror the modified feed at last again. [...]
Perhaps we should just migrate to the CVE API 2.0 (#1861) instead of putting too much effort into fixing the legacy mirroring behavior.

Agreed, although we should use the mentioned short-term workaround for the immediate fix.

[nscuro]

@Jasper-Ben I think I resolved most of the issues in #3070. Currently working on thorough testing, which includes digging up ancient issues that caused DT's matching logic to be how it is right now. Work is close to be ready for review.

[nscuro]

Anyone in this thread willing to review #3070, or give those changes a spin? I can push container images to Docker Hub if necessary.

[Jasper-Ben]

Anyone in this thread willing to review #3070, or give those changes a spin? I can push container images to Docker Hub if necessary.

Sure thing! Just give me an image to test 👍

[nscuro]

Image pushed to dependencytrack/apiserver:3070-fix-cpe-matching 🐳

Note, please use dependencytrack/frontend:snapshot when testing with frontend, as the API server image is built on top of 4.9.0-SNAPSHOT. You can use this Compose file if you don't have another one handy already.

[Jasper-Ben]

👋 @nscuro

Currently, the image doesn't yet work as intended. I uploaded a SBOM with following results:

Nearly all components report vulnerabilities CVE-2023-20583 and CVE-2022-24436, which are both CPU bugs.
Also a first check showed that some expected results are still missing:

cpe:2.3:*:*:nginx:1.20.1:*:*:*:*:*:*:* should match cpe:2.3:a:f5:nginx:1.20.1:*:*:*:*:*:*:* which in turn includes CVE-2021-3618. This vulnerability is not included in DT

[nscuro]

Thanks for the feedback @Jasper-Ben, I'll try to reproduce and include test cases for those. Can you share the SBOM you used for testing?

[Jasper-Ben]

Thanks for the feedback @Jasper-Ben, I'll try to reproduce and include test cases for those. Can you share the SBOM you used for testing?

Sure thing!
bom.txt (I know it is not perfect, e.g. ':' in component names, but that is a different story 😅)

Also I have following VEX file which resolves some of the CVEs I expect to find. Might be helpful for cross-referencing.
vex.txt

[nscuro]

At least in the test, the NGINX CPE is matching, both with and without version range ("up to (excluding)"):

https://github.com/nscuro/dependency-track/blob/2db9cd46d45f80d442a0362492f63952e59b514b/src/test/java/org/dependencytrack/tasks/scanners/InternalAnalysisTaskCpeMatchingTest.java#L225-L228

Can you confirm that CVE-2021-3618 is present in your DT instance? And if so, what do you see when clicking View Details -> Affected Components? The CPEs from the NVD should be listed there.

[nscuro]

For reference, this is what I'm seeing in a new instance:

And DT ends up finding this CVE, along with 1170 other vulnerabilities for this BOM:

I noticed however that the internal analysis took longer than usual, I am assuming this is happening due to the extensive use of ANY on the CPEs included in the BOM, which requires DT to fetch more data from the DB.

---

As for the CPU vulnerabilities:

• CVE-2023-20583 has the CPE cpe:2.3:h:amd:*:*:*:*:*:*:*:*:*
• CVE-2022-24436 has the CPE cpe:2.3:h:intel:*:*:*:*:*:*:*:*:*

One of the components it's matching against currently has the CPE cpe:2.3:*:*:xrandr:1.5.1:*:*:*:*:*:*:*.
So let's look at how this is getting matched:

Attribute	Source	Target	Result
Part	h	ANY	SUPERSET (#13)
Vendor	intel	ANY	SUPERSET (#13)
Product	ANY	xrandr	SUPERSET (#3)
Version	ANY	1.5.1	SUPERSET (#3)

The problem is that by using ANY for Part and Vendor in the target, it's bypassing the only attributes that reliably filter out false positives. Granted, specifying ANY for Product in the source is just asking for trouble and simply can't be correct on the NVD side.

It comes down to a combination of:

• Target CPE being too imprecise
• NVD data being unreliable sometimes

[Jasper-Ben]

At least in the test, the NGINX CPE is matching, both with and without version range ("up to (excluding)"):

https://github.com/nscuro/dependency-track/blob/2db9cd46d45f80d442a0362492f63952e59b514b/src/test/java/org/dependencytrack/tasks/scanners/InternalAnalysisTaskCpeMatchingTest.java#L225-L228

Can you confirm that CVE-2021-3618 is present in your DT instance? And if so, what do you see when clicking View Details -> Affected Components? The CPEs from the NVD should be listed there.

Now the CVE (and many more) are found in the project. Maybe the scan was not finished yet, or my manual triggering of a re-scan fixed it. 👍

[Jasper-Ben]

As for the CPU vulnerabilities:

* [CVE-2023-20583](https://github.com/advisories/GHSA-52f4-rvv8-64gj) has the CPE `cpe:2.3:h:amd:*:*:*:*:*:*:*:*:*`

* [CVE-2022-24436](https://github.com/advisories/GHSA-m7hv-7hvq-q3xx) has the CPE `cpe:2.3:h:intel:*:*:*:*:*:*:*:*:*`

One of the components it's matching against currently has the CPE cpe:2.3:*:*:xrandr:1.5.1:*:*:*:*:*:*:*. So let's look at how this is getting matched:
Attribute Source Target Result
Part h ANY SUPERSET (#13)
Vendor intel ANY SUPERSET (#13)
Product ANY xrandr SUPERSET (#3)
Version ANY 1.5.1 SUPERSET (#3)

The problem is that by using ANY for Part and Vendor in the target, it's bypassing the only attributes that reliably filter out false positives. Granted, specifying ANY for Product in the source is just asking for trouble and simply can't be correct on the NVD side.

It comes down to a combination of:

* Target CPE being too imprecise

* NVD data being unreliable sometimes

That makes sense. Thanks for the break-down. Then these are "valid" false positives due to imprecise data. That is then something that needs improvement on the component CPE generating side of things.

[Jasper-Ben]

@nscuro here is a vex to suppress the hardware CVEs for the provided SBOM. I will continue to check the remaining CVEs 🙂

vex_mod.txt

[Jasper-Ben]

I just cross-referenced ~15 components and their remaining CVEs against the NIST CPE search. Pleased to share that DT found ALL vulnerabilities that the CPE search returned. In addition, DT even found a handful CVEs that NIST search falsely did NOT include. Really, really happy with the result! 🥳

[nscuro]

Awesome, thanks for checking @Jasper-Ben!

DT even found a handful CVEs that NIST search falsely did NOT include

While that's great, I'd love to understand why, as we're not doing anything special (AFAICT). Can you share a few of those CVEs?

[Jasper-Ben]

While that's great, I'd love to understand why, as we're not doing anything special (AFAICT). Can you share a few of those CVEs?

NIST CPE search will only return results, if a CPE entry explicitly exists in their DB. E.g. searching for CPE cpe:2.3:*:*:libxslt:1.1.35:*:*:*:*:*:*:* will not return results on the NIST CPE search.

DT however will find the valid CVE CVE-2022-29824 for this version of libxslt.

I also noticed that there are some false positives for CVEs, probably due to bad CVE data. e.g. unzip 6.0 reports CVE-2008-0888 in DT.

[nscuro]

NIST CPE search will only return results, if a CPE entry explicitly exists in their DB.

Oh wow, that's a bummer. But good to know that DT behaves as expected here.

---

Thanks for all the input, and apologies that resolving this took so long. I'll merge #3070 which will also close this PR. I think we are in a way better position now when it comes to reproducing and fixing any upcoming issues in CPE matching.

[Jasper-Ben]

Thanks for all the input, and apologies that resolving this took so long.

No worries, thanks for getting it fixed!

I think we are in a way better position now when it comes to reproducing and fixing any upcoming issues in CPE matching.

Agreed! Do you have an ETA for the next release? Would rather not deploy a development image in production 😉

[nscuro]

Do you have an ETA for the next release?

Later today. I am in the process of finalizing the release.