x/vuln/cmd/govulncheck: unexpected trace with uncalled methods for GO-2024-3106

[knqyf263]

govulncheck version

Go: go1.23.0
Scanner: [email protected]
DB: https://vuln.go.dev
DB updated: 2024-09-06 20:44:22 +0000 UTC

No vulnerabilities found.

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

It reproduces at v1.1.3, which is the latest as of today.

Output of go env in your module/workspace:

GO111MODULE=''
GOARCH='arm64'
GOBIN=''
GOCACHE='/Users/teppei/Library/Caches/go-build'
GOENV='/Users/teppei/Library/Application Support/go/env'
GOEXE=''
GOEXPERIMENT=''
GOFLAGS=''
GOHOSTARCH='arm64'
GOHOSTOS='darwin'
GOINSECURE=''
GOMODCACHE='/Users/teppei/pkg/mod'
GONOPROXY=''
GONOSUMDB=''
GOOS='darwin'
GOPATH='/Users/teppei'
GOPRIVATE=''
GOPROXY='https://proxy.golang.org,direct'
GOROOT='/opt/homebrew/Cellar/go/1.23.0/libexec'
GOSUMDB='sum.golang.org'
GOTMPDIR=''
GOTOOLCHAIN='local'
GOTOOLDIR='/opt/homebrew/Cellar/go/1.23.0/libexec/pkg/tool/darwin_arm64'
GOVCS=''
GOVERSION='go1.23.0'
GODEBUG=''
GOTELEMETRY='local'
GOTELEMETRYDIR='/Users/teppei/Library/Application Support/go/telemetry'
GCCGO='gccgo'
GOARM64='v8.0'
AR='ar'
CC='cc'
CXX='c++'
CGO_ENABLED='1'
GOMOD='/Users/teppei/src/github.com/aquasecurity/trivy/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 -arch arm64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -ffile-prefix-map=/var/folders/9_/3y18vrrs5bz6gbkq0kc_4fv80000gn/T/go-build1122797270=/tmp/go-build -gno-record-gcc-switches -fno-common'

What did you do?

Run govulncheck on the root directory of this repository with this commit

$ git rev-parse HEAD
04a854c3373952cc555bb4c2877d7cb0c4eb8335

What did you see happen?

$ govulncheck -show trace ./...

=== Symbol Results ===

Vulnerability #1: GO-2024-3106
    Stack exhaustion in Decoder.Decode in encoding/gob
  More info: https://pkg.go.dev/vuln/GO-2024-3106
  Standard library
    Found in: encoding/[email protected]
    Fixed in: encoding/[email protected]
    Example traces found:
      #1: for function encoding/gob.Decoder.Decode
        pkg/fanal/secret/scanner.go:430:19: github.com/aquasecurity/trivy/pkg/fanal/secret.Scanner.Scan
        src/sync/once.go:67:11: sync.Once.Do
        src/sync/once.go:76:4: sync.Once.doSlow
        internal/cert/default_cert.go:49:76: google.golang.org/api/internal/cert.DefaultSource
        internal/cert/enterprise_cert.go:36:25: google.golang.org/api/internal/cert.NewEnterpriseCertificateProxySource
        client/client.go:182:26: github.com/googleapis/enterprise-certificate-proxy/client.Cred
        src/net/rpc/client.go:196:27: net/rpc.NewClient
        src/net/rpc/client.go:206:2: net/rpc.NewClientWithCodec
        src/net/rpc/client.go:109:40: net/rpc.Client.input
        src/net/rpc/client.go:228:21: net/rpc.gobClientCodec.ReadResponseHeader
        src/encoding/gob/decoder.go:193:21: encoding/gob.Decoder.Decode

Your code is affected by 1 vulnerability from the Go standard library.
This scan also found 2 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.

I've encountered an unexpected detection by govulncheck in our project. The tool reports a vulnerability (GO-2024-3106) in encoding/gob, but the reported trace doesn't align with our source code.

