Skip to content

debug/pe: (*File).ImportedSymbols's ImportDirectory decoding implicitly assumes it can decode LittleEndian values when data might be short #76724

@odeke-em

Description

@odeke-em

Go version

go1.26-devel_ce0e803ab9 Mon Aug 11 11:12:55 2025 -0700

Output of go env in your module/workspace:

AR='ar'
CC='clang'
CGO_CFLAGS='-I/Users/emmanuelodeke/Desktop/openSrc/rocksdb/include'
CGO_CPPFLAGS=''
CGO_CXXFLAGS='-O2 -g'
CGO_ENABLED='1'
CGO_FFLAGS='-O2 -g'
CGO_LDFLAGS='-L/Users/emmanuelodeke/Desktop/openSrc/rocksdb -lrocksdb -lstdc++ -lm -lz -lbz2 -lsnappy -llz4 -lzstd'
CXX='clang++'
GCCGO='gccgo'
GO111MODULE=''
GOAMD64='v1'
GOARCH='amd64'
GOAUTH='netrc'
GOBIN='/Users/emmanuelodeke/go/bin'
GOCACHE='/Users/emmanuelodeke/Library/Caches/go-build'
GOCACHEPROG=''
GODEBUG=''
GOENV='/Users/emmanuelodeke/Library/Application Support/go/env'
GOEXE=''
GOEXPERIMENT=''
GOFIPS140='off'
GOFLAGS=''
GOGCCFLAGS='-fPIC -arch x86_64 -m64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -ffile-prefix-map=/var/folders/v3/7z434qpx5v3bw0wh8h2myfpw0000gn/T/go-build3287023084=/tmp/go-build -gno-record-gcc-switches -fno-common'
GOHOSTARCH='amd64'
GOHOSTOS='darwin'
GOINSECURE=''
GOMOD='/Users/emmanuelodeke/Desktop/openSrc/bugs/golang/73548/go.mod'
GOMODCACHE='/Users/emmanuelodeke/go/pkg/mod'
GONOPROXY=''
GONOSUMDB=''
GOOS='darwin'
GOPATH='/Users/emmanuelodeke/go'
GOPRIVATE=''
GOPROXY='https://proxy.golang.org,direct'
GOROOT='/Users/emmanuelodeke/go/src/go.googlesource.com/go'
GOSUMDB='sum.golang.org'
GOTELEMETRY='on'
GOTELEMETRYDIR='/Users/emmanuelodeke/Library/Application Support/go/telemetry'
GOTMPDIR=''
GOTOOLCHAIN='auto'
GOTOOLDIR='/Users/emmanuelodeke/go/src/go.googlesource.com/go/pkg/tool/darwin_amd64'
GOVCS=''
GOVERSION='go1.26-devel_ce0e803ab9 Mon Aug 11 11:12:55 2025 -0700'
GOWORK=''
PKG_CONFIG='pkg-config'

What did you do?

As a follow-up to #76721, the bug that I reported about File.ImportedSymbols seeking to virtual address lacks bounds checks before using VirtualAddress, after I apply the fixes that I proposed, we encounter another bug which is that when dealing with ImportDirectory decoding in this code section with this code, after we applied the prior fix and can run this code https://go.dev/play/p/xLsv4dfUnSH

package main

import (
	"bytes"
	"debug/pe"
)

func main() {
	br := bytes.NewReader([]byte("MZ0000000000000000000000000000000000000000000000000000000000\x10\x01\x00\x000000000000000000000000000000000000000000\x00\x000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000PE\x00\x00d\x86\b\x000000\x00\x00\x00\x000000\xf0\x0000\v\x020000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000\x10\x00\x00\x000000000020\x00\x000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000\x00\x0000000000000000000000000000000000000000\x00\x0000000000000000000000000000000000000000\x00\x0000000000000000000000000000000000000000\x00\x0000000000000000000000000000000000000000\x00\x0000000000000000000000\x00\x000\x02\x00\x00\"\x00\x00\x0000000000\x00\x0000000000000000000000000000000000000000\x00\x0000000000000000000000000000000000000000\x00\x00000000"))
	f, err := pe.NewFile(br)
	if err != nil {
		panic(err)
	}
	defer f.Close()
	f.ImportedSymbols()
}

What did you see happen?

panic: runtime error: slice bounds out of range [:8] with capacity 6

goroutine 1 [running]:
debug/pe.(*File).ImportedSymbols(0xc696d48?)
	/Users/emmanuelodeke/go/src/go.googlesource.com/go/src/debug/pe/file.go:421 +0x9f9
main.main()
	/Users/emmanuelodeke/Desktop/openSrc/bugs/golang/repro/xLsv4dfUnSH.go:15 +0x10f
exit status 2

What did you expect to see?

No crash

Diagnosis

@@ -408,9 +412,15 @@ func (f *File) ImportedSymbols() ([]string, error) {
                 dt.dll, _ = getString(names, int(dt.Name-ds.VirtualAddress))
                 d, _ = ds.Data()
                 // seek to OriginalFirstThunk
+                if diff := dt.OriginalFirstThunk - ds.VirtualAddress; diff >= uint32(len(d)) {
+                       continue
+                }
                 d = d[dt.OriginalFirstThunk-ds.VirtualAddress:]
                 for len(d) > 0 {
                        if pe64 { // 64bit
+                               if len(d) < 8 {
+                                       return nil, errors.New("thunk parsing needs at least 8-bytes")
+                               }
                                va := binary.LittleEndian.Uint64(d[0:8])
                                d = d[8:]
                                if va == 0 {
@@ -423,6 +433,9 @@ func (f *File) ImportedSymbols() ([]string, error) {
                                        all = append(all, fn+":"+dt.dll)
                                }
                        } else { // 32bit
+                               if len(d) <= 4 {
+                                       return nil, errors.New("thunk parsing needs at least 5-bytes")
+                               }
                                va := binary.LittleEndian.Uint32(d[0:4])
                                d = d[4:]
                                if va == 0 {

Kindly cc-ing @alexbrainman

Metadata

Metadata

Assignees

No one assigned

    Labels

    BugReportIssues describing a possible bug in the Go implementation.

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions