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: false positive for GO-2025-3408 #71484

Open
ping-localhost opened this issue Jan 30, 2025 · 11 comments
Open

x/vuln: false positive for GO-2025-3408 #71484

ping-localhost opened this issue Jan 30, 2025 · 11 comments
Assignees
Labels
vulncheck or vulndb Issues for the x/vuln or x/vulndb repo WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided.

Comments

@ping-localhost
Copy link

ping-localhost commented Jan 30, 2025

govulncheck version

Go: go1.23.5
Scanner: [email protected]
DB: https://vuln.go.dev
DB updated: 2025-01-29 20:18:58 +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='on'
GOARCH='arm64'
GOBIN=''
GOCACHE='/Users/mitchell/Library/Caches/go-build'
GOENV='/Users/mitchell/Library/Application Support/go/env'
GOEXE=''
GOEXPERIMENT=''
GOFLAGS=''
GOHOSTARCH='arm64'
GOHOSTOS='darwin'
GOINSECURE=''
GOMODCACHE='/Users/mitchell/go/pkg/mod'
GONOPROXY=''
GONOSUMDB=''
GOOS='darwin'
GOPATH='/Users/mitchell/go'
GOPRIVATE=''
GOPROXY='https://proxy.golang.org,direct'
GOROOT='/opt/homebrew/opt/go/libexec'
GOSUMDB='sum.golang.org'
GOTMPDIR=''
GOTOOLCHAIN='auto'
GOTOOLDIR='/opt/homebrew/opt/go/libexec/pkg/tool/darwin_arm64'
GOVCS=''
GOVERSION='go1.23.5'
GODEBUG=''
GOTELEMETRY='local'
GOTELEMETRYDIR='/Users/mitchell/Library/Application Support/go/telemetry'
GCCGO='gccgo'
GOARM64='v8.0'
AR='ar'
CC='cc'
CXX='c++'
CGO_ENABLED='1'
GOMOD='/dev/null'
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 -arch arm64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -ffile-prefix-map=/var/folders/c6/4l4ylj_530z56dccw0b7_pq00000gn/T/go-build2006941850=/tmp/go-build -gno-record-gcc-switches -fno-common'

What did you do?

Using github.com/mattermost/mattermost/server/public/model in my project, which has a dependency on github.com/hashicorp/yamux (which I don't use) causes [email protected] to imply that GO-2025-3408 affects me (via sync.Once which is called by time.LoadLocation).

Sample code

package main

import (
	"fmt"
	"time"

	"github.com/mattermost/mattermost/server/public/model"
)

func main() {
	// Use something that calls `sync.Once`
	netherlands, err := time.LoadLocation("Europe/Amsterdam")
	if err != nil {
		panic(err)
	}

	// Just use anything from the Mattermost package as an example
	post := &model.Post{Message: "Hello!", ChannelId: "ID"}

	// Output because we can
	fmt.Println(netherlands, post.Message)
}

Repository: https://github.com/ping-localhost/vuln-check-reproducible

What did you see happen?

[16:56:12] ➜  vuln-check-reproducible git:(master) govulncheck ./...                                  
=== Symbol Results ===

Vulnerability #1: GO-2025-3408
    DefaultConfig has dangerous defaults causing hung Read in
    github.com/hashicorp/yamux
  More info: https://pkg.go.dev/vuln/GO-2025-3408
  Module: github.com/hashicorp/yamux
    Found in: github.com/hashicorp/[email protected]
    Fixed in: N/A
    Example traces found:
      #1: main.go:12:39: vuln.main calls time.LoadLocation, which eventually calls yamux.Client
      #2: main.go:12:39: vuln.main calls time.LoadLocation, which eventually calls yamux.DefaultConfig

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.

What did you expect to see?

Since I never actually use Yamux, I do not expect the CVE to be picked up. Somewhere along the line govulncheck thinks that sync.Once.Do will call yamux.

@ping-localhost ping-localhost added the vulncheck or vulndb Issues for the x/vuln or x/vulndb repo label Jan 30, 2025
@gopherbot gopherbot modified the milestones: Unreleased, vuln/unplanned Jan 30, 2025
@zpavlinovic
Copy link
Contributor

zpavlinovic commented Jan 30, 2025

How is this a false negative? Perhaps you mean a false positive?

@ping-localhost ping-localhost changed the title x/vuln: false negative for GO-2025-3408 x/vuln: false positive for GO-2025-3408 Jan 30, 2025
@ping-localhost
Copy link
Author

How is this a false negative? Perhaps you mean a false positive?

You're totally correct. I properly named my repository and then messed up in the issue. 🤦‍♂️

My bad!

@zpavlinovic
Copy link
Contributor

zpavlinovic commented Jan 30, 2025

Since I never actually use Yamux, I do not expect the CVE to be picked up.

govulncheck does not check just the modules you directly use. It also analyzes transitive dependencies. This is absolutely necessary as such dependencies can also be vulnerable and if your code eventually exercises them, your code is also vulnerable.

Somewhere along the line govulncheck thinks that sync.Once.Do will call yamux.

It could be that your code indeed does not call yamux. It is hard for me to tell, but it looks that way. In general, govulncheck can have false positives (this is an inherent property of all vulnerability checking tools). We are working on providing suppression mechanisms for false positives.

@seankhliao
Copy link
Member

I believe this is the same as #69446 (closed without resolution).
where govulncheck lacks context sensitvity #69446 (comment)

The trace is as below, which can't be right (the standard library won't call an external dependency).