This is the relevant source code.
https://github.com/aquasecurity/trivy/blob/04a854c3373952cc555bb4c2877d7cb0c4eb8335/pkg/fanal/secret/scanner.go#L430-L433

Upon closer inspection, the actual code at this location is simply using sync.Once to copy a byte array:

copyCensored.Do(func() {
	censored = make([]byte, len(args.Content))
	copy(censored, args.Content)
})

This code doesn't appear to be directly calling google.golang.org/api/internal/cert.DefaultSource as reported in the trace. To verify this, I used debugger (delve) to step through the code line by line, but I couldn't confirm any call to DefaultSource.
Given this discrepancy, I'm puzzled about why govulncheck is detecting this vulnerability in our code. I'm wondering if I'm missing something or if this could be a false positive.

I'd greatly appreciate any insights into why this detection is occurring and how I might resolve or mitigate it.

What did you expect to see?

I expected govulncheck to either not report any vulnerabilities or to accurately pinpoint the uses of vulnerable functions. The current output, which seems to detect the vulnerability in code that doesn't appear to be present or in use, has left us uncertain about how to proceed with addressing this reported issue.

[gabyhelp]

Related Issues and Documentation

• x/vuln/cmd/govulncheck: doesn't report vulnerabilities for replaced modules #68254 (closed)
• x/vuln: reduce noise in example traces #61305
• x/vuln: json output always exits 0 #61704 (closed)
• x/exp/cmd/govulncheck: exclude (fixed) vulnerability info from the own project #48079 (closed)
• x/vuln: enhance deduping of similar traces #68100
• x/vuln/cmd/govulncheck: consuming 90%+ of memory and runs without an end #67838 (closed)
• x/vuln: false positive call stack for GO-2023-2402 in Kubernetes #64834
• x/vuln: govulncheck does not match when using vulnerable symbol #55937 (closed)
• x/vuln/cmd/govulncheck: stack overflow #51560 (closed)
• x/vuln/cmd/govulncheck: consider showing all locations a vulnerable symbol is called rather than only one #59485

_{(Emoji vote if this was helpful or unhelpful; more detailed feedback welcome in this discussion.)}

[adonovan]

This confusing report is due to a false edge in the call graph constructed by vulncheck. In essence, it is unable to deduce that each call to once.Do calls exactly the function f provided to it; instead, it acts as if each call to once.Do calls all functions f provided to any call to once.Do, including this one:

• https://github.com/googleapis/google-api-go-client/blob/v0.197.0/internal/cert/default_cert.go#L48

The technical term for this limitation is that the call graph lacks "context sensitivity".

--

Aside: why is the list of source locations in the trace formatted so strangely? It is quite hard to tell that some of these paths (src/sync/once.go) are in the standard library whereas others (e.g. internal/cert/default_cert.go) are in arbitrary modules:

        pkg/fanal/secret/scanner.go:430:19: github.com/aquasecurity/trivy/pkg/fanal/secret.Scanner.Scan
        src/sync/once.go:67:11: sync.Once.Do
        src/sync/once.go:76:4: sync.Once.doSlow
        internal/cert/default_cert.go:49:76: google.golang.org/api/internal/cert.DefaultSource

@zpavlinovic @timothy-king

[timothy-king]

This is a known limitation of govulncheck. https://pkg.go.dev/golang.org/x/vuln/cmd/govulncheck#hdr-Limitations

Govulncheck analyzes function pointer and interface calls conservatively, which may result in false positives or inaccurate call stacks in some cases.

As @adonovan mentioned this specific case is related to "context sensitivity". sync.Once.Do happens to be near the top of the list of functions I want to add context sensitivity to if we get around to it. Unfortunately, there are no plans to do this at the moment.

[knqyf263]

@adonovan @timothy-king Thanks for your response.

This is a known limitation of govulncheck. https://pkg.go.dev/golang.org/x/vuln/cmd/govulncheck#hdr-Limitations
Govulncheck analyzes function pointer and interface calls conservatively, which may result in false positives or inaccurate call stacks in some cases.

