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: doesn't report vulnerabilities for replaced modules #68254

Closed
ivanvc opened this issue Jul 1, 2024 · 6 comments
Closed
Assignees
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

@ivanvc
Copy link

ivanvc commented Jul 1, 2024

govulncheck version

Go: go1.21.11
Scanner: [email protected]
DB: https://vuln.go.dev
DB updated: 2024-06-28 18:33:10 +0000 UTC

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

Yes

Output of go env in your module/workspace:

GO111MODULE=''
GOARCH='amd64'
GOBIN=''
GOCACHE='/home/ivan/.cache/go-build'
GOENV='/home/ivan/.config/go/env'
GOEXE=''
GOEXPERIMENT=''
GOFLAGS=''
GOHOSTARCH='amd64'
GOHOSTOS='linux'
GOINSECURE=''
GOMODCACHE='/home/ivan/.local/share/asdf/installs/golang/1.21.11/packages/pkg/mod'
GONOPROXY=''
GONOSUMDB=''
GOOS='linux'
GOPATH='/home/ivan/.local/share/asdf/installs/golang/1.21.11/packages'
GOPRIVATE=''
GOPROXY='https://proxy.golang.org,direct'
GOROOT='/home/ivan/.local/share/asdf/installs/golang/1.21.11/go'
GOSUMDB='sum.golang.org'
GOTMPDIR=''
GOTOOLCHAIN='auto'
GOTOOLDIR='/home/ivan/.local/share/asdf/installs/golang/1.21.11/go/pkg/tool/linux_amd64'
GOVCS=''
GOVERSION='go1.21.11'
GCCGO='gccgo'
GOAMD64='v1'
AR='ar'
CC='gcc'
CXX='g++'
CGO_ENABLED='1'
GOMOD='/home/ivan/Code/Personal/etcd/etcd/go.mod'
GOWORK=''
CGO_CFLAGS='-O2 -g'
CGO_CPPFLAGS=''
CGO_CXXFLAGS='-O2 -g'
CGO_FFLAGS='-O2 -g'
CGO_LDFLAGS='-O2 -g'
PKG_CONFIG='pkg-config'
GOGCCFLAGS='-fPIC -m64 -pthread -Wl,--no-gc-sections -fmessage-length=0 -ffile-prefix-map=/tmp/go-build879763449=/tmp/go-build -gno-record-gcc-switches'

What did you do?