Vulnerability #1: GO-2025-3408
    DefaultConfig has dangerous defaults causing hung Read in
    github.com/hashicorp/yamux
  More info: https://pkg.go.dev/vuln/GO-2025-3408
  Module: github.com/hashicorp/yamux
    Found in: github.com/hashicorp/[email protected]
    Fixed in: N/A
    Example traces found:
      #1: for function github.com/hashicorp/yamux.Client
        main @ github.com/ping-localhost/vuln-check-reproducible/main.go:12:39
        LoadLocation @ stdlib/src/time/zoneinfo.go:675:17
        Once.Do @ stdlib/src/sync/once.go:69:11
        Once.doSlow @ stdlib/src/sync/once.go:78:4
        getGRPCMuxer @ github.com/hashicorp/go-plugin/client.go:1153:48
        NewGRPCClientMuxer @ github.com/hashicorp/go-plugin/internal/grpcmux/grpc_client_muxer.go:53:27
        Client @ github.com/hashicorp/yamux/mux.go:105:6
      #2: for function github.com/hashicorp/yamux.DefaultConfig
        main @ github.com/ping-localhost/vuln-check-reproducible/main.go:12:39
        LoadLocation @ stdlib/src/time/zoneinfo.go:675:17
        Once.Do @ stdlib/src/sync/once.go:69:11
        Once.doSlow @ stdlib/src/sync/once.go:78:4
        getGRPCMuxer @ github.com/hashicorp/go-plugin/client.go:1153:48
        NewGRPCClientMuxer @ github.com/hashicorp/go-plugin/internal/grpcmux/grpc_client_muxer.go:48:28
        DefaultConfig @ github.com/hashicorp/yamux/mux.go:58:6

@ping-localhost
Copy link
Author

ping-localhost commented Jan 30, 2025

@seankhliao thank you for posting the trace. It is exactly why I opened this issue, as that shouldn't happen.

@zpavlinovic
Copy link
Contributor

@seankhliao thank you for posting the trace. It is exactly why I opened this issue, as that shouldn't happen.

We currently don't have immediate plans on improving the precision of govulncheck. (Due to the nature of the problem, this can in principle be done indefinitely). We are instead focusing on providing supression mechanisms.

@zpavlinovic zpavlinovic self-assigned this Jan 30, 2025
@zpavlinovic zpavlinovic added the WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided. label Jan 30, 2025
@ping-localhost
Copy link
Author

ping-localhost commented Feb 3, 2025

I don't understand why you've added WaitingForInfo, since all the information has been provided AFAIK. 😅

Furthermore, I don't want to ignore this CVE, since that sets a bad precedent. It would be much better if the detection is fixed, since there is no way that a core package like sync would ever call an external package.

@zpavlinovic
Copy link
Contributor

zpavlinovic commented Feb 3, 2025

I've added the label since it will automatically close the issue unless some other information shows up that would make us keep it open.

Again, we don't have current plans on improving the precision of govulncheck. We could do that in principle forever since it is, as all other vulnerability tools, trying to solve an undecidable problem. Our focus is now on suppression mechanisms.

... since there is no way that a core package like sync would ever call an external package.

I am not sure what this means. sync.Once.Do takes a function as input. That function can be defined in the user package or an external package. I actually think that hashicorp's getGRPCMuxer is passed to sync.Once.Do somewhere in the dependency code, but it is not passed via your main -> time.LoadLocation path. Roughly speaking, this is where the analysis trips. It does not have the necessary context tracking the provenance of all values passed to sync.Once.Do, which is really hard.

@ping-localhost
Copy link
Author

ping-localhost commented Feb 4, 2025

For those that are also running into this issue (and would like to keep their CVE checks active), I've switched to using govulncheck -mode=binary, since that doesn't trigger an actual CVE error.

Source mode (default)

[12:41:08] ➜  vuln-check-reproducible git:(master) govulncheck ./...                                  
=== Symbol Results ===

Vulnerability #1: GO-2025-3408
    DefaultConfig has dangerous defaults causing hung Read in
    github.com/hashicorp/yamux
  More info: https://pkg.go.dev/vuln/GO-2025-3408
  Module: github.com/hashicorp/yamux
    Found in: github.com/hashicorp/[email protected]
    Fixed in: N/A
    Example traces found:
      #1: main.go:12:39: vuln.main calls time.LoadLocation, which eventually calls yamux.Client
      #2: main.go:12:39: vuln.main calls time.LoadLocation, which eventually calls yamux.DefaultConfig

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.

Binary mode

[12:41:24] ➜  vuln-check-reproducible git:(master) govulncheck -mode=binary vuln-check-reproducible 
=== Symbol Results ===

No vulnerabilities found.

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

@dduzgun-security
Copy link

For your info, GO-2025-3408 got withdrawn. More details here: golang/vulndb#3453

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
vulncheck or vulndb Issues for the x/vuln or x/vulndb repo WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided.
Projects
None yet
Development

No branches or pull requests

6 participants