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

x/vuln/cmd/govulncheck: communicate current Go version to user when analyzing source #56097

Closed
zpavlinovic opened this issue Oct 7, 2022 · 3 comments
Assignees
Labels
FrozenDueToAge UX Issues that involve UXD/UXR input vulncheck or vulndb Issues for the x/vuln or x/vulndb repo
Milestone

Comments

@zpavlinovic
Copy link
Contributor

zpavlinovic commented Oct 7, 2022

When analyzing source code, results of govulncheck should be interpreted w.r.t. to the current Go version. For instance, if the user is having go1.18 at PATH, we will report reachable standard library vulnerabilities for that version only. Users need to be aware of this.

Although documentation mentions this, perhaps we can do better. Ideally, we should further help avoiding the scenario where users run govulncheck only for, say, a single Go version where a standard library vulnerability is not exercised and then have an expectation that their programs are safe from std vulnerabilities for all Go versions.

Approach 1

One approach is to make explicit to the user in the govulncheck output for which Go/std version govulncheck analysis was conducted: CL.

Approach 2

An alternative approach is to pretend that the user is having all Go versions at PATH simultaneously and then just list at which Go versions are the reachable std vulnerabilities exercised. This is similar to #54841. There are few issues with this approach:

  1. The std code we load is still the std code for the Go version at PATH. In theory, if the current version is, say, go1.18 but a vulnerable symbol is introduced in, say, go1.19, we would miss reporting that symbol. This might not be a problem in practice because if the code for go1.18 compiles, then it clearly cannot use the symbol introduced later. However, it could be importing it though. I could also be missing something obvious.

  2. We should not bake this logic into vulncheck library, which means we need to come up with a module version that vulncheck should interpret as "all" and document it. We would also need to change a lot of tests, but that is less of a problem.

@jba @julieqiu @hyangah @fflewddur

@zpavlinovic zpavlinovic added UX Issues that involve UXD/UXR input vulncheck or vulndb Issues for the x/vuln or x/vulndb repo labels Oct 7, 2022
@zpavlinovic zpavlinovic added this to the vuln/2022 milestone Oct 7, 2022
@zpavlinovic zpavlinovic self-assigned this Oct 7, 2022
@timothy-king
Copy link
Contributor

Can we be clearer about what the "user journey" we are worried about is?

Here is my understanding a local dev of package P is at go1.19. The CI building system is at go1.18.0. There is a hypothetical vulnerable std library function Vuln which is vulnerable in go.18.0. P reaches Vuln. This vuln was fixed by 1.19. The user checks locally with govulncheck and P has no vulns against 1.19. A binary built by CI using 1.18.0 containing the vuln is then deployed.

Is this the concern? Or is there a motivating case I am missing?

@zpavlinovic
Copy link
Contributor Author

Yes, that would be the concern.

@gopherbot
Copy link
Contributor

Change https://go.dev/cl/463106 mentions this issue: cmd/govulncheck: add environment message

softdev050 added a commit to softdev050/Golangvuln that referenced this issue Apr 5, 2023
We define environment as Go version at PATH (showed only for source
mode), govulncheck version (effectively x/vuln version), and list of
vulnerability databases with their timestamp.

The CL also moves some code around.

Fixes golang/go#56097
Fixes golang/go#56514
Updates golang/go#56207

Change-Id: I2e2f179a5421b3dfc1e1f1e4bd0ed13d16735364
Reviewed-on: https://go-review.googlesource.com/c/vuln/+/463106
Reviewed-by: Jonathan Amsterdam <jba@google.com>
Run-TryBot: Zvonimir Pavlinovic <zpavlinovic@google.com>
Reviewed-by: Julie Qiu <julieqiu@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
sayjun0505 added a commit to sayjun0505/Golangvuln that referenced this issue Apr 8, 2023
We define environment as Go version at PATH (showed only for source
mode), govulncheck version (effectively x/vuln version), and list of
vulnerability databases with their timestamp.

The CL also moves some code around.

Fixes golang/go#56097
Fixes golang/go#56514
Updates golang/go#56207

Change-Id: I2e2f179a5421b3dfc1e1f1e4bd0ed13d16735364
Reviewed-on: https://go-review.googlesource.com/c/vuln/+/463106
Reviewed-by: Jonathan Amsterdam <jba@google.com>
Run-TryBot: Zvonimir Pavlinovic <zpavlinovic@google.com>
Reviewed-by: Julie Qiu <julieqiu@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
stanislavkononiuk added a commit to stanislavkononiuk/Golangvuln that referenced this issue Jun 26, 2023
We define environment as Go version at PATH (showed only for source
mode), govulncheck version (effectively x/vuln version), and list of
vulnerability databases with their timestamp.

The CL also moves some code around.

Fixes golang/go#56097
Fixes golang/go#56514
Updates golang/go#56207

Change-Id: I2e2f179a5421b3dfc1e1f1e4bd0ed13d16735364
Reviewed-on: https://go-review.googlesource.com/c/vuln/+/463106
Reviewed-by: Jonathan Amsterdam <jba@google.com>
Run-TryBot: Zvonimir Pavlinovic <zpavlinovic@google.com>
Reviewed-by: Julie Qiu <julieqiu@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
@golang golang locked and limited conversation to collaborators Jan 30, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FrozenDueToAge UX Issues that involve UXD/UXR input vulncheck or vulndb Issues for the x/vuln or x/vulndb repo
Projects
None yet
Development

No branches or pull requests

3 participants