Suggested `go list -m json all` vulnerability checks swathes of dependencies that never end up in a binary

[dnwe]

• What are you trying to do?

When checking for vulnerabilities in Golang dependencies using Sonatype's OSS Index via nancy sleuth, the README advises to run go list -json -m all | nancy sleuth [flags]. However, this will frequently flag up vulnerabilities for test dependencies and dependencies of dependencies that never actually end up in any resulting binary, but are simply tracked by the go.mod/go.sum. These don't show up in go list -deps or go version -m <binary> and the only way to satisfy the scanner for them is to put arbritrary replace { ... } directives to bump their versions even though it has no effect on the binary.

• How could we solve this issue? (Not knowing is okay!)

Parsing the json output of go list -deps -json ./... should give a more accurate picture of the dependencies that are actually used to make up the project binaries. Currently it is possible to mux that into a go list -m all style format by doing something like:

go list -deps -json ./... \
        | jq -s 'unique_by(.Module.Path)|.[]|select(has("Module"))|.Module' \
        | nancy sleuth [flags]

But I think it would be helpful if the sleuth command itself could understand the go list -deps -json format itself and directly parse it.

cc @bhamail / @DarthHater

[dnwe]

Related issues around go list -m all vs go list -deps output can be found in the following upstream issues: https://github.com/golang/go/issues/27900 and https://github.com/golang/go/issues/26955

[DarthHater]

@dnwe this is super helpful, thanks for filing this.

I think @bhamail is taking a look, and likely @zendern . We originally used go list -json -m all because way back when, it was the best we knew of. Is the -deps flag new? That's mostly helpful so we know to tell people what version of Go they need to use, etc.. It's entirely possible we missed it when we originally looked, but I don't recall seeing it before today!

[DarthHater]

However, what do you mean by not end up in the binary, in regards to a replace? Say for example you are using viper and a version that has jwt-go and that dep is vulnerable at a certain version. You may not end up calling the code path for jwt-go but viper itself will need jwt-go available to compile. Replacing at least ensure that's when you compile, you are compiling against a non vulnerable version of that library. I have personally never gone super far down the rabbit hole (and am willing to be educated big time) on how the go compilation works, as in does it do some amount of tree shaking, so if a dependency is not really needed (but is imported by another dependency), does it (and it's resulting code paths) just never make it into the end result?

[dnwe]

@DarthHater the viper --> jwt-go dependecy (famously unmaintained and vulnerable) is a good example

Whilst there might be reachable areas of viper as a library that will reach out to etcd+jwt-go, I believe the go build command will walk the tree of import paths at the package level and will only compile-in what is reachable from your func main. Lets walkthrough my understanding of that process below:

As an example, say you just have a bare-bones main.go that looks like this:

package main

import (
    "github.com/spf13/viper"
)

func main() {
    _ = viper.Viper{}
}

The go list -m all command will claim that this depends on etcd and jwt-go:

% go list -m all | grep 'etcd\|jwt-go'
github.com/coreos/etcd v3.3.13+incompatible
github.com/dgrijalva/jwt-go v3.2.0+incompatible
go.etcd.io/bbolt v1.3.2

Yet go mod why will tell you that your main doesn't need any of those:

% go mod why -m github.com/dgrijalva/jwt-go
# github.com/dgrijalva/jwt-go
(main module does not need module github.com/dgrijalva/jwt-go)

Similarly for go list -deps:

% go list -deps -json ./... | jq -s 'unique_by(.Module.Path)|.[]|select(has("Module"))|.Module' | grep etcd

And if you verbosely compile you can see all the import paths the compiler navigates, and that doesn't include etcd or jwt-go:

% go build -a -v 2>&1 | grep 'etcd\|jwt-go'

Now if you follow Viper's documentation and decide you want to enable Remote Key/Value Store Support then you might add the import like so:

package main

import (
    "github.com/spf13/viper"
    _ "github.com/spf13/viper/remote"
)

func main() {
    _ = viper.Viper{}
}

Then the etcd client will start showing up as a true dependency:

% go build -a -v 2>&1 | grep 'etcd\|jwt-go'
github.com/coreos/etcd/pkg/pathutil
github.com/coreos/etcd/version
github.com/coreos/etcd/pkg/types
github.com/coreos/etcd/pkg/srv
github.com/coreos/etcd/client
github.com/bketelsen/crypt/backend/etcd

% go list -deps -json ./... | jq -s 'unique_by(.Module.Path)|.[]|select(has("Module"))|.Module' | grep etcd
  "Path": "github.com/coreos/etcd",
  "Dir": "/Users/dnwe/.local/pkg/mod/github.com/coreos/etcd@v3.3.13+incompatible",
  "GoMod": "/Users/dnwe/.local/pkg/mod/cache/download/github.com/coreos/etcd/@v/v3.3.13+incompatible.mod"

You'll notice however that we still haven't been compiling in jwt-go even now we've pulled in the etcd dependency. The reason for that is that jwt-go is actually a test-only dependency of the etcd client package. So your CI process would only ever actually be touching the jwt-go code if you running go test for the github.com/coreos/etcd/client package. You can find this dependency path if you checkout spf13/viper locally and then run go mod why there.

% go mod why -m github.com/dgrijalva/jwt-go
# github.com/dgrijalva/jwt-go
github.com/spf13/viper/remote
github.com/bketelsen/crypt/config
github.com/bketelsen/crypt/backend/etcd
github.com/coreos/etcd/client
github.com/coreos/etcd/client.test
github.com/coreos/etcd/integration
github.com/coreos/etcd/etcdserver
github.com/coreos/etcd/auth
github.com/dgrijalva/jwt-go

Notice the github.com/coreos/etcd/client --> github.com/coreos/etcd/client.test path there. That's because in etcd/client/main_test.go they've declared a package client_test that imports etcd/integration to stand up a real etcd server for running some integration tests against. That in turn pulls in the etcd authentication package with is what ends up needing jwt-go.

So in this case, I don't think it would be correct for nancy to flag up the jwt-go vulnerability as a problem for my parent package/module because it is unreachable for me.

Only if we ended up importing etcd/auth or jwt-go (either directly or indirectly) would it be a valid vulnerability to flag in this case

[dnwe]

@DarthHater (regarding your question on go list -deps I believe @rsc added it in go1.11 (https://github.com/golang/go/commit/90e860f1a8f1412dad5f0c6ae10b650352c3a6b3)

[DarthHater]

@dnwe this is great information, we super appreciate it! A few of us are talking about it, it may be a week or two before we get much done (PR's are welcome, if you have interest too), but this is neat in that we'd always wondered if there was a good way to exclude the deps that aren't making it into your project. You've done a lot of detective work (just like good ole nancy) and I'll likely be in touch to mail you some stuff as a thanks, as well!

[jdbranham]

May not be relevant for this particular issue...
If I'm not mistaken, go list -deps still lists dependencies that are only required for tests of the imported modules.
[which is still code that never ends up in the final build].

[DarthHater]

@jdbranham I THINK I sidestepped that in my implementation, but I did see those in TestImports in the JSON. You mind taking a look at what I put together and see if I scratched that itch :)