x/vuln: improve OpenVEX status output for fixed vulnerabilities

[knqyf263]

govulncheck version

Go: go1.22.4
Scanner: govulncheck@v1.1.2
DB: https://vuln.go.dev
DB updated: 2024-07-03 16:27:09 +0000 UTC

Does this issue reproduce at the latest version of golang.org/x/vuln?

Yes

Output of go env in your module/workspace:

GO111MODULE=''
GOARCH='arm64'
GOBIN=''
GOCACHE='/Users/teppei/Library/Caches/go-build'
GOENV='/Users/teppei/Library/Application Support/go/env'
GOEXE=''
GOEXPERIMENT=''
GOFLAGS=''
GOHOSTARCH='arm64'
GOHOSTOS='darwin'
GOINSECURE=''
GOMODCACHE='/Users/teppei/pkg/mod'
GONOPROXY=''
GONOSUMDB=''
GOOS='darwin'
GOPATH='/Users/teppei'
GOPRIVATE=''
GOPROXY='https://proxy.golang.org,direct'
GOROOT='/opt/homebrew/Cellar/go/1.22.4/libexec'
GOSUMDB='sum.golang.org'
GOTMPDIR=''
GOTOOLCHAIN='auto'
GOTOOLDIR='/opt/homebrew/Cellar/go/1.22.4/libexec/pkg/tool/darwin_arm64'
GOVCS=''
GOVERSION='go1.22.4'
GCCGO='gccgo'
AR='ar'
CC='cc'
CXX='c++'
CGO_ENABLED='1'
GOMOD='/Users/teppei/src/github.com/aquasecurity/trivy/go.mod'
GOWORK=''
CGO_CFLAGS='-O2 -g'
CGO_CPPFLAGS=''
CGO_CXXFLAGS='-O2 -g'
CGO_FFLAGS='-O2 -g'
CGO_LDFLAGS='-O2 -g'
PKG_CONFIG='pkg-config'
GOGCCFLAGS='-fPIC -arch arm64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -ffile-prefix-map=/var/folders/9_/3y18vrrs5bz6gbkq0kc_4fv80000gn/T/go-build2940602674=/tmp/go-build -gno-record-gcc-switches -fno-common'

What did you do?

Run govulncheck -format openvex ./... on my project

What did you see happen?

When scanning projects with govulncheck, all non-impacting vulnerabilities are reported with the status "not_affected" in the generated OpenVEX statements, regardless of whether the project is using a version that has already fixed the vulnerability.

For example, when scanning a project using k8s.io/client-go v0.29.0, govulncheck outputs the following for vulnerability GO-2021-0064:

{
  "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",
    "aliases": [
      "CVE-2020-8565",
      "GHSA-8cfg-vx93-jvxw"
    ]
  },
  "products": [
    {
      "@id": "Unknown Product"
    }
  ],
  "status": "not_affected",
  "justification": "vulnerable_code_not_present",
  "impact_statement": "Govulncheck determined that the vulnerable code isn't called"
}

What did you expect to see?

I expected govulncheck to differentiate between vulnerabilities that are not present and those that have been fixed in the version being used. In the case of GO-2021-0064, I believe the status should be reported as "fixed" for version 0.29.0 since this vulnerability was addressed in k8s.io/client-go v0.20.0-alpha.2.

Currently, govulncheck seems to only output 'affected' or 'not_affected' statuses when generating OpenVEX. From my understanding of the OpenVEX specification, it could be more precise by using three statuses: 'affected', 'not_affected', and 'fixed'. I think it should use the 'fixed' status when the project is using a version that has already addressed the vulnerability.

In fact, the "fixed" vulnerabilities are not shown with govulncheck -show verbose ./.... They appear to be internally distinct. Please correct me if I'm missing something.

Thanks for the great work!

[mauri870]

Introducing a new status sounds reasonable. I reviewed the OpenVEX specification but couldn't find the appropriate field to inform about the fixed version. From my understanding, we should have a follow-up statement with the status set to "fixed" and products.@id containing the fixed version. Currently, this key is not used and always contains Unknown Product. I believe this requires further consideration to properly align the implementation with the specification.

I think we could start by differentiating the status of "fixed" from "not_affected."

cc @golang/vulndb

[cagedmantis]

@golang/vulndb

[knqyf263]

but couldn't find the appropriate field to inform about the fixed version.

Interesting. I had never thought about including a fixed version.

@puerco Is it possible to include fixed versions?

[puerco]

Yes fixed is one of the defined vex status labels.

An algorithm would go like this:

