Similar Issues

• x/vuln: Missing output message when no vulnerabilities are found #61328
• x/vuln: govulncheck has broken output for only stdlib vulns #59323
• x/vuln: govulncheck reports no vulnerabilities while deps.dev reports two vulns #54970
• x/exp/vulndb/govulncheck: CVEs are not detected in GOPATH mode #51591
• x/vuln: outdated govulncheck documentation #61152
• x/vuln: use govulncheck in bazel #61494
• x/tools/gopls: broken build with x/vuln@latest #59837
• Gox/vuln: #60486

_{(Emoji vote if this was helpful or unhelpful; more detailed feedback welcome in this discussion.)}

A tentative fix/improvement for this is done in golang/vuln#11.

@maceonthompson

To summarize, the proposal here is to use (purls of) packages of detected vulnerabilities as product IDs. This would allow govulncheck openvex output to work with other tools, such as trivy and grype.

This proposal is in contrast with what we were thinking initially and that is to have the artifact under analysis as the product. That approach, however, is difficult when dealing with, say, different package patterns of a module. The proposal here does not have these issues. It also seems openvex spec allows what is proposed here.

Hey @maceonthompson 👋🏻

@zpavlinovic thanks for explanation above.

FYI we were having an initial discussion about a possible implementation for this in golang/vuln#11 (comment). The code is still available in my fork https://github.com/macedogm/vuln/commits/openvex-improvement/.

It's simple, but works. The biggest challenge that I saw is that govulncheck's OSV struct doesn't contain a way to track the exact affected version of the vulnerable package/path that was identified in the scanned code (see in https://github.com/golang/vuln/blob/d44b651a2e0f43966413e20b65cb619bdb621e7e/internal/vulncheck/emit.go#L21). Due to that, I added an Internal struct inside OSV to hold the needed info. Otherwise, the biggest benefit of the OpenVEX spec cannot be achieved, which is to identify the exact product (package and version) not affected, which can be removed by the downstream vulnerability scanners (Trivy, Grype etc.), although govulncheck knows about the exact version.

The product, in the OpenVEX spec, can have subcomponents. My initial implementation doesn't add this.

Where I work we are planning to use this as a way to remove hundreds of false positives flagged by vulnerability scanners. Initially we are still using my forked version.

Happy to collaborate further if needed.

Pushing things inside OSV struct is not the way to go IMO. There should be a different way of doing this, but that is an implementation detail. Your code is useful nonetheless. We'll use the issue here to decide whether to proceed with using vulnerable packages as product IDs.

Hi @macedogm,

Thank you for the work/time you've already spent on this issue!

If the idea is to use the source of the vulnerability (i.e. the identifier of the offending dependency) in the product field, I think govulncheck’s output already contains all the information we need.
OSVs contain module and package paths for a vulnerability, and findings always contain the module path and imported version of the offending vulnerable package/path. If govulncheck emits an OSV and no findings, that means that the vulnerable package was not imported at a vulnerable version.

I really like this proposal, but I think it goes against the VEX specification (see Section 2.5.1 and 2.5.2), which state that the Product ID should be the artifact that you’re scanning, and subcomponents are where vulnerable product IDs can be filled in. If we kept the initial idea but populated the subcomponent field instead, would this work/help your use case?

Hey @maceonthompson.

Thanks for the update.

I really like this proposal, but I think it goes against the VEX specification (see Section 2.5.1 and 2.5.2), which state that the Product ID should be the artifact that you’re scanning, and subcomponents are where vulnerable product IDs can be filled in.

You are totally right, in the implementation that I did I was listing directly the affected path/package and version. Example from my forked version:

    {    
      "vulnerability": {
        "@id": "https://pkg.go.dev/vuln/GO-2021-0061",
        "name": "GO-2021-0061",
        "description": "Denial of service in gopkg.in/yaml.v2",
        "aliases": [
          "CVE-2021-4235",
          "GHSA-r88r-gmrh-7j83"
        ]
      },   
      "products": [
        {
          "@id": "pkg:golang/gopkg.in/yaml.v2@2.4.0"
        }
      ],   
      "status": "not_affected",
      "justification": "vulnerable_code_not_present",
      "impact_statement": "Govulncheck determined that the vulnerable code isn't called"
    },

If we kept the initial idea but populated the subcomponent field instead, would this work/help your use case?

Yes, it would definitely work and be in full compliance with the spec 👍🏻
Out of curiosity, how do you plan to derive the artifact, for example, when the scan is done in source versus binary mode?

Thanks for working on this, it's much appreciated.

Hello from the @openvex project 👋

Thanks for opening this issue, I have been meaning to contribute a patch to fix this but I'm glad this discussion is already underway!

Let me add some context on the VEX product and subproduct.

@maceonthompson's reading of the spec is correct here:

the VEX specification (see Section 2.5.1 and 2.5.2), which state that the Product ID should be the artifact that you’re scanning

Let's imagine two scenarios, source and binary.

For example, when running in source mode, to craft a statement for the source of vexctl (at a random commit) the main vexctl module would act as the product and the subcomponent would point to the dependency containing the vulnerability. Here is an example fixed from the output of govulncheck:

    {
      "vulnerability": {
        "@id": "https://pkg.go.dev/vuln/GO-2024-2947",
        "name": "GO-2024-2947",
        "description": "Leak of sensitive information to log files in github.com/hashicorp/go-retryablehttp",
        "aliases": [
          "GHSA-3633-5h82-39pq"
        ]
      },
      "products": [
        {
          "@id": "pkg:golang/github.com/openvex/vexctl@c7362b73a06e2f8e4285b115d98e84f0ebfb3db8",
          "subcomponents":  [
              {
                  "@id": "pkg:golang/github.com/hashicorp/go-retryablehttp@v0.7.5"
              }
          ]
        }
      ],
      "status": "affected",
    }

