search

sonatype-nexus-community / jake

Check your Python environments for vulnerable Open Source packages with OSS Index or Sonatype Nexus Lifecycle.
https://jake.readthedocs.io/
Apache License 2.0
114 stars 24 forks source link

[BUG] Invalid references for NVD CVE identifiers in CycloneDX JSON 1.4 format #93

Closed damiencarol closed 2 years ago

damiencarol commented 2 years ago

Describe the bug Version <= 1.4.1 produce wrong references in the CycloneDX JSON report.

To Reproduce Steps to reproduce the behavior:

  1. Run jake ddt -o ~/dd2/unittests/scans/cyclonedx/jake2.json --output-format json --schema-version 1.4 --clear-cache
  2. check the report data

Expected behavior

According to the specification, references like CVE identifier should be like this:

      "references": [
        {
          "id": "CVE-2018-7489",
          "source": {
            "name": "NVD",
            "url": "https://nvd.nist.gov/vuln/detail/CVE-2019-9997"
          }
        }
      ],

But current format is like this which is not compliant with the schema of the spec:

            "references": [
                {
                    "source": {
                        "url": "https://nvd.nist.gov/vuln/detail/CVE-2021-33203"
                    }
                }
            ],

A lot of data are missing. I'm interested in the source > name and the id which critical to know it's CVE by program.

Desktop (please complete the following information):

Additional context I'm working with reports of Jake/CycloneDX JSON 1.4 format

damiencarol commented 2 years ago

FYI @madpah

madpah commented 2 years ago

Thanks @damiencarol - will look at getting a fix for this.

madpah commented 2 years ago

@damiencarol - so this is a little bit of a challenge to implement correctly when data is being sourced from OSS Index.

Let's take an example - pkg:pypi/pyjwt@1.3.0 - the data returned by OSS Index today is:

[
  {
    "coordinates": "pkg:pypi/pyjwt@1.3.0",
    "reference": "https://ossindex.sonatype.org/component/pkg:pypi/pyjwt@1.3.0?utm_source=mozilla&utm_medium=integration&utm_content=5.0",
    "vulnerabilities": [
      {
        "id": "4dc8bf86-e2ee-45b0-881f-bb4f03748b5b",
        "displayName": "CVE-2017-11424",
        "title": "[CVE-2017-11424]  Improper Access Control",
        "description": "In PyJWT 1.5.0 and below the `invalid_strings` check in `HMACAlgorithm.prepare_key` does not account for all PEM encoded public keys. Specifically, the PKCS1 PEM encoded format would be allowed because it is prefaced with the string `-----BEGIN RSA PUBLIC KEY-----` which is not accounted for. This enables symmetric/asymmetric key confusion attacks against users using the PKCS1 PEM encoded public keys, which would allow an attacker to craft JWTs from scratch.",
        "cvssScore": 7.5,
        "cvssVector": "CVSS:3.0/AV:N/AC:L/PR:N/UI:N/S:U/C:N/I:H/A:N",
        "cve": "CVE-2017-11424",
        "reference": "https://ossindex.sonatype.org/vulnerability/4dc8bf86-e2ee-45b0-881f-bb4f03748b5b?component-type=pypi&component-name=pyjwt&utm_source=mozilla&utm_medium=integration&utm_content=5.0",
        "externalReferences": [
          "https://github.com/jpadilla/pyjwt/pull/277",
          "https://nvd.nist.gov/vuln/detail/CVE-2017-11424"
        ]
      }
    ]
  }
]

This tells us of 1 known Vulnerability - CVE-2017-11424 which has two external references:

Note - we have no additional information about the external references.

From a read of the specification, the id within each reference for a vulnerability is supposed to contain the reference for this same vulnerability in other bug tracking systems.

I suspect the correct course of action here will be to stop mapping OSS Index externalReferences to a Vulnerabilities references.

What are your thoughts?

damiencarol commented 2 years ago

@madpah 3 things:

  1. thanks a lot for the well constructed feedback, it's very useful for me to understand data flow from Jake
  2. I have the feeling that reference data should be mapped to the references/reference attribute of CycloneDX and externalReferences is more the attribute advisories of CycloneDX. But ...

  3. This means that we will lose the very useful CVE data in references in the same form as in the specification, like :

    "references": [
    {
      "id": "CVE-2018-7489",
      "source": {
        "name": "NVD",
        "url": "https://nvd.nist.gov/vuln/detail/CVE-2019-9997"
      }
    }
  ],

    So my advice would be to just:

    1. switch externalReferences to advisories
    2. switch reference (with OSS URL) to be a reference in references CycloneDX report
    3. if we have a CVE, add another references\reference in the CycloneDX with this format: 
      "references": [
{
  "id": "CVE-2018-7489",
  "source": {
    "name": "NVD",
    "url": "https://nvd.nist.gov/vuln/detail/CVE-2019-9997"
  }
}
],

Again thanks a lot for the insights/feedback

madpah commented 2 years ago

@damiencarol - so, I believe what we are saying (double confirming here!):

  1. OSS Index reference --> CycloneDX references

    "cve": "CVE-2017-11424",
"reference": "https://ossindex.sonatype.org/vulnerability/4dc8bf86-e2ee-45b0-881f-bb4f03748b5b?component-type=pypi&component-name=pyjwt&utm_source=mozilla&utm_medium=integration&utm_content=5.0",

    becomes

    "references": [
    {
      "id": "CVE-2017-11424",
      "source": {
        "url": "https://ossindex.sonatype.org/vulnerability/4dc8bf86-e2ee-45b0-881f-bb4f03748b5b?component-type=pypi&component-name=pyjwt&utm_source=mozilla&utm_medium=integration&utm_content=5.0"
      }
    }
]

  2. OSS Index externalReferences --> CycloneDX advisories

    "externalReferences": [
      "https://github.com/jpadilla/pyjwt/pull/277",
      "https://nvd.nist.gov/vuln/detail/CVE-2017-11424"
]

    becomes:

    "advisories": [
{
  "url": "https://github.com/jpadilla/pyjwt/pull/277"
},
{
  "url": "https://nvd.nist.gov/vuln/detail/CVE-2017-11424"
}
]

