Go detector overreports dependencies, sometimes causing false vuln claims that are unusually difficult to "upgrade anyway"

[dagood]

I'm helping work on a project that needs to build https://github.com/konveyor/kantra/tree/215adcbbdfc337b5dee439404dc280d78e3e62f6 in Microsoft systems. This means component-detection is used to detect its dependencies, and the dependencies are associated with known vulnerabilities.

Behavior

CG claimed recently that that commit depends on golang.org/x/crypto v0.23.0, and the alert says:

Upgrade golang.org/x/crypto from v0.23.0 to 0.31.0 to fix the vulnerability.

We think this is a false positive: the project doesn't depend on x/crypto. But, it wouldn't hurt to upgrade anyway, and we might as well, to be sure. So, we run go get -v -u golang.org/x/[email protected]. That adds the dependency to go.mod and go.sum.

Then we run go mod tidy to clean up and prepare to submit the PR. It removes the dependency from go.mod and go.sum!

This puts us in a bad state, where there are a few workarounds:

• Never run go mod tidy again (and adjust CI to not require tidiness). This is a bad continuous state to be in.
• Add fake usage of x/crypto to a Go file just so that the dependency sticks around in go.mod.
  • Fortunately, by using a tools build tag, this has no side effects other than clutter. But you need to be familiar with Go and with the specific project's infrastructure to apply this workaround consistently.

I think component-detection needs to fix this type of false positive. It's particularly disruptive, compared with other types of false positive where we can simply upgrade the dependency whether or not it's strictly necessary.

Repro example and go list

I put together a simplified example of this false positive. https://github.com/dagood/repro/tree/repro/go-cg-unupgradeable-component/myapp is an example app myapp that depends on a module somelibrary that uses x/crypto v0.23.0, but only one of its two library packages uses a package from x/crypto.

The detector currently uses go list -mod=readonly -m -json all to discover dependencies. The -m means it's in "module mode", which doesn't match what go list, go mod tidy, and the rest of the Go ecosystem normally cares about, which are packages. go mod graph would overreport in a similar way if it were used for initial component detection.

...
{
	"Path": "golang.org/x/crypto",
	"Version": "v0.23.0",
	"Time": "2024-05-06T13:42:02Z",
	"Indirect": true
}

If you use e.g. go list -mod=readonly -deps -f '{{.ImportPath}} ---- {{.Module}}' ./..., it doesn't include x/crypto because at a package level, x/crypto is totally unused.

...
fmt ---- <nil>
strings ---- <nil>
example.org/somelibrary/numbers ---- example.org/somelibrary v1.0.0 => ../somelibrary
example.org/myapp ---- example.org/myapp

(You can also add logic to the command itself to filter down to packages that are associated with a module: go list -mod=readonly -deps -f '{{if .Module}}{{.ImportPath}} ---- {{.Module}}{{end}}' ./....)

I also wrote a test app that does use the part of somelibrary that uses x/crypto, and you can see x/crypto 0.23.0 show up in the right places to be detected: https://github.com/dagood/repro/tree/repro/go-cg-unupgradeable-component/myappcrypto.

govulncheck

I'm not proposing to use govulncheck for Component Governance. It would be good, but not necessary to fix the most disruptive false positives. It looks even deeper at the dependencies. For example:

$ govulncheck ./... # in myapp:
No vulnerabilities found.

$ govulncheck ./... # in myappcrypto
=== Symbol Results ===

No vulnerabilities found.

Your code is affected by 0 vulnerabilities.
This scan also found 0 vulnerabilities in packages you import and 1
vulnerability in modules you require, but your code doesn't appear to call these
vulnerabilities.
Use '-show verbose' for more details.

(The vulnerability is specifically in golang.org/x/crypto/ssh, which isn't used here.)