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: consider showing all locations a vulnerable symbol is called rather than only one #59485

Open
fflewddur opened this issue Apr 7, 2023 · 3 comments
Labels
NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. vulncheck or vulndb Issues for the x/vuln or x/vulndb repo

Comments

@fflewddur
Copy link

What version of Go are you using (go version)?

$ go version
go version go1.20.3 darwin/arm64

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

Yes

What did you do?

I ran govulncheck on a sample module that calls a single vulnerable function in three separate locations. Instead of seeing all three locations, the output only shows the filename and line number for one invocation. The sample module I used is available from https://go.dev/play/p/_B6yVIfrkZl

This is the output from govulncheck:

λ govulncheck ./...
govulncheck is an experimental tool. Share feedback at https://go.dev/s/govulncheck-feedback.

Using go1.20.3 and govulncheck@v0.0.0 with
vulnerability data from https://vuln.go.dev (last modified 2023-04-06 19:19:26 +0000 UTC).

Scanning your code and 46 packages across 1 dependent module for known vulnerabilities...
Your code is affected by 1 vulnerability from 1 module.

Vulnerability #1: GO-2021-0113
  Due to improper index calculation, an incorrectly formatted
  language tag can cause Parse to panic via an out of bounds read.
  If Parse is used to process untrusted user inputs, this may be
  used as a vector for a denial of service attack.

  More info: https://pkg.go.dev/vuln/GO-2021-0113

  Module: golang.org/x/text
    Found in: golang.org/x/text@v0.3.5
    Fixed in: golang.org/x/text@v0.3.7

    Call stacks in your code:
      main.go:76:29: github.com/julieqiu/govulncheckdemo/module5.foo3 calls golang.org/x/text/language.Parse
      main.go:88:20: github.com/julieqiu/govulncheckdemo/module5.foobar calls golang.org/x/text/language.MustParse

=== Informational ===

Found 1 vulnerability in packages that you import, but there are no call
stacks leading to the use of this vulnerability. You may not need to
take any action. See https://pkg.go.dev/golang.org/x/vuln/cmd/govulncheck
for details.

Vulnerability #1: GO-2022-1059
  An attacker may cause a denial of service by crafting an
  Accept-Language header which ParseAcceptLanguage will take
  significant time to parse.
  More info: https://pkg.go.dev/vuln/GO-2022-1059
  Found in: golang.org/x/text@v0.3.5
  Fixed in: golang.org/x/text@v0.3.8

What did you expect to see?

I expected govulncheck to report the location of all three invocations of the vulnerable function, language.Parse(). Specifically, in the "Call stacks in your code:" section, I expected to see rows for lines 50 and 63, in addition to line 76.

Alternatively, showing all invocations may make sense only in verbose mode (-v flag), but if we go that route, it would still be helpful to explain in the output how many times the vulnerable symbol is used, and that the output only shows one example. Without either of these changes, it seems possible to misinterpret govulncheck's output as saying that the given module only invokes a vulnerable symbol in one, perhaps innocuous, location, when in reality it may be invoked in other more critical locations.

What did you see instead?

govulncheck only showed the filename and line number for one invocation of language.Parse().

@fflewddur fflewddur added the vulncheck or vulndb Issues for the x/vuln or x/vulndb repo label Apr 7, 2023
@gopherbot gopherbot modified the milestones: Unreleased, vuln/unplanned Apr 7, 2023
@julieqiu julieqiu modified the milestones: vuln/unplanned, vuln/v0.2.0 Apr 7, 2023
@mknyszek mknyszek added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Apr 8, 2023
@mknyszek
Copy link
Contributor

mknyszek commented Apr 8, 2023

CC @golang/vulndb

@julieqiu julieqiu modified the milestones: vuln/v0.2.0, vuln/v0.1.0 Apr 13, 2023
@julieqiu julieqiu self-assigned this Apr 13, 2023
@julieqiu julieqiu modified the milestones: vuln/v0.1.0, vuln/v0.2.0 Apr 14, 2023
@gopherbot
Copy link

Change https://go.dev/cl/485515 mentions this issue: cmd/govulncheck: add test for multiple entry points

gopherbot pushed a commit to golang/vuln that referenced this issue Apr 18, 2023
When govulncheck finds that the same vulnerable symbol is called more
than once, only one invocation is displayed to the user.

Add a test for this behavior, before it is changed in a later CL.

For golang/go#59485

Change-Id: I71f1ce08501a5deaa4521561f065016f0aa3888c
Reviewed-on: https://go-review.googlesource.com/c/vuln/+/485515
Run-TryBot: Julie Qiu <julie@golang.org>
Reviewed-by: Zvonimir Pavlinovic <zpavlinovic@google.com>
Reviewed-by: Julie Qiu <julieqiu@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
@gopherbot
Copy link

Change https://go.dev/cl/485898 mentions this issue: cmd/govulncheck/testdata: add test for json mode for multiple entry points

gopherbot pushed a commit to golang/vuln that referenced this issue Apr 18, 2023
…oints

 When govulncheck finds that the same vulnerable symbol is called more
than once, only one invocation is displayed to the user in -json mode.

Add a test for this behavior, before it is changed in a later CL.

For golang/go#59485

Change-Id: I667e3e3c9c113991383b040191edb25858f992bb
Reviewed-on: https://go-review.googlesource.com/c/vuln/+/485898
Auto-Submit: Julie Qiu <julieqiu@google.com>
Run-TryBot: Julie Qiu <julie@golang.org>
Reviewed-by: Julie Qiu <julieqiu@google.com>
Reviewed-by: Tatiana Bradley <tatianabradley@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Zvonimir Pavlinovic <zpavlinovic@google.com>
@julieqiu julieqiu modified the milestones: vuln/v0.2.0, vuln/v0.3.0 Apr 18, 2023
@julieqiu julieqiu removed their assignment Apr 19, 2023
@julieqiu julieqiu modified the milestones: vuln/v0.3.0, vuln/unplanned Jul 3, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. vulncheck or vulndb Issues for the x/vuln or x/vulndb repo
Projects
None yet
Development

No branches or pull requests

4 participants