x/vuln/cmd/govulncheck: consider showing all locations a vulnerable symbol is called rather than only one

fflewddur commented 1 year ago

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().

mknyszek commented 1 year ago

CC @golang/vulndb

gopherbot commented 1 year ago

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

gopherbot commented 1 year ago

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

golang / go