At etcd-io/etcd, we recently had a vulnerability reported (GHSA-5x4g-q5rc-36jp / golang/vulndb#2952). Our govulncheck job didn't catch this report, as we're using a replace in our go.mod; we were notified by other projects that had our project as a dependency.

Running govulncheck at our repository yields no vulnerabilities:

$ govulncheck ./...
No vulnerabilities found.

What did you see happen?

As stated, it didn't report the vulnerability. If we remove the replace statements from go.mod (i.e., go mod edit -json | jq .Replace[].Old.Path -r | xargs -n1 go mod edit -dropreplace && go mod tidy):

diff --git a/go.mod b/go.mod
index 356ad3485..24231d411 100644
--- a/go.mod
+++ b/go.mod
@@ -4,19 +4,6 @@ go 1.21
 
 toolchain go1.21.11
 
-replace (
-       go.etcd.io/etcd/api/v3 => ./api
-       go.etcd.io/etcd/client/pkg/v3 => ./client/pkg
-       go.etcd.io/etcd/client/v2 => ./client/v2
-       go.etcd.io/etcd/client/v3 => ./client/v3
-       go.etcd.io/etcd/etcdctl/v3 => ./etcdctl
-       go.etcd.io/etcd/etcdutl/v3 => ./etcdutl
-       go.etcd.io/etcd/pkg/v3 => ./pkg
-       go.etcd.io/etcd/raft/v3 => ./raft
-       go.etcd.io/etcd/server/v3 => ./server
-       go.etcd.io/etcd/tests/v3 => ./tests
-)
-
 require (
        github.com/bgentry/speakeasy v0.1.0
        github.com/dustin/go-humanize v1.0.0

It does report the vulnerabilities.

I'm unsure if this is a limitation/expected behavior of govulncheck, or a bug. If it is a limitation of govulncheck, we'll need to find a workaround to automate a job that can detect vulnerabilities reported in vulndb for our own source code.

What did you expect to see?

$ govulncheck ./...

=== Symbol Results ===

Vulnerability #1: GO-2024-2527
    Etcd pkg Insecure ciphers are allowed by default in
    go.etcd.io/etcd/client/pkg/v3
  More info: https://pkg.go.dev/vuln/GO-2024-2527
  Module: go.etcd.io/etcd/client/pkg/v3
    Found in: go.etcd.io/etcd/client/pkg/[email protected]
    Fixed in: N/A
    Example traces found:
      #1: tools/etcd-dump-metrics/main.go:155:31: etcd.main calls embed.StartEtcd, which eventually calls fileutil.CreateDirAll
      #2: contrib/raftexample/raft.go:275:20: raftexample.raftNode.startRaft calls fileutil.Exist
      #3: tools/etcd-dump-logs/main.go:111:9: etcd.main calls wal.WAL.Close, which eventually calls fileutil.Fdatasync
      #4: tools/etcd-dump-logs/main.go:110:42: etcd.main calls wal.WAL.ReadAll, which eventually calls fileutil.FileBufReader.FileInfo
      #5: tools/etcd-dump-metrics/main.go:155:31: etcd.main calls embed.StartEtcd, which eventually calls fileutil.Fsync
      #6: tools/etcd-dump-metrics/main.go:155:31: etcd.main calls embed.StartEtcd, which eventually calls fileutil.IsDirWriteable
      #7: tools/etcd-dump-metrics/main.go:155:31: etcd.main calls embed.StartEtcd, which eventually calls fileutil.LockFile
      #8: tools/etcd-dump-logs/main.go:106:27: etcd.main calls wal.OpenForRead, which eventually calls fileutil.NewFileBufReader
      #9: tools/etcd-dump-logs/main.go:106:27: etcd.main calls wal.OpenForRead, which eventually calls fileutil.NewFileReader
      #10: tools/etcd-dump-metrics/main.go:155:31: etcd.main calls embed.StartEtcd, which eventually calls fileutil.OpenDir
      #11: tools/etcd-dump-metrics/main.go:155:31: etcd.main calls embed.StartEtcd, which eventually calls fileutil.Preallocate
      #12: tools/etcd-dump-metrics/main.go:155:31: etcd.main calls embed.StartEtcd, which eventually calls fileutil.PurgeFileWithDoneNotify
      #13: tools/etcd-dump-metrics/main.go:155:31: etcd.main calls embed.StartEtcd, which eventually calls fileutil.PurgeFileWithoutFlock
      #14: contrib/raftexample/raft.go:282:21: raftexample.raftNode.startRaft calls wal.Exist, which calls fileutil.ReadDir
      #15: tools/etcd-dump-metrics/main.go:155:31: etcd.main calls embed.StartEtcd, which eventually calls fileutil.RemoveMatchFile
      #16: tools/etcd-dump-metrics/main.go:155:31: etcd.main calls embed.StartEtcd, which eventually calls fileutil.TouchDirAll
      #17: tools/etcd-dump-logs/main.go:106:27: etcd.main calls wal.OpenForRead, which eventually calls fileutil.TryLockFile
      #18: contrib/raftexample/raft.go:282:21: raftexample.raftNode.startRaft calls wal.Exist, which calls fileutil.WithExt
      #19: tools/etcd-dump-logs/main.go:110:42: etcd.main calls wal.WAL.ReadAll, which calls fileutil.ZeroToEnd
      #20: tools/etcd-dump-metrics/install_linux.go:27:2: etcd.init calls fileutil.init
      #21: tools/etcd-dump-metrics/main.go:155:31: etcd.main calls embed.StartEtcd, which eventually calls logutil.ConvertToZapLevel
      #22: contrib/lock/client/client.go:169:29: client.main calls client.New, which eventually calls logutil.CreateDefaultZapLogger
      #23: tools/etcd-dump-metrics/main.go:155:31: etcd.main calls embed.StartEtcd, which eventually calls logutil.MergeOutputPaths
      #24: tools/etcd-dump-metrics/main.go:155:31: etcd.main calls embed.StartEtcd, which eventually calls logutil.NewJournalWriter
      #25: tools/etcd-dump-metrics/etcd.go:27:2: etcd.init calls embed.init, which calls logutil.init
      #26: tools/etcd-dump-metrics/main.go:167:14: etcd.main calls zap.Logger.Panic, which eventually calls logutil.init
      #27: contrib/lock/client/client.go:157:13: client.main calls fmt.Printf, which eventually calls logutil.journalWriter.Write
      #28: tools/etcd-dump-metrics/main.go:155:31: etcd.main calls embed.StartEtcd, which eventually calls pathutil.CanonicalURLPath
      #29: dummy.go:21:2: etcd.init calls client.init, which calls pathutil.init
      #30: tools/etcd-dump-db/main.go:65:31: etcd.main calls cobra.Command.Execute, which eventually calls srv.GetClient
      #31: tools/etcd-dump-metrics/main.go:155:31: etcd.main calls embed.StartEtcd, which eventually calls srv.GetCluster
      #32: tools/etcd-dump-metrics/etcd.go:27:2: etcd.init calls embed.init, which calls srv.init
      #33: tools/etcd-dump-metrics/main.go:155:31: etcd.main calls embed.StartEtcd, which eventually calls systemd.DialJournal
      #34: tools/etcd-dump-metrics/etcd.go:27:2: etcd.init calls embed.init, which eventually calls systemd.init
      #35: dummy.go:24:2: etcd.init calls integration.init, which calls testutil.init
      #36: tools/etcd-dump-metrics/main.go:155:31: etcd.main calls embed.StartEtcd, which eventually calls tlsutil.GetCipherSuites
      #37: tools/etcd-dump-metrics/main.go:155:31: etcd.main calls embed.StartEtcd, which eventually calls tlsutil.GetTLSVersion
      #38: tools/etcd-dump-metrics/metrics.go:31:42: etcd.fetchMetrics calls transport.NewTimeoutTransport, which eventually calls tlsutil.NewCert
      #39: tools/etcd-dump-metrics/main.go:155:31: etcd.main calls embed.StartEtcd, which eventually calls tlsutil.NewCertPool
      #40: dummy.go:24:2: etcd.init calls integration.init, which calls tlsutil.init
      #41: tools/local-tester/bridge/bridge.go:219:22: bridge.main calls net.Listen, which eventually calls transport.Controls.Control
      #42: contrib/raftexample/raft.go:321:24: raftexample.raftNode.startRaft calls rafthttp.Transport.AddPeer, which eventually calls transport.IsClosedConnError
      #43: tools/etcd-dump-metrics/main.go:155:31: etcd.main calls embed.StartEtcd, which eventually calls transport.LimitListener
      #44: tools/etcd-dump-metrics/main.go:155:31: etcd.main calls embed.StartEtcd, which eventually calls transport.NewListenerWithOpts
      #45: tools/etcd-dump-metrics/main.go:155:31: etcd.main calls embed.StartEtcd, which eventually calls transport.NewTLSListener
      #46: tools/etcd-dump-metrics/metrics.go:31:42: etcd.fetchMetrics calls transport.NewTimeoutTransport
      #47: tools/etcd-dump-metrics/main.go:155:31: etcd.main calls embed.StartEtcd, which eventually calls transport.NewTransport
      #48: contrib/raftexample/raft.go:321:24: raftexample.raftNode.startRaft calls rafthttp.Transport.AddPeer, which eventually calls transport.NewTransport
      #49: tools/etcd-dump-metrics/main.go:155:31: etcd.main calls embed.StartEtcd, which eventually calls transport.SelfCert
      #50: tools/etcd-dump-metrics/main.go:155:31: etcd.main calls embed.StartEtcd, which calls transport.SocketOpts.Empty
      #51: tools/benchmark/cmd/util.go:100:34: cmd.mustCreateConn calls transport.TLSInfo.ClientConfig
      #52: tools/etcd-dump-metrics/main.go:155:31: etcd.main calls embed.StartEtcd, which eventually calls transport.TLSInfo.Empty
      #53: tools/etcd-dump-metrics/main.go:155:31: etcd.main calls embed.StartEtcd, which eventually calls transport.TLSInfo.ServerConfig
      #54: tools/etcd-dump-db/main.go:66:15: etcd.main calls fmt.Fprintln, which eventually calls transport.TLSInfo.String
      #55: tools/etcd-dump-metrics/main.go:155:31: etcd.main calls embed.StartEtcd, which eventually calls transport.WithSkipTLSInfoCheck
      #56: tools/etcd-dump-metrics/main.go:155:31: etcd.main calls embed.StartEtcd, which eventually calls transport.WithSocketOpts
      #57: tools/etcd-dump-metrics/main.go:155:31: etcd.main calls embed.StartEtcd, which eventually calls transport.WithTLSInfo
      #58: tools/etcd-dump-metrics/main.go:155:31: etcd.main calls embed.StartEtcd, which eventually calls transport.WithTimeout
      #59: contrib/lock/client/client.go:195:18: client.main calls bufio.Reader.ReadByte, which eventually calls transport.baseConfig
      #60: contrib/lock/client/client.go:195:18: client.main calls bufio.Reader.ReadByte, which eventually calls transport.baseConfig
      #61: contrib/lock/storage/storage.go:100:21: storage.main calls http.ListenAndServe, which eventually calls transport.baseConfig
      #62: tools/etcd-dump-metrics/metrics.go:25:2: etcd.init calls transport.init
      #63: contrib/lock/storage/storage.go:100:21: storage.main calls http.ListenAndServe, which eventually calls transport.keepaliveListener.Accept
      #64: tools/etcd-dump-metrics/main.go:155:31: etcd.main calls embed.StartEtcd, which eventually calls transport.limitListener.Accept
      #65: tools/etcd-dump-metrics/main.go:169:5: etcd.main calls embed.Etcd.Close, which eventually calls transport.limitListener.release
      #66: tools/etcd-dump-metrics/install_linux.go:39:2: etcd.install calls tls.Conn.Close, which calls transport.limitListenerConn.Close
      #67: contrib/raftexample/raft.go:321:24: raftexample.raftNode.startRaft calls rafthttp.Transport.AddPeer, which eventually calls transport.rwTimeoutDialer.Dial
      #68: contrib/lock/storage/storage.go:100:21: storage.main calls http.ListenAndServe, which eventually calls transport.rwTimeoutListener.Accept
      #69: tools/etcd-dump-metrics/main.go:155:31: etcd.main calls embed.StartEtcd, which eventually calls transport.setReuseAddress
      #70: tools/etcd-dump-metrics/main.go:155:31: etcd.main calls embed.StartEtcd, which eventually calls transport.setReusePort
      #71: contrib/lock/client/client.go:195:18: client.main calls bufio.Reader.ReadByte, which eventually calls transport.timeoutConn.Read
      #72: tools/etcd-dump-logs/main.go:354:18: etcd.listEntriesType calls io.WriteString, which calls transport.timeoutConn.Write
      #73: contrib/lock/storage/storage.go:100:21: storage.main calls http.ListenAndServe, which eventually calls transport.tlsKeepaliveListener.Accept
      #74: contrib/lock/storage/storage.go:100:21: storage.main calls http.ListenAndServe, which eventually calls transport.tlsListener.Accept
      #75: tools/etcd-dump-metrics/main.go:169:5: etcd.main calls embed.Etcd.Close, which calls transport.tlsListener.Close
      #76: tools/etcd-dump-metrics/main.go:169:5: etcd.main calls embed.Etcd.Close, which calls transport.unixListener.Close
      #77: contrib/raftexample/raft.go:321:24: raftexample.raftNode.startRaft calls rafthttp.Transport.AddPeer, which eventually calls transport.unixTransport.RoundTrip
      #78: tools/etcd-dump-metrics/main.go:169:5: etcd.main calls embed.Etcd.Close, which eventually calls types.ID.String
      #79: tools/etcd-dump-metrics/main.go:155:31: etcd.main calls embed.StartEtcd, which eventually calls types.IDFromString
      #80: tools/etcd-dump-metrics/metrics.go:55:11: etcd.getMetrics calls sort.Sort, which calls types.IDSlice.Len
      #81: tools/etcd-dump-metrics/metrics.go:55:11: etcd.getMetrics calls sort.Sort, which eventually calls types.IDSlice.Less
      #82: tools/etcd-dump-metrics/metrics.go:55:11: etcd.getMetrics calls sort.Sort, which eventually calls types.IDSlice.Swap
      #83: contrib/raftexample/raft.go:321:24: raftexample.raftNode.startRaft calls rafthttp.Transport.AddPeer, which calls types.NewURLs
      #84: tools/etcd-dump-metrics/main.go:155:31: etcd.main calls embed.StartEtcd, which eventually calls types.NewURLsMap
      #85: tools/etcd-dump-metrics/main.go:155:31: etcd.main calls embed.StartEtcd, which eventually calls types.NewUnsafeSet
      #86: tools/etcd-dump-metrics/metrics.go:55:11: etcd.getMetrics calls sort.Sort, which calls types.URLs.Len
      #87: tools/etcd-dump-metrics/metrics.go:55:11: etcd.getMetrics calls sort.Sort, which eventually calls types.URLs.Less
      #88: tools/etcd-dump-metrics/main.go:155:31: etcd.main calls embed.StartEtcd, which eventually calls types.URLs.Sort
      #89: tools/etcd-dump-metrics/main.go:155:31: etcd.main calls embed.StartEtcd, which eventually calls types.URLs.StringSlice
      #90: tools/etcd-dump-metrics/metrics.go:55:11: etcd.getMetrics calls sort.Sort, which eventually calls types.URLs.Swap
      #91: tools/etcd-dump-metrics/main.go:155:31: etcd.main calls embed.StartEtcd, which eventually calls types.URLsMap.Len
      #92: tools/etcd-dump-metrics/main.go:155:31: etcd.main calls embed.StartEtcd, which eventually calls types.URLsMap.String
      #93: tools/etcd-dump-metrics/metrics.go:55:11: etcd.getMetrics calls sort.Sort, which calls types.Uint64Slice.Len
      #94: tools/etcd-dump-metrics/metrics.go:55:11: etcd.getMetrics calls sort.Sort, which eventually calls types.Uint64Slice.Less
      #95: tools/etcd-dump-metrics/metrics.go:55:11: etcd.getMetrics calls sort.Sort, which eventually calls types.Uint64Slice.Swap
      #96: tools/etcd-dump-logs/main.go:33:2: etcd.init calls types.init
      #97: contrib/lock/storage/storage.go:100:21: storage.main calls http.ListenAndServe, which eventually calls types.unsafeSet.Add
      #98: contrib/lock/storage/storage.go:100:21: storage.main calls http.ListenAndServe, which eventually calls types.unsafeSet.Contains
      #99: contrib/lock/storage/storage.go:100:21: storage.main calls http.ListenAndServe, which eventually calls types.unsafeSet.Remove
      #100: contrib/lock/storage/storage.go:100:21: storage.main calls http.ListenAndServe, which eventually calls types.unsafeSet.Values

Your code is affected by 1 vulnerability from 1 module.
This scan found no other vulnerabilities in packages you import or modules you
require.
Use '-show verbose' for more details.
@ivanvc ivanvc added the vulncheck or vulndb Issues for the x/vuln or x/vulndb repo label Jul 1, 2024
@gopherbot gopherbot modified the milestones: Unreleased, vuln/unplanned Jul 1, 2024
@gophun
Copy link

gophun commented Jul 1, 2024

Replacements with file paths don't have versions and could be anything, which means they can't be matched against a vulnerability database.

@seankhliao
Copy link
Member

perhaps the etcd repo would be better served by using a workspace rather than local replaces?

@seankhliao seankhliao changed the title x/vuln: govulncheck doesn't report vulneratbilities for replaced modules x/vuln/cmd/govulncheck: doesn't report vulneratbilities for replaced modules Jul 1, 2024
@thanm
Copy link
Contributor

thanm commented Jul 1, 2024

@golang/vulndb

Not sure there if there is a whole lot that can be done here given the way replace works, as others have pointed out.

@thanm thanm added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Jul 1, 2024
@zpavlinovic zpavlinovic self-assigned this Jul 1, 2024
@zpavlinovic zpavlinovic changed the title x/vuln/cmd/govulncheck: doesn't report vulneratbilities for replaced modules x/vuln/cmd/govulncheck: doesn't report vulnerabilities for replaced modules Jul 1, 2024
@zpavlinovic
Copy link
Contributor

Govulncheck uses the replacement module as the source of truth. If your replacement is a local directory, then govulncheck really does not know what that replacement is and what its version is.

We could use instead the module and version being replaced, but that could lead to incorrect results. I agree with the rest of the folks here, I am not sure if there is something we can do here.

@zpavlinovic zpavlinovic added the WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided. label Jul 1, 2024
@ivanvc
Copy link
Author

ivanvc commented Jul 12, 2024

Thanks for your answers and suggestions. It makes sense to introduce a Go workspace in the project. I'm closing this issue, as I understand the limitation that govulncheck can't check for vulnerabilities if we have a replace with local source code.

@ivanvc ivanvc closed this as completed Jul 12, 2024
@thanm thanm removed the WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided. label Jul 12, 2024
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

7 participants