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: govulncheck is confused when go command version doesn't match that used to build it #55045

Closed
neild opened this issue Sep 13, 2022 · 6 comments
Assignees
Labels
FrozenDueToAge vulncheck or vulndb Issues for the x/vuln or x/vulndb repo
Milestone

Comments

@neild
Copy link
Contributor

neild commented Sep 13, 2022

$ go version
go version devel go1.20-54182ff54a Fri Sep 9 20:29:05 2022 +0000 darwin/arm64
$ ~/src/gos/go1.18.5/bin/go version
go version go1.18.5 darwin/arm64
$ ~/src/gos/go1.18.5/bin/go run golang.org/x/vuln/cmd/govulncheck@latest .
govulncheck is an experimental tool. Share feedback at https://go.dev/s/govulncheck-feedback.

Scanning for dependencies with known vulnerabilities...
govulncheck: Packages contain errors:
/Users/dneil/src/go2/src/runtime/debuglog.go:660:15: String not declared by package unsafe
/Users/dneil/src/go2/src/runtime/proc.go:623:16: String not declared by package unsafe
/Users/dneil/src/go2/src/strings/builder.go:48:16: String not declared by package unsafe
/Users/dneil/src/go2/src/strings/builder.go:48:30: SliceData not declared by package unsafe
/Users/dneil/src/go2/src/strings/clone.go:27:16: String not declared by package unsafe
/Users/dneil/src/go2/src/os/file.go:249:27: StringData not declared by package unsafe
/Users/dneil/src/go2/src/crypto/x509/internal/macos/corefoundation.go:66:29: SliceData not declared by package unsafe
/Users/dneil/src/go2/src/crypto/x509/internal/macos/corefoundation.go:77:29: StringData not declared by package unsafe

exit status 1
$ cat main.go
package main

import "net/http"

func main() {
	http.ListenAndServe(":8080", nil)
}

Not quite sure what's going on here.

@gopherbot gopherbot added this to the Unreleased milestone Sep 13, 2022
@zpavlinovic zpavlinovic modified the milestones: Unreleased, vuln/2022 Sep 13, 2022
@julieqiu julieqiu added vulncheck or vulndb Issues for the x/vuln or x/vulndb repo and removed x/vuln labels Sep 13, 2022
@adonovan
Copy link
Member

This is clearly a problem in go/packages, but I can't reproduce it (either in vulncheck or in go/packages/gopackages), using the exact same two go versions on my machine.

# build go1.20 devel
$ cd $HOME/w/goroot/src/
$ git co 54182ff54a
$ ./make.bash
$ ../bin/go version 
go version devel go1.20-54182ff54a Fri Sep 9 20:29:05 2022 +0000 darwin/arm64

# install go1.18.5
$ go install golang.org/dl/go1.18.5@latest
$ go1.18.5 download
$ go1.18.5 version
go version go1.18.5 darwin/arm64

# build govulncheck with 1.18 and run it with 1.20-devel
$ cd $HOME/w/vulndb
$ git co abdd677
$ (export PATH=$HOME/w/goroot/bin:$PATH; go version; go1.18.5 run ./cmd/govulncheck/ .)
go version devel go1.20-54182ff54a Fri Sep 9 20:29:05 2022 +0000 darwin/arm64
govulncheck is an experimental tool. Share feedback at https://go.dev/s/govulncheck-feedback.

Scanning for dependencies with known vulnerabilities...
Found 1 known vulnerability.

Vulnerability #1: GO-2022-0969 [...success message omitted...]
exit status 3

# target program
$ cat main.go 
package main

import "net/http"

func main() {
	http.ListenAndServe(":8080", nil)
}

Can you verify this by running golang.org/x/tools/go/packages/gopackages -mode=types instead of govulncheck, and report what it prints? That would at least eliminate govulncheck.

@zpavlinovic
Copy link
Contributor

This is clearly a problem in go/packages, but I can't reproduce it (either in vulncheck or in go/packages/gopackages), using the exact same two go versions on my machine.

...

This is what I have.

$ go version
go version devel go1.20-54182ff54a Fri Sep 9 20:29:05 2022 +0000 linux/amd64

$ ~/old_go/go1.18.5/bin/go version
go version go1.18.5 linux/amd64

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

Scanning for dependencies with known vulnerabilities...
No vulnerabilities found.

$ ~/old_go/go1.18.5/bin/go run ./cmd/govulncheck/ .
govulncheck is an experimental tool. Share feedback at https://go.dev/s/govulncheck-feedback.

Scanning for dependencies with known vulnerabilities...
govulncheck: Packages contain errors:
[redacted path]/go/src/runtime/debuglog.go:660:15: String not declared by package unsafe
...

exit status 1

Can you verify this by running golang.org/x/tools/go/packages/gopackages -mode=types instead of govulncheck, and report what it prints? That would at least eliminate govulncheck.

gopackages seems to not report errors in dependent packages, which vulncheck does. I've added

 if packages.PrintErrors(lpkgs) > 0 {
                return errors.New("errors in dependent packages")
 }

to gopackages. Then I get an error with allsyntax mode

gopackages -mode=allsyntax .
[redacted=path]/go/src/runtime/debuglog.go:660:15: String not declared by package 
...
gopackages: errors in dependent packages

but not with types mode.

@adonovan
Copy link
Member

adonovan commented Sep 27, 2022

gopackages seems to not report errors in dependent packages, which vulncheck does. I've added [...] Then I get an error with allsyntax mode

Ah, thanks, now it's easy to reproduce:

$ go1.18.5 build -o ./x ./go/packages/gopackages
$ ( export PATH=$HOME/w/goroot/bin:$PATH; ./x -mode=syntax runtime 2>&1 ) | less
/Users/adonovan/w/goroot/src/runtime/debuglog.go:296:20: StringData not declared by package unsafe
/Users/adonovan/w/goroot/src/runtime/debuglog.go:658:15: String not declared by package unsafe
/Users/adonovan/w/goroot/src/runtime/heapdump.go:159:37: StringData not declared by package unsafe
/Users/adonovan/w/goroot/src/runtime/heapdump.go:202:32: StringData not declared by package unsafe
...

I should have guessed the reason from the initial report, which is obvious in hindsight: you can't use an older version of go/types than the version of Go used by the source you are trying to type-check. If you use go1.18 to build the application, then its std library is from go1.18, so its go/types won't understand newer features like unsafe.StringData. (Recall that the unsafe package is implemented directly in the type checker.)

In principle you could build the application using the go1.20 source of go/types using a go1.18 toolchain (though you might need to backport go/types to go1.18 if it happens to use new features like generics) and it would work. But we don't support that.

So the bug is that go/packages doesn't issue a clear error when it encounters Go code that is too new. I think the fix is for go/packages to specify release tags to go list that are no newer than its version of go/types. I'll file a separate issue for that.

@zpavlinovic
Copy link
Contributor

Thank @adonovan, this makes sense.

@neild, would the solution to this issue in govulncheck then be a good message too?

@neild
Copy link
Contributor Author

neild commented Sep 27, 2022

@neild, would the solution to this issue in govulncheck then be a good message too?

Maybe? Certainly, if the go tool version needs to match (or just be less than?) the version used to build govulncheck, then there needs to be a good error message when this constraint is violated.

@gopherbot
Copy link
Contributor

Change https://go.dev/cl/435356 mentions this issue: go/packages: warn if 'go list' on PATH is too new

@golang golang locked and limited conversation to collaborators Sep 28, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FrozenDueToAge vulncheck or vulndb Issues for the x/vuln or x/vulndb repo
Projects
None yet
Development

No branches or pull requests

5 participants