-
Notifications
You must be signed in to change notification settings - Fork 17.8k
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
x/vuln: improve OpenVEX status output for fixed vulnerabilities #68338
Comments
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 I think we could start by differentiating the status of "fixed" from "not_affected." cc @golang/vulndb |
@golang/vulndb |
Interesting. I had never thought about including a fixed version. @puerco Is it possible to include fixed versions? |
Yes An algorithm would go like this:
When looking at the dependencies, if one has a CVE associated with it:
Note that the critical part is getting the dependency versions correct in the subcomponent (see the examples in #68152 (comment) )
Two notes there:
|
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
@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. |
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.
|
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. |
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? |
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. |
In our use case, we filter out "fixed" vulnerabilities as we're more interested in suppressing false positives, so it's totally fine. |
Change https://go.dev/cl/597396 mentions this issue: |
I think that should be the first approach then. We can add the "fixed" vulns if that is something users would really benefit from. |
This change modifies govulncheck's VEX output to no longer include vulnerabilities that are not imported at a vulnerable version. This matches the text output of govulncheck, and is in line with most other vulnerability scanners. updates golang/go#68338 Change-Id: If7041fd4624d023f623db8daf35a2e76f41d1d29 Reviewed-on: https://go-review.googlesource.com/c/vuln/ /597396 LUCI-TryBot-Result: Go LUCI <[email protected]> Reviewed-by: Mauri de Souza Meneguzzo <[email protected]> Reviewed-by: Zvonimir Pavlinovic <[email protected]>
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. |
govulncheck version
Go: go1.22.4
Scanner: [email protected]
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:What did you do?
Run
govulncheck -format openvex ./...
on my projectWhat 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: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!
The text was updated successfully, but these errors were encountered: