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

debug/pe: attempt to allocate giant slice #17809

Closed
qjerome opened this issue Nov 5, 2016 · 5 comments
Closed

debug/pe: attempt to allocate giant slice #17809

qjerome opened this issue Nov 5, 2016 · 5 comments
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Milestone

Comments

@qjerome
Copy link

qjerome commented Nov 5, 2016

Please answer these questions before submitting your issue. Thanks!

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

go version go1.7 linux/amd64

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

GOARCH="amd64"
GOBIN="xxxxxxxxxxxxxxxxxxxxxx"
GOEXE=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOOS="linux"
GOPATH="/xxxxxxxxxxxxxxxx"
GORACE=""
GOROOT="/home/quentin/.gvm/gos/go1.7"
GOTOOLDIR="/home/quentin/.gvm/gos/go1.7/pkg/tool/linux_amd64"
CC="gcc"
GOGCCFLAGS="-fPIC -m64 -pthread -fmessage-length=0 -fdebug-prefix-map=/tmp/go-build542001008=/tmp/go-build -gno-record-gcc-switches"
CXX="g++"
CGO_ENABLED="1"

What did you do?

I triggered a bug in the debug/pe golang package.
I tried to attach the file which triggered the bug but neither EXE nor ZIP are allowed to be attached. So if you want it, please reach me by email.

When I open the file I get a panic with an error message: fatal error: runtime: out of memory

The bug is located at debug/pe/symbol.go:33 syms := make([]COFFSymbol, fh.NumberOfSymbols) because the NumberOfSymbols is too big:
In gdb:
print fh.NumberOfSymbols
$1 = 3168779836

This is probably due to a bad parsing of the FileHeader of the Coff file at debug/pe/file.go:84 or errors in previous structures parsed.

I also identified a logic problem in the package that may be link to the problem. In debug/pe/pe.go the field NumberOfSymbols of struct FileHeader is type uint32. However, in debug/pe/symbol.go, a check is done on NumberOfSymbols as if it could be lower than 0 (line 26: if fh.NumberOfSymbols <= 0) which can happen only when NumberOfSymbols is 0.

I loaded the binary into another PE parser (radare2) and the NUMBER_OF_SYMBOLS field is actually (numberOfSymbols : 0x00000010 = 0x000000b8) which is far from what it is parsed by the package. I did not dig further but I guess it is enough for you to patch the package.

What did you expect to see?

No bug :)

What did you see instead?

A file not being able to be opened !

@josharian josharian changed the title Bug in debug/pe package debug/pe: attempt to allocate giant slice Nov 5, 2016
@josharian
Copy link
Contributor

cc @alexbrainman

@josharian josharian added this to the Go1.8 milestone Nov 5, 2016
@alexbrainman
Copy link
Member

I tried to attach the file which triggered the bug but neither EXE nor ZIP are allowed to be attached. So if you want it, please reach me by email.

Thank you for the offer, but I do not know your email address. You can try and send it to me (alex dot brainman at gmail dot com), but I suspect we might have trouble emailing executable. Alternatively maybe just encode the executable in some way (for example https://play.golang.org/p/1DgIEd2H3C) and attach it to this issue.

When I open the file I get a panic with an error message: fatal error: runtime: out of memory

That is not good. I will try and fix this once I see the file.

This is probably due to a bad parsing of the FileHeader of the Coff file at debug/pe/file.go:84 or errors in previous structures parsed.

I will wait to see the file before deciding.

I also identified a logic problem in the package that may be link to the problem. In debug/pe/pe.go the field NumberOfSymbols of struct FileHeader is type uint32. However, in debug/pe/symbol.go, a check is done on NumberOfSymbols as if it could be lower than 0 (line 26: if fh.NumberOfSymbols <= 0) which can happen only when NumberOfSymbols is 0.

I am not worried about that. It just a pointless extra check.

Thank you for good report.

Alex

@alexbrainman
Copy link
Member

@qjerome got your file in the mail. I will investigate this properly when I have some free time, and report back here.

If you know how to fix this and would like to do it yourself, here https://golang.org/doc/contribute.html is how to send your change.

Thank you.

Alex

@quentinmit quentinmit added the NeedsFix The path to resolution is known, but the work has not been done. label Nov 7, 2016
@rsc rsc modified the milestones: Go1.9, Go1.8 Nov 11, 2016
@alexbrainman
Copy link
Member

Here https://go-review.googlesource.com/33170 is the fixes. It is simple enough, and fixes library crash. I think CL should go in go1.8.

Alex

@gopherbot
Copy link

CL https://golang.org/cl/33170 mentions this issue.

@golang golang locked and limited conversation to collaborators Nov 17, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Projects
None yet
Development

No branches or pull requests

6 participants