With regards to your point 3, the logic is fine, but:

  1. There is no guarantee that a URL will exist in externalReferences - this may be empty
  2. If externalReferences is not empty - which URL to use?
  3. This would be a duplicate of data already covered on the main Vulnerability:
    • cve is present in id for the Vulnerability

Let me know if I've miss-understood your point 3. Thanks for getting involved!

damiencarol commented 2 years ago

@madpah Nah it's good. what you described is good. If we can have these modifications I can build a smart "detection" that detect CVE in the Id of the reference. Go for it, I will do more tests with this.

madpah commented 2 years ago

Thanks for coming back so quickly @damiencarol - glad we agree :-)

madpah commented 2 years ago

Although - doing point 1 will duplicate data further - so is it even needed @damiencarol ?

See this example:


"vulnerabilities": [
    {
      "bom-ref": "4dc8bf86-e2ee-45b0-881f-bb4f03748b5b",
      "id": "CVE-2017-11424",
      "source": {
        "name": "Oss Index",
        "url": "https://ossindex.sonatype.org/vulnerability/4dc8bf86-e2ee-45b0-881f-bb4f03748b5b?component-type=pypi&component-name=pyjwt&utm_source=python-oss-index-lib%400.2.1&utm_medium=integration"
      },
      "references": [
        {
          "id": "CVE-2017-11424",
          "source": {
            "name": "Oss Index",
            "url": "https://ossindex.sonatype.org/vulnerability/4dc8bf86-e2ee-45b0-881f-bb4f03748b5b?component-type=pypi&component-name=pyjwt&utm_source=python-oss-index-lib%400.2.1&utm_medium=integration"
          }
        }
      ],
...

The data now held in references[0] is a repeat of id and source. Can you not utilise these fields?

damiencarol commented 2 years ago

@madpah ok, I see the pb now. I think that you're using the CVE as an Id which is not correct IMO your data should be better and more aligned with specification if you produce this (IMVHO Im My Very Humble Opinion): Raw data:

[
  {
    "coordinates": "pkg:pypi/pyjwt@1.3.0",
    "reference": "https://ossindex.sonatype.org/component/pkg:pypi/pyjwt@1.3.0?utm_source=mozilla&utm_medium=integration&utm_content=5.0",
    "vulnerabilities": [
      {
        "id": "4dc8bf86-e2ee-45b0-881f-bb4f03748b5b",
        "displayName": "CVE-2017-11424",
        "title": "[CVE-2017-11424]  Improper Access Control",
        "description": "In PyJWT 1.5.0 and below the `invalid_strings` check in `HMACAlgorithm.prepare_key` does not account for all PEM encoded public keys. Specifically, the PKCS1 PEM encoded format would be allowed because it is prefaced with the string `-----BEGIN RSA PUBLIC KEY-----` which is not accounted for. This enables symmetric/asymmetric key confusion attacks against users using the PKCS1 PEM encoded public keys, which would allow an attacker to craft JWTs from scratch.",
        "cvssScore": 7.5,
        "cvssVector": "CVSS:3.0/AV:N/AC:L/PR:N/UI:N/S:U/C:N/I:H/A:N",
        "cve": "CVE-2017-11424",
        "reference": "https://ossindex.sonatype.org/vulnerability/4dc8bf86-e2ee-45b0-881f-bb4f03748b5b?component-type=pypi&component-name=pyjwt&utm_source=mozilla&utm_medium=integration&utm_content=5.0",
        "externalReferences": [
          "https://github.com/jpadilla/pyjwt/pull/277",
          "https://nvd.nist.gov/vuln/detail/CVE-2017-11424"
        ]
      }
    ]
  }
]

CycloneDX

...
  "vulnerabilities": [
    {
      "bom-ref": "...",
      "id": "4dc8bf86-e2ee-45b0-881f-bb4f03748b5b",
      "source": {
        "name": "OSS index",
        "url": "https://ossindex.sonatype.org/vulnerability/4dc8bf86-e2ee-45b0-881f-bb4f03748b5b?component-type=pypi&component-name=pyjwt"
      },
      "references": [
        {
          "id": "CVE-2017-11424",
          "source": {
            "name": "NVD",
            "url": "https://nvd.nist.gov/vuln/detail/CVE-2017-11424"
          }
        }
      ],
...

See the Id now is the ID of OSS index. We can even use it in DefectDojo for internal process as it would be to use directly the id of the vulnerability. For the CVE we have another system dedicated to it.

I hope it's clear.

madpah commented 2 years ago

Makes sense @damiencarol - I'll also feedback to CycloneDX folks about the documentation (https://cyclonedx.org/docs/1.4/json/#vulnerabilities_items_id) as this isn't as clear as it could be IMVHO :-)

If you wouldn't mind checking the example produced with these changes on #94, that would be super and we can get this merged and released.

Thanks again for your input @damiencarol.

damiencarol commented 2 years ago

testing this branch right now.

damiencarol commented 2 years ago

@madpah made some tests and the branch is good! You also implemented the advisories. :+1: 

...
"advisories": [
                {
                    "url": "https://www.djangoproject.com/weblog/2018/feb/01/security-releases/"
                },
                {
                    "url": "https://nvd.nist.gov/vuln/detail/CVE-2018-6188"
                }
            ],
...

jake2.json

damiencarol commented 2 years ago

Thanks for the quick fix @madpah ! 👍