debug/elf: timeout/oom in NewFile

[catenacyber]

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/yNYIA1Rditp?v=gotip

What did you expect to see?

The program finishing and printing Hello

What did you see instead?

Program exited.

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

[mknyszek]

Is this related to #45599? CC @ianlancetaylor @thanm

[mknyszek]

Note that #45599 was fixed about a month ago on tip. I see that you're running go.dev/play against tip, so maybe this is still an issue?

[catenacyber]

Yes this is an issue on tip, so likely similar to #45599 but different...

[gopherbot]

Change https://go.dev/cl/429601 mentions this issue: debug/elf: avoid using saferio to read data from SHT_NOBITS sections

[ZekeLu]

The test case uncovers another issue: incorrect huge size (3349099659509720927 bytes) of the SHT_NOBITS section. An SHT_NOBITS section may have a nonzero size, but it occupies no space in the file. Since the section size is "conceptual" and the data is provided by a zeroReader, saferio.ReadData does not help in this case. In fact, saferio.ReadData will make it worse because it will eat up the memory one small block by one small block and results in an OOM finally.

It seems that there is not way to tell whether the section size is valid. So https://go.dev/cl/429601 returns the section data by calling make([]byte, s.Size) directly. When the size is huge, it panics with runtime error: makeslice: len out of range fast.

I think a further fix is to set a limit for the size of the SHT_NOBITS section and refuse to read from this section when the size is larger than the limit. But I can not find a documented limit for this section.

[kortschak]

The docs at https://refspecs.linuxfoundation.org/LSB_2.1.0/LSB-Core-generic/LSB-Core-generic/elftypes.html have

Name	Value	Description
SHT_NOBITS	0x8	A section of this type occupies no space in the file but otherwise resembles SHT_PROGBITS. Although this section contains no bytes, the sh_offset member contains the conceptual file offset.

I'd suggest that for this case we do

func (s *Section) Data() ([]byte, error) {
    if s.Type == SHT_NOBITS {
        return nil, nil
    }
    return saferio.ReadData(s.Open(), s.Size)
}

This reflects the wording from the docs in that no data is present, but the user can still obtain the sh_offset through the s.Offset field. The user can obviously also obtain the claimed size through s.Size.

[gopherbot]

Change https://go.dev/cl/430155 mentions this issue: debug/elf: validate shstrndx

[ZekeLu]

@kortschak I like this solution! See also the discussion in #18667.

Since the CL (https://go.dev/cl/375216) that introduced this behavior has been released since go1.18, it seems that it's too late to change the behavior now.

And this solution requires the caller to check the section type. If the caller would like to check the section type, it can avoid calling (*Section).Data at all when the section type is SHT_NOBITS¹. It seems that breaking the compatibility does not worth it.

Regarding the caller of (*Section).Data. I found that debug/elf/file.go should not call (*Section).Data when the section type is SHT_NOBITS. The testing elf file has an invalid shstrndx pointing to the SHT_NOBITS section. According to https://refspecs.linuxfoundation.org/elf/gabi4+/ch4.eheader.html, shstrndx should point to the section name string table, which has the type SHT_STRTAB. I have sent another CL (https://go.dev/cl/430155) to validate shstrndx, which will in turn prevent it from calling (*Section).Data at the first place.

Footnotes

1. here is one such caller: https://github.com/aclements/go-obj/blob/91d9e299b01bb7d48c3ab33b24cdabb3fec63885/obj/elf.go#L420-L454 ↩

[ZekeLu]

@aarzilli Can you take a look at https://go.dev/cl/429601 ? The consensus there is to let (*Section).Data return an error when it is an SHT_NOBITS section. Thank you!