CVE de-duplication in VEX causes loss of information

[Jasper-Ben]

Hi @loulou123546, I was looking at your commit 4dd0573 again and I noticed that this causes information loss:

1. Multiple components might be affected by the same CVE
2. Some of those components might still be vulnerable
3. Some of those components might have the CVE already fixed as part of a backport ("resolved")
4. Some of those components might have the CVE set to be ignored ("not_affected")

However, the analysis state is stored for each CVE entry, e.g.:

    {
      "id": "CVE-2019-0190",
      "source": {
        "name": "NVD",
        "url": "https://nvd.nist.gov/vuln/detail/CVE-2019-0190"
      },
      "analysis": {
        "state": "not_affected"
      },
      "affects": [
        {
          "ref": "urn:cdx:669105be-c6a0-4451-8a06-bc4e54244ba0/1#13944da4-7d07-4cc6-bfca-4da7076a7b06"
        }
      ]
    }

Thus, a de-duplication here will cause multiple components with different analysis states to be thrown together into a single CVE entry, thus losing the actual analysis state of the CVE within any package following the first occurrence.

The problem gets more obvious when looking at my PR in which I additionally provide analysis.detail, which can differ from package to package:

https://github.com/savoirfairelinux/meta-cyclonedx/pull/6/files#diff-a4ee7e0998b61bc3346a018617dce8ea248d8f0cefe23140ad613469ced57082R195-R197

Which makes sense: The reason to ignore CVE-X in package Y might differ from why it is ignored in package Z. Thus, it really only makes sense to have separate CVE entries per package.

---

I think what caused a lot of confusion on this (at least on my part) is an unfortunate formulation in the CycloneDX json schema.

Here it says for the description of the id field:

The identifier that uniquely identifies the vulnerability.

Which I understood to mean that the id entry needs to be unique. But if you actually look at the json schema definition, the id field is not set to unique (aka missing a "uniqueItems": true):

https://github.com/CycloneDX/cyclonedx-go/blob/6f53207eded10e9e2a362772b253621840e7ad94/schema/bom-1.4.schema.json#L1435-L1444

So what the description actually means is that from the provided id value the vulnerability must be uniquely identifiable (i.e. in the NIST DB), NOT that the entry needs to be unique.

---

I would therefore suggest that we revert 4dd0573.

[loulou123546]

Hi, I see, this seem legit. I will look at that this afternoon

[loulou123546]

resolved when merging #8

[groetingc]

Hi @Jasper-Ben !
I'm again struggling around with packages, which are listed in the bom.json file but not really installed in final image.
I know all about the arguments to leave them in the list as it is done so far.
However, I just would like to ask for an advice how to implement similar functionality as we see in cve-check.bbclass https://git.yoctoproject.org/poky/tree/meta/classes/cve-check.bbclass?h=kirkstone#n194 re-using the image_list_installed_packages(d) function to get same behavior than using yocto's cve scanning default behavior.
Any input would be very helpful, thanks!
BR,
Claus

[Jasper-Ben]

Hi @groetingc,

technically that is possible, but it would probably require some major refactoring of the current code.

The way meta-cyclonedx currently works is that is simply "extends" all .bb files with the content of the bbclass, writing the cyclonedx output directly to the deploy directory. That means that whenever any .bb file is used in the build process the cyclonedx tasks are run, regardless of whether the build output ends up in the image.

Unfortunately, the way that scoping works in Yocto, there is no way to know whether the output ends up in the image at the point in time when the tasks for an individual recipe are run. Only once the rootfs is put together you can actually figure it out using the image_list_installed_packages(d) function.

That's why the function in cve-check.bbclass that actually writes the output to the deploy directory (and uses the image_list_installed_packages(d) function is) called after the rootfs construction is complete:

ROOTFS_POSTPROCESS_COMMAND:prepend = "${@'cve_check_write_rootfs_manifest; ' if d.getVar('CVE_CHECK_CREATE_MANIFEST') == '1' else ''}"

So to achieve this, the code becomes a bit more complex.

Before:

1. .bb file is used in build process, directly append to file in the deploy directory.

After:

1. .bb file is used, create a temporary file containing the cyclonedx information snippet for this recipe (/build/tmp/work/<path_to_recipe_folder>/cyclonedx/...)
2. Write function that iterates over the output of image_list_installed_packages(d) and for each of those merges the snippets into a final sbom file and writes it into the deploy directory.
3. prepend said function to ROOTFS_POSTPROCESS_COMMAND so that it is called after the image has been built.

[Jasper-Ben]

IMO writing the cyclonedx output directly in the deploy directory while the process is still running is kind of bad practice anyways, as it will leave you with an incomplete sbom file in the deploy directly if the build is cancelled prematurely.

Writing the SBOM data for the individual recipes into temporary files and then just combining them together after the build is complete would be the cleaner solution, so I was already thinking about changing this. In that case we could probably put the limitation to installed packages behind a feature flag.

[Jasper-Ben]

@groetingc I created a seperate issue to keep track of this: #9

Feel free to give this a shot. If not, I will probably address this at some point but I cannot exactly say when.