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.

Sign up for GitHub

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

Jump to bottom

encoding/gob: preallocation in decodeSlice can consume large amounts of memory #55338

Closed
catenacyber opened this issue Sep 22, 2022 · 8 comments
Closed

encoding/gob: preallocation in decodeSlice can consume large amounts of memory #55338

catenacyber opened this issue Sep 22, 2022 · 8 comments
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided.
Milestone
Backlog

Comments

@catenacyber
Copy link
Contributor

What version of Go are you using (go version)?

$ go version
go version go1.17.6 darwin/amd64

Does this issue reproduce with the latest release?

Yes

What operating system and processor architecture are you using (go env)?

go env Output
$ go env
GO111MODULE=""
GOARCH="amd64"
GOBIN=""
GOCACHE="/Users/catena/Library/Caches/go-build"
GOENV="/Users/catena/Library/Application Support/go/env"
GOEXE=""
GOEXPERIMENT=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="darwin"
GOINSECURE=""
GOMODCACHE="/Users/catena/go/pkg/mod"
GONOPROXY=""
GONOSUMDB=""
GOOS="darwin"
GOPATH="/Users/catena/go"
GOPRIVATE=""
GOPROXY="https://proxy.golang.org,direct"
GOROOT="/usr/local/go"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/usr/local/go/pkg/tool/darwin_amd64"
GOVCS=""
GOVERSION="go1.17.6"
GCCGO="gccgo"
AR="ar"
CC="clang"
CXX="clang++"
CGO_ENABLED="1"
GOMOD="/Users/catena/go/src/github.com/catenacyber/go/src/go.mod"
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -arch x86_64 -m64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fdebug-prefix-map=/var/folders/pp/dc1dtf9x2js3v0jx_m010nqr0000gn/T/go-build4237848497=/tmp/go-build -gno-record-gcc-switches -fno-common"
GOROOT/bin/go version: go version go1.17.6 darwin/amd64
GOROOT/bin/go tool compile -V: compile version go1.17.6
uname -v: Darwin Kernel Version 21.3.0: Wed Jan  5 21:37:58 PST 2022; root:xnu-8019.80.24~20/RELEASE_X86_64
ProductName:	macOS
ProductVersion:	12.2.1
BuildVersion:	21D62
lldb --version: lldb-1316.0.9.41
Apple Swift version 5.6 (swiftlang-5.6.0.323.62 clang-1316.0.20.8)
gdb --version: GNU gdb (GDB) 9.1

What did you do?

Run https://go.dev/play/p/nQR5wqtQV1K?v=gotip

What did you expect to see?

The program finishing and printing somme dummy data

What did you see instead?

Nothing but


Program exited.

This ends up allocating 4GB from encoding/gob.(*Decoder).decodeSlice
Can we have some function to set ourselves tooBig ?

Found by https://github.com/catenacyber/ngolo-fuzzing on oss-fuzz
https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=50748

cc @rolandshoemaker

The team decided on treating this issue as PUBLIC track per our security policy, in particular because the documentation for Decoder indicates it is not designed with adversarial inputs in mind
@mvdan
Copy link
Member

mvdan commented Sep 22, 2022

See previously #27635 and #28321.

Can we have some function to set ourselves tooBig ?

For the sake of fuzzing, I guess you could patch the codebase to change the constant to whatever you see fit.

I'm not sure whether it makes sense to add an API option just for that limit; if we were to improve the API for handling untrusted data, I think we would have to do it more generally, like #20221.

@cherrymui cherrymui added this to the Backlog milestone Sep 22, 2022
@cherrymui cherrymui added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Sep 22, 2022
@cherrymui
Copy link
Member

This ends up allocating 4GB from encoding/gob.(*Decoder).decodeSlice

Sorry, is this the bug? Is allocating 4GB OOM? Or what is it? It is unclear to me what the problem is. Thanks.

Can we have some function to set ourselves tooBig ?

I'm not sure I understand what the purpose of adjusting tooBig is. Could you clarify? (But based on the comment above, this is probably not what we're going to do.)

@cherrymui cherrymui added the WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided. label Sep 22, 2022
@rolandshoemaker
Copy link
Member

rolandshoemaker commented Sep 22, 2022 •
edited

decodeSlice is preemptively allocating a slice as described by a malformed gob message, which describes a very large slice but not its content, by using reflect.MakeSlice. Because of this an adversarial, or corrupt, message can cause a very small message to consume a very large amount of memory.

One solution is not preallocating the slice and using reflect.Append instead to grow it (this will presumably have some performance impact.)

@cherrymui
Copy link
Member

$ go version
go version go1.17.6 darwin/amd64

What version of Go do you use? Go 1.17.6 is no longer supported. I understand that the bug may still exist, but could you use newer version and report bugs for a supported version of Go? Thanks.

@rolandshoemaker rolandshoemaker changed the title encoding/gob: oom encoding/gob: preallocation in decodeSlice can consume large amounts of memory Sep 22, 2022
@qiulaidongfeng
Copy link
Contributor

go version
go version go1.19 windows/amd64

Will output Hello, 世界
But in https://go.dev/play/p/nQR5wqtQV1K?v=gotip , Will output Program exited.

@ianlancetaylor
Copy link
Contributor

Let's just use reflect.Append. Sent https://go.dev/cl/433296.

@gopherbot
Copy link

Change https://go.dev/cl/433296 mentions this issue: encoding/gob: use saferio.SliceCap when decoding a slice

@catenacyber
Copy link
Contributor Author

What version of Go do you use? Go 1.17.6 is no longer supported. I understand that the bug may still exist, but could you use newer version and report bugs for a supported version of Go? Thanks.

I actually now use go version go1.18.3 darwin/amd64
But I just copy pasted the previous reports ;-)
And I use the playground to test these reports

@golang golang locked and limited conversation to collaborators Sep 25, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided.
Projects
None yet
Milestone
Backlog
Development

No branches or pull requests
7 participants
@ianlancetaylor @mvdan @rolandshoemaker @gopherbot @cherrymui @catenacyber @qiulaidongfeng