fix(dotnet): bound PE resource parsing by what the file actually holds

Resource sizes and RVAs are user-controlled uint32s that sized buffers before anything
checked the file held that many bytes, so a small crafted section could reserve up to 4GB.

The walk now carries the section's reader and its base RVA for its whole lifetime, so
`reader.Size()` is the single authoritative bound and one `offsetOf` helper rejects every
RVA the section does not hold. That replaces the per-node section value the recursion used
to synthesize, whose size argument was only correct by convention.

Bounding the allocations alone turns the OOM into a hang, though: nothing in the format
stops entries from aliasing, so thousands of them may name one fat blob and every offset
still checks out. A 256KB section built that way drove 24GB of allocation over two minutes
with peak memory flat. Capping entry *count* does not help since the cost is per byte read,
so the walk now charges every read against a budget of a few times the section size. Real
binaries come in at about 1x, since a well-formed section's entries partition it.

A resource tree that stops early no longer fails the whole file. It says nothing about the
CLR directory or an embedded deps.json, and dropping the package from the SBOM over one
malformed section is worse than reporting the fields we did get.

Also widens the entry counts before summing them (two uint16s can sum past 0xFFFF and wrap),
and bounds the name length and section header count against the reader for the same reason.

Signed-off-by: Alex Goodman <wagoodman@users.noreply.github.com>

syft/pkg/cataloger/internal/dotnet/pe/pe.go

