debug/buildinfo,internal/xcoff: oom (infinite loop ?)

[catenacyber]

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

$ go version
go version go1.21 linux/amd64

Does this issue reproduce with the latest release?

It happens only on gotip, not on go 1.21

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

go env Output

$ go env
GO111MODULE=""
GOARCH="amd64"
GOBIN=""
GOCACHE="/root/.cache/go-build"
GOENV="/root/.config/go/env"
GOEXE=""
GOEXPERIMENT=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOINSECURE=""
GOMODCACHE="/root/go/pkg/mod"
GONOPROXY=""
GONOSUMDB=""
GOOS="linux"
GOPATH="/root/go"
GOPRIVATE=""
GOPROXY="https://proxy.golang.org,direct"
GOROOT="/root/.go"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/root/.go/pkg/tool/linux_amd64"
GOVCS=""
GOVERSION="go1.21"
GCCGO="gccgo"
GOAMD64="v1"
AR="ar"
CC="clang"
CXX="clang++"
CGO_ENABLED="1"
GOMOD="/src/ngolo-fuzzing/go.mod"
GOWORK=""
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -m64 -pthread -fno-caret-diagnostics -Qunused-arguments -Wl,--no-gc-sections -fmessage-length=0 -fdebug-prefix-map=/tmp/go-build2481516251=/tmp/go-build -gno-record-gcc-switches"

What did you do?

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

What did you expect to see?

The program finishing and printing Hello, without allocating too much
(as happens with go 1.21)

Regression range is given as 5fe3f0a:856cf23a8acfa14756a6e9b82ace76f5604262c9

What did you see instead?

Nothing : Program exited

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

[mauri870]

I was able to reproduce this in linux as well as macos. I believe it is related to CL514075, issue #61644.

The initial magic bytes matches conditions for xcoff, the routine that is causing the high memory usage is xcoffExe.ReadData, particulary the line with saferio.ReadDataAt

[mauri870]

@gopherbot add NeedsFix

[mauri870]

Same issue as #59817, but there it was fixed for PE.

[mauri870]

I think we are missing some validation when opening an xcoff file, the section size for this example is 3989164744982871388 which causes this append to grow the buffer too large, 10MB at a time. The logic appears to be correct, maybe there is something off with the validation that this a valid xcoff file.

[AlexanderYastrebov]

I think the logic is disconnected here:

go/src/debug/buildinfo/buildinfo.go

Lines 151 to 163 in 340a4f5

	// Read segment or section to find the build info blob.
	// On some platforms, the blob will be in its own section, and DataStart
	// returns the address of that section. On others, it's somewhere in the
	// data segment; the linker puts it near the beginning.
	// See cmd/link/internal/ld.Link.buildinfo.
	dataAddr, dataSize := x.DataStart()
	if dataSize == 0 {
	return "", "", errNotGoExe
	}
	data, err := x.ReadData(dataAddr, dataSize)
	if err != nil {
	return "", "", err
	}

It tries to locate data section address by section type (x.DataStart()):

go/src/debug/buildinfo/buildinfo.go

Lines 405 to 410 in 340a4f5

	func (x *xcoffExe) DataStart() (uint64, uint64) {
	if s := x.f.SectionByType(xcoff.STYP_DATA); s != nil {
	return s.VirtualAddress, s.Size
	}
	return 0, 0
	}

but then finds a wrong section based on the virtual address (x.ReadData()):

go/src/debug/buildinfo/buildinfo.go

Lines 392 to 403 in 340a4f5

	func (x *xcoffExe) ReadData(addr, size uint64) ([]byte, error) {
	for _, sect := range x.f.Sections {
	if sect.VirtualAddress <= addr && addr <= sect.VirtualAddress+sect.Size-1 {
	n := sect.VirtualAddress + sect.Size - addr
	if n > size {
	n = size
	}
	return saferio.ReadDataAt(sect, n, int64(addr-sect.VirtualAddress))
	}
	}
	return nil, errors.New("address not mapped")
	}

The wrong section happens to be a .bss section that uses zeroReaderAt that reads infinite amount of zeroes:

go/src/internal/xcoff/file.go

Lines 263 to 265 in 340a4f5

	if scnptr == 0 { // .bss must have all 0s
	r2 = zeroReaderAt{}
	}

Its not clear whether sections virtual addresses could actually overlap, if not then this should be validated during parsing.
Also using a never-ending reader does not seem like a safe choice.

Update: apparently buildInfoMagic blob may reference addresses in other sections.

[AlexanderYastrebov]

Maybe using eofReaderAt that always returns 0, io.EOF for .bss instead of zeroReaderAt would be enough to fix this.

[mauri870]

@AlexanderYastrebov Would you be willing to send in a fix? Thanks

[gopherbot]

Change https://go.dev/cl/553616 mentions this issue: internal/xcoff: change zeroReaderAt to eofReaderAt for .bss with all 0s