Yes, I read the document before, but I didn't understand how it actually produces false positives. From this statement alone, I could not tell whether mine was the case introduced due to conservative analysis.

In essence, it is unable to deduce that each call to once.Do calls exactly the function f provided to it; instead, it acts as if each call to once.Do calls all functions f provided to any call to once.Do

This explanation helps me understand my case. So, it looks like a false positive in this case.

I will be careful with the sync.Once.Do calls, but are there any other cases to watch out for? Do similar problems occur if it's passing an anonymous function, like sync.Once.Do? Or does it happen in specific functions? I would like to review the vulnerabilities found by govulncheck to see if they are correct. If you have documentation, I'll reference it. Thanks.

[zpavlinovic]

...

I will be careful with the sync.Once.Do calls, but are there any other cases to watch out for? Do similar problems occur if it's passing an anonymous function, like sync.Once.Do? Or does it happen in specific functions? I would like to review the vulnerabilities found by govulncheck to see if they are correct. If you have documentation, I'll reference it. Thanks.

It is hard to prescribe a general recipe for places to look in search of potential imprecision or false positives. That being said, functions that are used at several places and that accept other functions or interface values (as inputs or free variables) are a good candidate. sync.Once.Do is one example and sort.SliceStable might be another one.

[zpavlinovic]

Aside: why is the list of source locations in the trace formatted so strangely? It is quite hard to tell that some of these paths (src/sync/once.go) are in the standard library whereas others (e.g. internal/cert/default_cert.go) are in arbitrary modules:

        pkg/fanal/secret/scanner.go:430:19: github.com/aquasecurity/trivy/pkg/fanal/secret.Scanner.Scan
        src/sync/once.go:67:11: sync.Once.Do
        src/sync/once.go:76:4: sync.Once.doSlow
        internal/cert/default_cert.go:49:76: google.golang.org/api/internal/cert.DefaultSource

We could in principle show full paths, but we initially decided to show just the paths relative to module root/GOROOT/GOMODCACHE to avoid potential clutter (admittedly, this also simplifies implementation and tests). The symbols have package information, which should help users. We wanted to see how users would react to it before expanding them to full paths.

[adonovan]

It is hard to prescribe a general recipe for places to look in search of potential imprecision or false positives.

The general rule is: each func-typed variable in the program (e.g. the parameter f of sync.Once.Do) has a single abstraction of the set of specific function values that it can hold, independent of calling context. All values that can flow to that variable will appear to be mingled together, even if in any given call to once.Do the parameter f always refers to exactly one function.

We could in principle show full paths, but we initially decided to show just the paths relative to module root/GOROOT/GOMODCACHE to avoid potential clutter

I think it is quite confusing, as the same initial directory segments will likely appear across a variety of modules (cmd, pkg, src, internal). You could at least show the last segment of the module name, for example:

[email protected]/src/sync/once.go:67:11: ...
[email protected]/pkg/store/cache/caching_bucket_test.go:442:12: ...

[zpavlinovic]

I think it is quite confusing, as the same initial directory segments will likely appear across a variety of modules (cmd, pkg, src, internal). You could at least show the last segment of the module name, for example:

[email protected]/src/sync/once.go:67:11: ...
[email protected]/pkg/store/cache/caching_bucket_test.go:442:12: ...

Created #69490.

[knqyf263]

The general rule is: each func-typed variable in the program (e.g. the parameter f of sync.Once.Do) has a single abstraction of the set of specific function values that it can hold, independent of calling context. All values that can flow to that variable will appear to be mingled together, even if in any given call to once.Do the parameter f always refers to exactly one function.

Thanks for explaining! It will help me review the results next time. Also, if this information is documented somewhere, it may help someone facing suspect results, as I do. Anyway, I'll close this issue. Thanks.

[zpavlinovic]

I agree that we should add more documentation. I've opened an issue to track this: #69519