@@ -9,7 +9,6 @@ import (
 	"io"
 	"unicode/utf16"
 
-	"github.com/scylladb/go-set/strset"
 	"github.com/scylladb/go-set/u32set"
 
 	"github.com/anchore/syft/internal/log"
@@ -17,7 +16,87 @@ import (
 	"github.com/anchore/syft/syft/internal/unionreader"
 )
 
-const peMaxAllowedDirectoryEntries = 0x1000
+const (
+	peMaxAllowedDirectoryEntries = 0x1000
+
+	// peResourceBudgetFactor bounds the total bytes a walk will read out of a resource section, as a
+	// multiple of that section's own size. The per-directory cap above only bounds the fan-out of one
+	// node, and nothing in the format stops entries from aliasing: thousands of them may name the same
+	// blob, so a tree whose every individual offset is in bounds can still drive work quadratic in the
+	// section size. A well-formed section's entries partition it rather than overlapping, so real binaries
+	// come in right around 1x and the slack here only absorbs padding and shared string tables.
+	peResourceBudgetFactor = 4
+
+	// clrDebugInfoResourceName is the only resource name any downstream logic asks about.
+	clrDebugInfoResourceName = "CLRDEBUGINFO"
+)
+
+// resourceWalk is the state shared across a single resource directory traversal.
+//
+// reader and baseRVA are fixed for the whole walk: every RVA a nested entry names resolves against the
+// same origin and the same bytes, so reader.Size() is the one authoritative bound on every offset derived
+// from them. Holding them here rather than on a per-node value is what keeps that invariant structural.
+type resourceWalk struct {
+	reader  *bytes.Reader
+	baseRVA uint32
+
+	// dirs tracks the RVAs already parsed (prevents infinite recursion edge cases)
+	dirs *u32set.Set
+
+	// fields collects version resource keys and their values
+	fields map[string]string
+
+	// hasCLRDebugInfo records whether a CLRDEBUGINFO resource name was seen
+	hasCLRDebugInfo bool
+
+	// budget is the number of bytes left that we are willing to read out of the section. Charging every
+	// read against one counter bounds the blobs, the names, and the tree walk itself together, and makes
+	// recursion depth fall out for free since each level costs at least a directory header.
+	budget int64
+}
+
+func newResourceWalk() *resourceWalk {
+	return &resourceWalk{
+		dirs:   u32set.New(),
+		fields: make(map[string]string),
+	}
+}
+
+// bind points the walk at a resource section's bytes and fixes the origin every RVA is measured from.
+func (w *resourceWalk) bind(reader *bytes.Reader, baseRVA uint32) {
+	w.reader = reader
+	w.baseRVA = baseRVA
+	w.budget = reader.Size() * peResourceBudgetFactor
+}
+
+// offsetOf turns an RVA into an offset into the walk's bytes, rejecting any RVA the section does not
+// actually hold. RVAs are user-controlled uint32s, so this is what keeps the subtraction from underflowing
+// and keeps every offset derived from one inside the buffer.
+func (w *resourceWalk) offsetOf(rva uint32) (int64, error) {
+	if rva < w.baseRVA {
+		return 0, fmt.Errorf("RVA=0x%x precedes its section base 0x%x", rva, w.baseRVA)
+	}
+
+	offset := int64(rva - w.baseRVA)
+	if offset >= w.reader.Size() {
+		return 0, fmt.Errorf("RVA=0x%x lies past its section end (baseRVA=0x%x size=0x%x)", rva, w.baseRVA, w.reader.Size())
+	}
+
+	return offset, nil
+}
+
+// errResourceBudget stops the walk rather than one entry: once the budget is gone every remaining sibling
+// would hit it too, so callers that normally log-and-continue have to propagate this one.
+var errResourceBudget = errors.New("resource walk read more of its section than a well-formed one could justify")
+
+// spend charges n bytes about to be read out of the section against the walk's budget.
+func (w *resourceWalk) spend(n int64) error {
+	w.budget -= n
+	if w.budget < 0 {
+		return errResourceBudget
+	}
+	return nil
+}
 
 var imageDirectoryEntryIndexes = []int{
 	pe.IMAGE_DIRECTORY_ENTRY_RESOURCE,       // where version resources are stored
@@ -162,15 +241,15 @@ func Read(f file.LocationReadCloser) (*File, error) {
 		return nil, fmt.Errorf("unable to parse PE sections: %w", err)
 	}
 
-	dirs := u32set.New()                        // keep track of the RVAs we have already parsed (prevent infinite recursion edge cases)
-	versionResources := make(map[string]string) // map of version resource keys to their values
-	resourceNames := strset.New()               // set of resource names found in the PE file
-	err = parseResourceDirectory(sections[pe.IMAGE_DIRECTORY_ENTRY_RESOURCE], dirs, versionResources, resourceNames)
-	if err != nil {
-		return nil, err
+	walk := newResourceWalk()
+	if err := parseResourceDirectory(sections[pe.IMAGE_DIRECTORY_ENTRY_RESOURCE], walk); err != nil {
+		// a resource tree that stops early still tells us about the fields it did yield, and says nothing
+		// about the CLR directory or an embedded deps.json. Failing the whole file here would drop the
+		// package from the SBOM entirely over one malformed section.
+		log.Tracef("unable to fully parse PE resource directory for %s: %v", f.RealPath, err)
 	}
 
-	c, err := parseCLR(sections[pe.IMAGE_DIRECTORY_ENTRY_COM_DESCRIPTOR], resourceNames)
+	c, err := parseCLR(sections[pe.IMAGE_DIRECTORY_ENTRY_COM_DESCRIPTOR], walk.hasCLRDebugInfo)
 	if err != nil {
 		return nil, fmt.Errorf("unable to parse PE CLR directory: %w", err)
 	}
@@ -184,7 +263,7 @@ func Read(f file.LocationReadCloser) (*File, error) {
 		Location:         f.Location,
 		CLR:              c,
 		EmbeddedDepsJSON: embeddedDepsJSON,
-		VersionResources: versionResources,
+		VersionResources: walk.fields,
 	}, nil
 }
 
@@ -297,12 +376,15 @@ func parseSectionHeaders(file unionreader.UnionReader, magic uint16, numberOfSec
 		return nil, nil, fmt.Errorf("unknown optional header magic: 0x%x", magic)
 	}
 
-	// read section headers
-	headers := make([]pe.SectionHeader32, numberOfSections)
-	for i := 0; i < int(numberOfSections); i++ {
-		if err := binary.Read(file, binary.LittleEndian, &headers[i]); err != nil {
+	// read section headers. numberOfSections is a uint16 straight out of the file header, so the slice
+	// grows to the headers that are actually there rather than reserving for all 65535 up front.
+	var headers []pe.SectionHeader32
+	for range numberOfSections {
+		var header pe.SectionHeader32
+		if err := binary.Read(file, binary.LittleEndian, &header); err != nil {
 			return nil, nil, fmt.Errorf("error reading section header: %w", err)
 		}
+		headers = append(headers, header)
 	}
 
 	return soi, headers, nil
@@ -310,8 +392,7 @@ func parseSectionHeaders(file unionreader.UnionReader, magic uint16, numberOfSec
 
 // parseCLR extracts the CLR (common language runtime) version information from the COM descriptor and makes
 // present/not-present determination based on the presence of CLR resource names.
-func parseCLR(sec *extractedSection, resourceNames *strset.Set) (*CLREvidence, error) {
-	hasCLRDebugResourceNames := resourceNames.HasAny("CLRDEBUGINFO")
+func parseCLR(sec *extractedSection, hasCLRDebugResourceNames bool) (*CLREvidence, error) {
 	if sec == nil || sec.Reader == nil {
 		return &CLREvidence{
 			HasClrResourceNames: hasCLRDebugResourceNames,
@@ -352,10 +433,23 @@ func readDataFromRVA(file io.ReadSeeker, rva, size uint32, sections []pe.Section
 		return nil, err
 	}
 
+	// size is a user-controlled uint32, so sizing the buffer from it alone lets a small file reserve up to
+	// 4GB. Clamping to the bytes that actually remain keeps the allocation exact in one shot, which an
+	// append-growing read cannot do: it holds both arrays at its final growth, so a legitimate 150MB
+	// bundle would cost well over twice its own size to scan.
+	end, err := file.Seek(0, io.SeekEnd)
+	if err != nil {
+		return nil, fmt.Errorf("error measuring file: %w", err)
+	}
+
 	if _, err := file.Seek(int64(offset), io.SeekStart); err != nil {
 		return nil, fmt.Errorf("error seeking to data: %w", err)
 	}
 
+	if remaining := end - int64(offset); remaining < int64(size) {
+		return nil, fmt.Errorf("error reading data: %d bytes declared at offset %d but only %d remain", size, offset, max(remaining, 0))
+	}
+
 	data := make([]byte, size)
 	if _, err := io.ReadFull(file, data); err != nil {
 		return nil, fmt.Errorf("error reading data: %w", err)
@@ -388,7 +482,7 @@ func readDataFromRVA(file io.ReadSeeker, rva, size uint32, sections []pe.Section
 // sources:
 // - https://learn.microsoft.com/en-us/windows/win32/debug/pe-format#the-rsrc-section
 // - https://learn.microsoft.com/en-us/previous-versions/ms809762(v=msdn.10)#pe-file-resources
-func parseResourceDirectory(sec *extractedSection, dirs *u32set.Set, fields map[string]string, names *strset.Set) error {
+func parseResourceDirectory(sec *extractedSection, w *resourceWalk) error {
 	if sec == nil || sec.Size <= 0 {
 		return nil
 	}
@@ -402,40 +496,63 @@ func parseResourceDirectory(sec *extractedSection, dirs *u32set.Set, fields map[
 		baseRVA = sec.RVA
 	}
 
-	offset := int64(sec.RVA - baseRVA)
-	if _, err := sec.Reader.Seek(offset, io.SeekStart); err != nil {
+	w.bind(sec.Reader, baseRVA)
+
+	return parseResourceDirectoryAt(sec.RVA, w)
+}
+
+func parseResourceDirectoryAt(rva uint32, w *resourceWalk) error {
+	offset, err := w.offsetOf(rva)
+	if err != nil {
+		return fmt.Errorf("resource directory: %w", err)
+	}
+
+	if _, err := w.reader.Seek(offset, io.SeekStart); err != nil {
 		return fmt.Errorf("error seeking to directory offset: %w", err)
 	}
 
 	var directoryHeader peImageResourceDirectory
-	if err := readIntoStruct(sec.Reader, &directoryHeader); err != nil {
+	if err := w.spend(int64(binary.Size(directoryHeader))); err != nil {
+		return err
+	}
+
+	if err := readIntoStruct(w.reader, &directoryHeader); err != nil {
 		return fmt.Errorf("error reading directory header: %w", err)
 	}
 
-	numEntries := int(directoryHeader.NumberOfNamedEntries + directoryHeader.NumberOfIDEntries)
+	// widen before adding: the two counts are uint16s that a crafted file can make sum past 0xFFFF,
+	// which would wrap and hide entries a real loader would still walk
+	numEntries := int(directoryHeader.NumberOfNamedEntries) + int(directoryHeader.NumberOfIDEntries)
 	switch {
 	case numEntries > peMaxAllowedDirectoryEntries:
 		return fmt.Errorf("too many entries in resource directory: %d", numEntries)
 	case numEntries == 0:
 		return fmt.Errorf("no entries in resource directory")
-	case numEntries < 0:
-		return fmt.Errorf("invalid number of entries in resource directory: %d", numEntries)
 	}
 
 	for i := range numEntries {
 		var entry peImageResourceDirectoryEntry
 
+		if err := w.spend(int64(binary.Size(entry))); err != nil {
+			return err
+		}
+
 		entryOffset := offset + int64(binary.Size(directoryHeader)) + int64(i*binary.Size(entry))
-		if _, err := sec.Reader.Seek(entryOffset, io.SeekStart); err != nil {
+		if _, err := w.reader.Seek(entryOffset, io.SeekStart); err != nil {
 			log.Tracef("error seeking to PE entry offset: %v", err)
 			continue
 		}
 
-		if err := readIntoStruct(sec.Reader, &entry); err != nil {
+		if err := readIntoStruct(w.reader, &entry); err != nil {
 			continue
 		}
 
-		if err := processResourceEntry(entry, baseRVA, sec, dirs, fields, names); err != nil {
+		if err := processResourceEntry(entry, w); err != nil {
+			// a budget exhausted partway down the tree is not a property of this one entry, so stop the
+			// walk rather than letting every sibling re-discover it
+			if errors.Is(err, errResourceBudget) {
+				return err
+			}
 			log.Tracef("error processing resource entry: %v", err)
 			continue
 		}
@@ -444,7 +561,7 @@ func parseResourceDirectory(sec *extractedSection, dirs *u32set.Set, fields map[
 	return nil
 }
 
-func processResourceEntry(entry peImageResourceDirectoryEntry, baseRVA uint32, sec *extractedSection, dirs *u32set.Set, fields map[string]string, names *strset.Set) error {
+func processResourceEntry(entry peImageResourceDirectoryEntry, w *resourceWalk) error {
 	// if the high bit is set, this is a directory entry, otherwise it is a data entry
 	isDirectory := entry.OffsetToData&0x80000000 != 0
 
@@ -456,75 +573,90 @@ func processResourceEntry(entry peImageResourceDirectoryEntry, baseRVA uint32, s
 
 	// read the string name of the resource directory
 	if nameIsString {
-		currentPos, err := sec.Reader.Seek(0, io.SeekCurrent)
+		currentPos, err := w.reader.Seek(0, io.SeekCurrent)
 		if err != nil {
 			return fmt.Errorf("error getting current reader position: %w", err)
 		}
 
-		if _, err := sec.Reader.Seek(int64(nameOffset), io.SeekStart); err != nil {
-			return fmt.Errorf("error restoring reader position: %w", err)
+		if _, err := w.reader.Seek(int64(nameOffset), io.SeekStart); err != nil {
+			return fmt.Errorf("error seeking to resource name: %w", err)
 		}
 
-		name, err := readUTF16WithLength(sec.Reader)
-		if err == nil {
-			names.Add(name)
+		// only one name matters downstream, so compare in place rather than retaining every name a
+		// crafted file cares to declare
+		name, err := readUTF16WithLength(w.reader, w)
+		switch {
+		case errors.Is(err, errResourceBudget):
+			return err
+		case err == nil && name == clrDebugInfoResourceName:
+			w.hasCLRDebugInfo = true
 		}
 
-		if _, err := sec.Reader.Seek(currentPos, io.SeekStart); err != nil {
+		if _, err := w.reader.Seek(currentPos, io.SeekStart); err != nil {
 			return fmt.Errorf("error restoring reader position: %w", err)
 		}
 	}
 
+	targetRVA := w.baseRVA + entryOffsetToData
+
 	if isDirectory {
-		subRVA := baseRVA + entryOffsetToData
-		if dirs.Has(subRVA) {
+		if w.dirs.Has(targetRVA) {
 			// some malware uses recursive PE references to evade analysis
-			return fmt.Errorf("recursive PE reference detected; skipping directory at baseRVA=0x%x subRVA=0x%x", baseRVA, subRVA)
+			return fmt.Errorf("recursive PE reference detected; skipping directory at baseRVA=0x%x subRVA=0x%x", w.baseRVA, targetRVA)
 		}
 
-		dirs.Add(subRVA)
-		err := parseResourceDirectory(
-			&extractedSection{
-				RVA:     subRVA,
-				BaseRVA: baseRVA,
-				Size:    sec.Size - (sec.RVA - baseRVA),
-				Reader:  sec.Reader,
-			},
-			dirs, fields, names)
+		w.dirs.Add(targetRVA)
+
+		return parseResourceDirectoryAt(targetRVA, w)
+	}
+
+	return parseResourceDataEntry(targetRVA, w)
+}
+
+func parseResourceDataEntry(rva uint32, w *resourceWalk) error {
+	offset, err := w.offsetOf(rva)
 	if err != nil {
-			return err
-		}
-		return nil
-	}
-	return parseResourceDataEntry(sec.Reader, baseRVA, baseRVA+entryOffsetToData, sec.Size, fields)
+		return fmt.Errorf("resource data entry: %w", err)
 	}
 
-func parseResourceDataEntry(reader *bytes.Reader, baseRVA, rva, remainingSize uint32, fields map[string]string) error {
-	var dataEntry peImageResourceDataEntry
-	offset := int64(rva - baseRVA)
-
-	if _, err := reader.Seek(offset, io.SeekStart); err != nil {
+	if _, err := w.reader.Seek(offset, io.SeekStart); err != nil {
 		return fmt.Errorf("error seeking to data entry offset: %w", err)
 	}
 
-	if err := readIntoStruct(reader, &dataEntry); err != nil {
+	var dataEntry peImageResourceDataEntry
+	if err := w.spend(int64(binary.Size(dataEntry))); err != nil {
+		return err
+	}
+
+	if err := readIntoStruct(w.reader, &dataEntry); err != nil {
 		return fmt.Errorf("error reading resource data entry: %w", err)
 	}
 
-	if remainingSize < dataEntry.Size {
-		return fmt.Errorf("resource data entry size exceeds remaining size")
+	// OffsetToData and Size are both user-controlled uint32s, so the region they describe has to be bounded
+	// against the bytes the section actually holds before it sizes the allocation below.
+	dataOffset, err := w.offsetOf(dataEntry.OffsetToData)
+	if err != nil {
+		return fmt.Errorf("resource data: %w", err)
+	}
+
+	if int64(dataEntry.Size) > w.reader.Size()-dataOffset {
+		return fmt.Errorf("resource data (offset=0x%x size=0x%x) extends past its section end 0x%x", dataOffset, dataEntry.Size, w.reader.Size())
+	}
+
+	if err := w.spend(int64(dataEntry.Size)); err != nil {
+		return err
 	}
 
 	data := make([]byte, dataEntry.Size)
-	if _, err := reader.Seek(int64(dataEntry.OffsetToData-baseRVA), io.SeekStart); err != nil {
+	if _, err := w.reader.Seek(dataOffset, io.SeekStart); err != nil {
 		return fmt.Errorf("error seeking to resource data: %w", err)
 	}
 
-	if _, err := reader.Read(data); err != nil {
+	if _, err := io.ReadFull(w.reader, data); err != nil {
 		return fmt.Errorf("error reading resource data: %w", err)
 	}
 
-	return parseVersionResourceSection(bytes.NewReader(data), fields)
+	return parseVersionResourceSection(bytes.NewReader(data), w.fields)
 }
 
 // parseVersionResourceSection parses a PE version resource section from within a resource directory.
@@ -697,8 +829,8 @@ func isTruncated(err error) bool {
 
 // readIntoStruct reads a struct from the reader and updates the offsets if provided.
 //
-// note: a truncated read must stay an error. Callers advance their loop counters by the offsets updated
-// below, so reporting a zeroed struct as a successful read leaves length-driven loops spinning forever.
+// note: EOF must stay an error. Callers advance their loop counters by the offsets updated below, so
+// reporting a zeroed struct as a successful read leaves length-driven loops spinning forever.
 func readIntoStruct[T any](reader io.Reader, data *T, offsets ...*int) error {
 	if err := binary.Read(reader, binary.LittleEndian, data); err != nil {
 		return err
@@ -757,7 +889,7 @@ func readUTF16(reader *bytes.Reader, offsets ...*int) string {
 
 // readUTF16WithLength reads a length-prefixed UTF-16 string from reader.
 // The first 2 bytes represent the number of UTF-16 code units.
-func readUTF16WithLength(reader *bytes.Reader) (string, error) {
+func readUTF16WithLength(reader *bytes.Reader, w *resourceWalk) (string, error) {
 	var length uint16
 	if err := binary.Read(reader, binary.LittleEndian, &length); err != nil {
 		return "", err
@@ -766,6 +898,17 @@ func readUTF16WithLength(reader *bytes.Reader) (string, error) {
 		return "", nil
 	}
 
+	// length is a user-controlled uint16 and binary.Read allocates a second buffer of its own, so a name
+	// the reader cannot satisfy must be rejected before either one is sized from it
+	size := int64(length) * 2
+	if size > int64(reader.Len()) {
+		return "", fmt.Errorf("declared name length %d exceeds the %d bytes remaining", length, reader.Len())
+	}
+
+	if err := w.spend(size); err != nil {
+		return "", err
+	}
+
 	// read length UTF-16 code units.
 	codes := make([]uint16, length)
 	if err := binary.Read(reader, binary.LittleEndian, &codes); err != nil {

syft/pkg/cataloger/internal/dotnet/pe/pe_resource_test.go

@@ -3,6 +3,7 @@ package pe
 import (
 	"bytes"
 	"encoding/binary"
+	"runtime"
 	"testing"
 	"time"
 
@@ -10,6 +11,182 @@ import (
 	"github.com/stretchr/testify/require"
 )
 
+// putResourceDir writes a resource directory header declaring a single ID entry, followed by that
+// entry pointing at offsetToData. isDir sets the high bit, which marks the target as a subdirectory.
+func putResourceDir(buf []byte, at int, offsetToData uint32, isDir bool) {
+	le := binary.LittleEndian
+	le.PutUint16(buf[at+12:], 0) // NumberOfNamedEntries
+	le.PutUint16(buf[at+14:], 1) // NumberOfIDEntries
+
+	entry := at + 16
+	le.PutUint32(buf[entry:], 1) // Name (ID, not a string)
+	if isDir {
+		offsetToData |= 0x80000000
+	}
+	le.PutUint32(buf[entry+4:], offsetToData)
+}
+
+const (
+	testSectionRVA  = 0x1000
+	testSectionSize = 0x400
+)
+
+// putResourceDirN is putResourceDir for directories with more than one entry, all of them pointing at the
+// same target. Aliasing like this is what the format permits and no real toolchain emits.
+func putResourceDirN(buf []byte, at, n int, offsetToData uint32, isDir bool) {
+	le := binary.LittleEndian
+	le.PutUint16(buf[at+12:], 0)         // NumberOfNamedEntries
+	le.PutUint16(buf[at+14:], uint16(n)) // NumberOfIDEntries
+
+	if isDir {
+		offsetToData |= 0x80000000
+	}
+	for i := range n {
+		entry := at + 16 + i*8
+		le.PutUint32(buf[entry:], uint32(i)) // Name (distinct IDs, not strings)
+		le.PutUint32(buf[entry+4:], offsetToData)
+	}
+}
+
+// boundWalk returns a walk bound to data as its resource section, as parseResourceDirectory would.
+func boundWalk(data []byte) *resourceWalk {
+	w := newResourceWalk()
+	w.bind(bytes.NewReader(data), testSectionRVA)
+	return w
+}
+
+// measureAlloc reports the bytes allocated while fn runs. The property these guards exist for is "a small
+// section cannot make us reserve a large buffer", and only a byte count states that: asserting an error
+// comes back would keep passing if the allocation were hoisted above the check.
+func measureAlloc(t *testing.T, fn func()) uint64 {
+	t.Helper()
+
+	var before, after runtime.MemStats
+	runtime.GC()
+	runtime.ReadMemStats(&before)
+	fn()
+	runtime.ReadMemStats(&after)
+
+	return after.TotalAlloc - before.TotalAlloc
+}
+
+func TestParseResourceDataEntry_SizePastSectionIsRejected(t *testing.T) {
+	// OffsetToData and Size are user-controlled uint32s. A data entry may only describe bytes its section
+	// actually holds, otherwise Size drives the allocation.
+	buf := make([]byte, testSectionSize)
+	le := binary.LittleEndian
+	le.PutUint32(buf[0x100:], testSectionRVA) // OffsetToData, resolves to section offset 0
+	le.PutUint32(buf[0x104:], 256*1024*1024)  // a size far past the 1KB section
+
+	w := boundWalk(buf)
+
+	var err error
+	allocated := measureAlloc(t, func() {
+		err = parseResourceDataEntry(testSectionRVA+0x100, w)
+	})
+
+	require.ErrorContains(t, err, "extends past its section end")
+	assert.Less(t, allocated, uint64(1<<20),
+		"the declared 256MB must never be reserved, so the guard has to run before the allocation")
+}
+
+func TestParseResourceDataEntry_OffsetBeforeSectionBaseIsRejected(t *testing.T) {
+	// OffsetToData is independent of baseRVA, so it can name an RVA before the section starts. The
+	// subtraction that turns it into a section offset would otherwise underflow to near 4GB.
+	buf := make([]byte, testSectionSize)
+	le := binary.LittleEndian
+	le.PutUint32(buf[0x100:], testSectionRVA-1) // OffsetToData, one byte before the section base
+	le.PutUint32(buf[0x104:], 16)
+
+	err := parseResourceDataEntry(testSectionRVA+0x100, boundWalk(buf))
+	require.ErrorContains(t, err, "precedes its section base")
+}
+
+func TestParseResourceDataEntry_EntryRVAPastSectionIsRejected(t *testing.T) {
+	// the entry's own RVA is as user-controlled as the data it points at
+	buf := make([]byte, testSectionSize)
+
+	err := parseResourceDataEntry(testSectionRVA+testSectionSize, boundWalk(buf))
+	require.ErrorContains(t, err, "lies past its section end")
+}
+
+func TestParseResourceDirectory_SubdirectoryPastSectionIsRejected(t *testing.T) {
+	// a subdirectory RVA is derived from baseRVA plus a user-controlled offset, so a child can name bytes
+	// past the section its parent lives in
+	buf := make([]byte, testSectionSize)
+	putResourceDir(buf, 0x000, testSectionSize, true) // root -> subdirectory at the section end
+
+	w := boundWalk(buf)
+	err := parseResourceDirectoryAt(testSectionRVA, w)
+
+	// the root loop logs and continues past a bad child, so the rejection shows up as an unvisited tree
+	require.NoError(t, err)
+	assert.Empty(t, w.fields)
+}
+
+func TestParseResourceDirectory_EntryCountsCannotWrap(t *testing.T) {
+	// NumberOfNamedEntries and NumberOfIDEntries are uint16s that a crafted file can make sum past 0xFFFF.
+	// Adding them at uint16 width would wrap to a small number and hide entries a real loader still walks.
+	buf := make([]byte, testSectionSize)
+	le := binary.LittleEndian
+	le.PutUint16(buf[12:], 0x8000)
+	le.PutUint16(buf[14:], 0x8001) // sums to 0x10001, which wraps to 1
+
+	err := parseResourceDirectoryAt(testSectionRVA, boundWalk(buf))
+	require.ErrorContains(t, err, "too many entries in resource directory")
+}
+
+func TestParseResourceDirectory_AliasedEntriesCannotAmplifyWork(t *testing.T) {
+	// nothing in the format stops thousands of entries from naming one fat blob, and every individual
+	// offset here is inside the section. Without a bound on total bytes read, a section this size drives
+	// tens of GB of allocation and minutes of parsing, so peak memory stays flat while the scan hangs.
+	const secSize = 256 * 1024
+	buf := make([]byte, secSize)
+	le := binary.LittleEndian
+
+	// root: 16 subdirectories at distinct RVAs, since aliased directories are already deduped
+	le.PutUint16(buf[14:], 16)
+	for i := range 16 {
+		entry := 16 + i*8
+		le.PutUint32(buf[entry:], uint32(i))
+		le.PutUint32(buf[entry+4:], uint32(0x1000+i*0x800)|0x80000000)
+	}
+
+	// each subdirectory: 4096 leaf entries, every one pointing at the same near-section-sized blob
+	for i := range 16 {
+		putResourceDirN(buf, 0x1000+i*0x800, 4096, 0x10000, false)
+	}
+	le.PutUint32(buf[0x10000:], testSectionRVA+0x20000)
+	le.PutUint32(buf[0x10004:], secSize-0x20000)
+
+	// make the blob a version resource, so each leaf that reaches it also pays for a full string-table walk
+	copy(buf[0x20000:], buildVersionResource(true))
+
+	w := newResourceWalk()
+	w.bind(bytes.NewReader(buf), testSectionRVA)
+
+	var err error
+	var timedOut bool
+	allocated := measureAlloc(t, func() {
+		done := make(chan error, 1)
+		go func() { done <- parseResourceDirectoryAt(testSectionRVA, w) }()
+		select {
+		case err = <-done:
+		case <-time.After(30 * time.Second):
+			timedOut = true
+		}
+	})
+
+	require.False(t, timedOut, "the walk did not terminate")
+	require.ErrorIs(t, err, errResourceBudget,
+		"the walk must stop once it has read more than a well-formed section could justify")
+	// the point is the asymptote, not the constant: each byte the budget allows may still be copied into a
+	// blob buffer and walked, so a small multiple of the section is expected. Before the bound this same
+	// input allocated about 24GB, so anything proportional is three orders of magnitude away from the bug.
+	assert.Less(t, allocated, uint64(32*secSize),
+		"total work must stay proportional to the section, not to the entries that alias into it")
+}
+
 // buildVersionResource returns a well-formed VS_VERSION_INFO blob whose last child is a VarFileInfo
 // block, which is the layout real toolchains emit. It ends exactly on a struct boundary. When
 // withFileVersionString is set, a StringFileInfo table supplies FileVersion directly.

syft/pkg/cataloger/internal/dotnet/pe/pe_rva_test.go

@@ -0,0 +1,59 @@
+package pe
+
+import (
+	"bytes"
+	"debug/pe"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+func TestReadDataFromRVA_BogusSizeDoesNotOverAllocate(t *testing.T) {
+	// size comes from the PE headers and spans the full uint32 range. Sizing the buffer from it up front
+	// let a tiny file reserve 4GB; the read must instead grow to what the file holds and then report the
+	// shortfall rather than returning a mostly-zero buffer as if it had been filled.
+	const fileSize = 512
+	sections := []pe.SectionHeader32{{VirtualAddress: 0x1000, VirtualSize: 0x1000, PointerToRawData: 0}}
+	r := &readSizeRecorder{Reader: bytes.NewReader(make([]byte, fileSize))}
+
+	var err error
+	allocated := measureAlloc(t, func() {
+		_, err = readDataFromRVA(r, 0x1000, 0xFFFFFFFF, sections)
+	})
+
+	require.Error(t, err, "a size the file cannot satisfy must not be reported as a successful read")
+	assert.Less(t, allocated, uint64(1<<20),
+		"the declared 4GB must never be reserved; the shortfall has to be caught before the allocation")
+	assert.LessOrEqual(t, r.maxRead, fileSize,
+		"no single read may exceed the file size, regardless of what the headers claim")
+}
+
+func TestReadDataFromRVA_ReadsFullyWhenSizeFits(t *testing.T) {
+	// the bound must not truncate a legitimate read
+	sections := []pe.SectionHeader32{{VirtualAddress: 0x1000, VirtualSize: 0x1000, PointerToRawData: 0}}
+	want := bytes.Repeat([]byte("z"), 128)
+
+	got, err := readDataFromRVA(bytes.NewReader(want), 0x1000, uint32(len(want)), sections)
+	require.NoError(t, err)
+	assert.Equal(t, int64(len(want)), got.Size())
+}
+
+func TestReadDataFromRVA_StopsAtSizeInALargerFile(t *testing.T) {
+	// a file exactly `size` bytes long cannot distinguish a bounded read from an unbounded one, since
+	// io.ReadAll stops at EOF either way. Only a file with bytes past `size` pins the limit.
+	sections := []pe.SectionHeader32{{VirtualAddress: 0x1000, VirtualSize: 0x2000, PointerToRawData: 0}}
+	const size = 128
+	file := append(bytes.Repeat([]byte("z"), size), bytes.Repeat([]byte("!"), 4096)...)
+	r := &readSizeRecorder{Reader: bytes.NewReader(file)}
+
+	got, err := readDataFromRVA(r, 0x1000, size, sections)
+	require.NoError(t, err)
+
+	data := make([]byte, got.Size())
+	_, err = got.ReadAt(data, 0)
+	require.NoError(t, err)
+	assert.Equal(t, bytes.Repeat([]byte("z"), size), data,
+		"the read must stop at size rather than running on into the rest of the file")
+	assert.LessOrEqual(t, r.maxRead, size, "no read may request more than the declared size")
+}