Now when it comes to binaries, you don't need to specify a package URL (a purl). If you have a purl for one, its fine but the product identifier field in OpenVEX takes a URI so, in the case of binaries, the best way to specify them is to use a URI with a file schema but note that the real value comes from anchoring the statement to the file with its hashes. Our tooling will operate with hashes when available.

This is the same output but with a linux binary built from the same build point as above. Note that the path really becomes irrelevant once we can content-address the binary with a hash:

    {
      "vulnerability": {
        "@id": "https://pkg.go.dev/vuln/GO-2024-2947",
        "name": "GO-2024-2947",
        "description": "Leak of sensitive information to log files in github.com/hashicorp/go-retryablehttp",
        "aliases": [
          "GHSA-3633-5h82-39pq"
        ]
      },
      "products": [
        {
          "@id": "file:///vexctl-linux-amd64",
          "hashes": {
              "sha-256": "f8d37ec5f6ee5fb1352b0162443a9c8fcd50dab5125fde288daca0533a9286b7",
              "sha-512": "875079d0b61c6a03dcba46eef3a877d5be8789a068ad5f182a876ee08f754e1f2d7cbb5f22be73a77ff49d5565e13579c85e3f9f32884ec107e5e7624484f093"
              },
              "subcomponents":  [
                  {
                      "@id": "pkg:golang/github.com/hashicorp/go-retryablehttp@v0.7.5"
                  }
              ]
        }
      ],
      "status": "affected",
    }

The important part is that the vex statement operates on the triad of a vulnerability in a dependency evaluated in the context of the product (source or an artifact including the dependency).

I'm happy to contribute a patch to handle these :)

By looking at trivy code, it seems that not having a package identifier for the product will result in nothing being matched regardless of subproducts. I might be wrong as I am not familiar with the code base.

Yes, that's correct. If a product has subcomponents, Trivy applies VEX to the dependency graph (the same idea is described in the OSV blog). Incorrect product ID results in nothing filtered as it doesn't match the graph.
https://aquasecurity.github.io/trivy/v0.53/docs/supply-chain/vex/#applying-vex-to-dependency-trees

If a product has no subcomponents, it works as a simple filter in Trivy, but in that case it may incorrectly filter vulnerabilities in another product.

{
  "vulnerability": {
    "@id": "https://pkg.go.dev/vuln/GO-2021-0064",
    "name": "GO-2021-0064",
    "description": "Unauthorized credential disclosure via debug logs in k8s.io/kubernetes and k8s.io/client-go",
  },
  "products": [
    {
      "@id": "pkg:golang/k8s.io/client-go@0.20.0-alpha.2"
    }
  ],
  "status": "not_affected",
  "justification": "vulnerable_code_not_present",
  "impact_statement": "Govulncheck determined that the vulnerable code isn't called"
}

For example, my project is not affected by GO-2021-0064 in k8s.io/client-go, but other projects may be affected. Ideally, the correct product and subcomponents should be filled in.

External tooling may be able to fill in the missing product but it's not ideal.

That's exactly what we're doing now.

Change https://go.dev/cl/598956 mentions this issue: internal/openvex: populate product subcomponents

A follow up question on subcomponent IDs:
Right now we are planning on having the subcomponent ID be just the module path (since that seems to be most helpful based on the examples above), but govulncheck has more specific package and symbol information that we could theoretically include as well.
Would it be helpful to encode that extra information at the end of the PURL?

I think the import path and version should be enough. What else could you encode in the purl? The more specific the purl is the harder it is to match but perhaps I'm missing another use case that may make more sense.

I agree that the matching becomes more difficult, which is why we're currently basing it just off the module path. We could, however, include the package path as well - which is technically part of the import path. But if the consensus is that the extra info isn't useful for this use case, I'm happy to leave it as is.

FWIW, this is also what the golang/vuln#11 proposed fix does. There is also a consistency issue. If someone runs govulncheck in -scan module mode, then package info will be missing.

My first answer is to agree with:

I think the import path and version should be enough.

@maceonthompson could you please provide some examples of how the subcomponent ID would be regarding what you mentioned?

... include the package path as well - which is technically part of the import path.

Asking, so we have detailed info to make a call and how different the information would be for each one of the scan mode - 'module', 'package', or 'symbol'.

@maceonthompson could you please provide some examples of how the subcomponent ID would be regarding what you mentioned?

@macedogm An example: Say there is a vulnerability in the module golang.org/x/vuln at version v1.0.0. More specifically, it is present in the package golang.org/x/vuln/scan and has the vulnerable symbol foo. Given each scan mode, this is what the ID could look like

This matches how pkgsite presents packages vs modules.
Otherwise, every scan mode would match "module" mode, not including the package path.

I was more interested in if that would be useful - it seems that the consensus is that easier matching is more important, so I'm happy to just include the module path.

@maceonthompson thanks for providing the examples.

I was more interested in if that would be useful - it seems that the consensus is that easier matching is more important, so I'm happy to just include the module path.

It's useful, but I fully agree that easier matching is more important, specially because it allows removing false positives at scale. 👍🏻