govulncheck looks at a binary or source code (any of those go into the vex product, see #68152 (comment) ).

When looking at the dependencies, if one has a CVE associated with it:

• If the dependency is vulnerable and the vuln can be triggered the status should be affected
• if the dependency is vulnerable but the bad function is never called, the status label would be not_affected
• If it is using a newer version of the dependency, already patched and not vulnerable, the status would be: fixed

Note that the critical part is getting the dependency versions correct in the subcomponent (see the examples in #68152 (comment) )

@mauri870 :

we should have a follow-up statement with the status set to "fixed" and products.@id containing the fixed version.

Two notes there:

1. First the fixed version should go into the subcomponent @ id of the product.
2. While VEX is designed to be a chronological series of statements, in this case I think you can't issue the sequenced statements, just a fixed statement and that's it. The VEX product (the source or a binary) is fixed at a point in time. In other words, the go.mod in the analyzed commit (when the product is the main module source) or the built binary either include the fixed dependency or don't. They would never change (as changing it would result in another commit or different binary). This means that a transition from affected to fixed never occurs when looking at a specific commit or built binary.

[knqyf263]

First the fixed version should go into the subcomponent @ id of the product.

That is the fixed version currently used in the product (a.k.a installed version) and different from the version that initially fixed the vulnerability, right? In the above example, the subcomponent version is v0.29.0, which already fixed GO-2021-0064, but the first fixed version of GO-2021-0064 is v0.20.0-alpha.2. The subcomponent id should be pkg:golang/k8s.io/client-go@v0.29.0, not pkg:golang/k8s.io/client-go@v0.20.0-alpha.2.

I reviewed the OpenVEX specification but couldn't find the appropriate field to inform about the fixed version.

@mauri870 Which version are you talking about? I thought you meant a fixed version, not an installed version, which already fixed the vulnerability. I may be wrong.

[mauri870]

@mauri870 Which version are you talking about? I thought you meant a fixed version, not an installed version, which already fixed the vulnerability. I may be wrong.

I was referring to the version that fixed the vuln, not the installed version. I thought we'll have to use a chronological series of statements but it seems that a single statement with a status "fixed" is the the way to do it.

Thanks @puerco for the detailed summary! Looks like handling #68152 (comment) would be part of the implementation needed to support the "fixed" status.

[knqyf263]

I was referring to the version that fixed the vuln, not the installed version.

Thanks for confirming. If I understand correctly, there is no place to fill in the fixed version, but you can put the installed version into the subcomponent id with the "fixed" status as below.

{
  "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",
    "aliases": [
      "CVE-2020-8565",
      "GHSA-8cfg-vx93-jvxw"
    ]
  },
  "products": [
    {
      "@id": "pkg:golang/github.com/org/repo@1.2.3",
      "subcomponents": [
      {
          "@id": "pkg:golang/k8s.io/client-go@0.29.0",
        }
      ]
    }
  ],
  "status": "fixed",
}

[macedogm]

When scanning projects with govulncheck, all non-impacting vulnerabilities are reported with the status "not_affected" in the generated OpenVEX statements, regardless of whether the project is using a version that has already fixed the vulnerability.

In case it helps, on my local fork of govulncheck that I did the initial work on #68152, I'm using the following modification to filter only the applicable vulnerabilities based on the used versions:

diff --git a/internal/vulncheck/fetch.go b/internal/vulncheck/fetch.go
index 700a7f9..0dcff98 100644
--- a/internal/vulncheck/fetch.go
+++ b/internal/vulncheck/fetch.go
@@ -17,11 +17,14 @@ func FetchVulnerabilities(ctx context.Context, c *client.Client, modules []*pack
        mreqs := make([]*client.ModuleRequest, len(modules))
        for i, mod := range modules {
                modPath := mod.Path
+               modVersion := mod.Version
                if mod.Replace != nil {
                        modPath = mod.Replace.Path
+                       modVersion = mod.Replace.Version
                }
                mreqs[i] = &client.ModuleRequest{
-                       Path: modPath,
+                       Path:    modPath,
+                       Version: modVersion,
                }
        }
        resps, err := c.ByModules(ctx, mreqs)

That helps reduce the noise.

FYI: In SUSE Rancher we are doing an initial work with govulncheck to reduce the noise in CVEs in the images that we publish.

[gabyhelp]

Related Issues and Documentation

• x/vuln: OpenVEX report lacks affected product #68152
• x/vuln: Missing output message when no vulnerabilities are found #61328 (closed)
• x/vuln: govulncheck reports no vulnerabilities while deps.dev reports two vulns #54970 (closed)
• x/vuln: add flag to provide actionable govulncheck fixes #56569 (closed)
• x/vuln: incorrect version is suggested as a fixed version for a vulnerability with multiple packages #54913 (closed)
• x/vuln: json output always exits 0 #61704 (closed)
• x/vuln: reports could exclude calls with unaffected arguments #54889

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

[zpavlinovic]

@maceonthompson

I think govulncheck should not report these vulnerabilities at all. That would be the simplest solution; it wouldn't complicate things internally for us. How satisfactory would that be?

[maceonthompson]

I'm happy to just drop those vulnerabilities. Right now, govulncheck emits those vulnerabilities in the JSON with no findings (it just emits the OSV), but we could definitely omit vulns with only an osv and no findings from the VEX output.

[knqyf263]

I think govulncheck should not report these vulnerabilities at all.

In our use case, we filter out "fixed" vulnerabilities as we're more interested in suppressing false positives, so it's totally fine.

[gopherbot]

Change https://go.dev/cl/597396 mentions this issue: internal/openvex: omit vulns with no findings

[zpavlinovic]

I think that should be the first approach then. We can add the "fixed" vulns if that is something users would really benefit from.

[maceonthompson]

Closing this, as the vex output now matches the text and SARIF outputs (in that it omits vulns that are not imported at a vulnerable version). We can re-open the issue if we end up changing this behavior.