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

Allow for vulnerability to be a list #12

Closed
garethr opened this issue Feb 4, 2023 · 7 comments
Closed

Allow for vulnerability to be a list #12

garethr opened this issue Feb 4, 2023 · 7 comments
Labels
area/vulnerability Issues and PRs related to the vulnerability field

Comments

@garethr
Copy link

garethr commented Feb 4, 2023

The spec doesn't require a single identifying scheme (which is good) but this raises a potential usability concern, as vulnerability is singular.

Take openSUSE-SU-2020:0051-1. Let's say I patch my way out of it and produce a

{
      "vulnerability": "openSUSE-SU-2020:0051-1",
      "products": [...],
      "status": "not_affected",
      "justification": "component_not_present",
      "impact_statement": "The vulnerable code was removed with a custom patch"
}

This vulnerability can be referenced in lots of other ways, including:

As per the current spec, you'd need multiple statements, each duplicating products, status, justification and impact_statement.

{
      "vulnerability": "openSUSE-SU-2020:0051-1",
      "products": [...],
      "status": "not_affected",
      "justification": "component_not_present",
      "impact_statement": "The vulnerable code was removed with a custom patch"
},
{
      "vulnerability": "GHSA-2qrg-x229-3v8q",
      "products": [...],
      "status": "not_affected",
      "justification": "vulnerable_code_not_present",
      "impact_statement": "The vulnerable code was removed with a custom patch"
}
...

Or rely on the consumer to reconcile. That reconciliation is relatively easy in theory, at least where a CVE already exists, but in practice it's still a pain given the breadth of the ecosystem.

Another use case might be where multiple vulnerabilities relate to a single package, and that package isn't present. eg. component_not_present. Again, in that case multiple near-identical statements could be made, but it may be useful to permit it as one statement as well.

Given the above, I'd like to discuss whether vulnerability should instead by vulnerabilities and allow for a list of vulnerability identifiers?

@luhring
Copy link
Contributor

luhring commented Feb 5, 2023

I agree with the motivation here. 👍 I brought this up in the CISA meetings, but at the time, the group consensus was to keep statements to single vulnerability identifiers.

I'm curious for others' thoughts on this!

cc: @puerco

@puerco
Copy link
Member

puerco commented Feb 6, 2023

Hey @garethr thanks for the suggestion, this is something we had antcipated in the past, so let me give a little bit of context:

  1. I don't think we should have a list of vulnerabilities. The VEX community voted on the structure of the statement and decided to not do that.
  2. That said, we used to have a references field exactly for this (you can see it here in the old repo). We removed it to simplify the format at launch, always knowing we could bring it back if we needed it, so this may be it.

@tschmidtb51
Copy link

tschmidtb51 commented Feb 9, 2023

I agree with the motivation here. +1 I brought this up in the CISA meetings, but at the time, the group consensus was to keep statements to single vulnerability identifiers.

I agree that in the CISA meetings it was decided that you can have only one vulnerability per VEX statement. BUT that doesn't mean that a vulnerability can't have multiple identifiers. Now you just have to make sure, nobody abuses the "multiple ids" to sneak in multiple vulnerabilities...

@garethr
Copy link
Author

garethr commented Feb 10, 2023

I agree that in the CISA meetings it was decided that you can have only one vulnerability per VEX statement. BUT that doesn't mean that a vulnerability can't have multiple identifiers.

(Emphasis added).

Agreed. This is the challenge. Agree on no to multiple vulnerabilities, but it's one vulnerability with multiple identifiers that is the issue.

@puerco that would at least allow for support of this. From a client point of view though, which one you choose as the vulnerability and which are added as references is arbitrary. So in effect it's really just a flat list. I'm be +1 on bringing that back. It's a small addition to the spec that could reduce the verbosity of resulting documents.

It does make the use case clearer from a creator perspective though, which could help to address the "Now you just have to make sure, nobody abuses the "multiple ids" to sneak in multiple vulnerabilities..." issue raised by @tschmidtb51.

@puerco
Copy link
Member

puerco commented Jun 14, 2023

Mmmh I agree, we should allow for the other references.

Keeping with the spirit of linked data, perhaps we should just have the references as a list and make the identifier an IRI. For example:

{
      "vulnerability": {
           "@id":"https://nvd.nist.gov/vuln/detail/CVE-2019-17571",
            "references": [
                "CVE-2019-17571",
                "GHSA-2qrg-x229-3v8q",
                "openSUSE-SU-2020:0051-1",
                "SNYK-RHEL7-LOG4J-1472071",
                "DSA-4686-1",
                "USN-4495",
                "DLA-2065-1",
            ]
      },
      "products": ["pkg:..."],
      "status": "not_affected",
      "justification": "component_not_present",
      "impact_statement": "The vulnerable code was removed with a custom patch"
},

This would allow for a more compact document when the IRI can be resolved to a resource that has all the data about the vulnerability, such as an "SBOM" (a security document in SPDX or CDX) or an advisory:

{
      "vulnerability": { "@id":"https://nvd.nist.gov/vuln/detail/CVE-2019-17571" }
      "products": ["pkg:..."],
      "status": "not_affected",
      "justification": "component_not_present",
      "impact_statement": "The vulnerable code was removed with a custom patch"
},

@puerco puerco added the area/vulnerability Issues and PRs related to the vulnerability field label Jun 14, 2023
@luhring
Copy link
Contributor

luhring commented Jun 14, 2023

I like this direction. It may also be worth checking out how the OSV schema thinks of "IDs" and "aliases" of vulnerabilities — their way of capturing multiple references to the same underlying vulnerability. https://ossf.github.io/osv-schema/#aliases-field

@puerco
Copy link
Member

puerco commented Aug 22, 2023

Enhancement proposal OPEV-0015 was merged and with the release of spec v0.2.0, the vulnerability field can now support multiple vulnerability aliases 🎉

@puerco puerco closed this as completed Aug 22, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/vulnerability Issues and PRs related to the vulnerability field
Projects
None yet
Development

No branches or pull requests

4 participants