fix: harden .NET PE/ELF parsing against malformed binaries (#5267)

* fix(dotnet): bound the bundle signature search by the file, not the headers

The single-file bundle marker search sized its buffer from `calculatePEEndOffset` /
`calculateELFEndOffset`, both of which sum user-controlled header fields. A tiny file
declaring 4GB sections drove an 8.6GB allocation, and on the ELF side the sum could
overflow negative and panic in `make`.

The search now clamps its window to the real file length before sizing anything. The PE
and ELF paths had grown two copies of the same find-and-read sequence, so they collapse
into one `bundle.ExtractDepsJSON`, which lets the signature search and the header reader
both go unexported.

Also guards `read7BitEncodedInt` against returning a negative length, which on a 32-bit
`int` would seek a manifest walk backwards and re-read the same bytes per declared file.

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

* fix(dotnet): stop a truncated version resource from spinning forever

`readIntoStruct` reported a truncated read as success and left the destination struct
zeroed. Its callers advance their loop counters by the offsets it updates, so the string
table loop consumed nothing, advanced nothing, and never reached its length bound: a
version resource whose header claims more content than the blob carries hung the parse.

A truncated read is now an error, and the two places where hitting the end of a blob is
normal say so explicitly. That distinction is what keeps a well-formed resource whose last
child is `VarFileInfo` from losing its `FileVersion` fallback. io.EOF and
io.ErrUnexpectedEOF mean the same thing here, so both take the same path.

The string table walk moves into its own function, since the two new arms put
`parseVersionResourceSection` over the funlen limit.

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

* 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>

* fix(dotnet): bound resource recursion depth and cap declared sizes absolutely

Review follow-ups on this branch. The headline one is a real crash: the resource
walk had no depth bound, and the `budget` comment claiming depth "falls out for
free" was wrong, since the budget scales with the section while the stack does
not. A chain of directories at distinct RVAs (which the `dirs` set does not
catch) recursed until the goroutine stack gave out. A 13.7MB crafted PE took the
whole process down with `fatal error: stack overflow`, which is unrecoverable,
so unlike every other bug on this branch no caller could log-and-continue past
it. Now capped at 32 levels, versus the 3 the format actually uses.

The other bounds moved from "the file" to a real ceiling. Clamping an allocation
to the bytes remaining in the file is not enough on its own: a mostly empty file
costs almost nothing inside a compressed layer, so a cheap artifact could still
authorize a multi-gigabyte read. Both the PE data directories and the bundle
marker search now cap the same way `elfutil` already does.

Also hardened in the bundle reader:

- `headerOffset` is validated against the file. It comes straight out of the
  file bytes and everything downstream seeks to it, so it was the least
  trustworthy value in there and the only one not being checked.
- a manifest entry count that could not fit in the remaining bytes is rejected
  rather than walked for hundreds of millions of reads.
- a non-positive `searchLimit` falls back to the whole file instead of reading
  nothing. It is only ever an optimization hint, so treating it as authoritative
  let one bogus ELF `p_offset` hide a bundle from us entirely.
- a short read no longer aborts the search. `unionreader` can return fewer bytes
  than the file reports, and the marker may well be in what we did get.

`pe.Read` now reports partial parses instead of choosing between dropping the
package and hiding the problem. It was inconsistent: a malformed resource tree
was non-fatal while an unreadable resource *section* one call away was fatal, and
the non-fatal path only reached a trace log, so the unknowns task could not see
that a file had been cataloged on incomplete evidence. Both now land on a new
`File.ParseErr`, which the dotnet and PE catalogers join into their unknowns and
still keep the package.

Worth noting for changelog purposes: the `isTruncated` guard fixes a
user-visible bug beyond the hardening. A version resource leaving 1 to 5
trailing bytes cuts the next struct in half, which reports as
`io.ErrUnexpectedEOF` rather than `io.EOF`; that used to propagate out and skip
the `VS_FIXEDFILEINFO` fallback, so those binaries lost their version entirely.

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

* fix(dotnet): treat a zero bundle offset as unbundled, not malformed

The apphost ships the 32-byte bundle signature compiled in with its 8-byte header
offset left as zero, and only `HostWriter.SetAsBundle` fills that slot in when you
publish as a single file. So a zero offset means "never bundled", which is what
every framework-dependent .NET executable looks like.

The bounds work on this branch folded that case in with the hostile ones, so the
most common shape a .NET binary takes started reporting a parse failure. It
surfaces through the PE cataloger rather than the dotnet one, since an apphost
carries no CLR metadata of its own: the package still comes out, with a spurious
unknown attached to it. `main` had this right and said so in a comment; the
regression is on this branch.

Reproduces on a fixture already in the repo. Of the 53 PE files in
`image-net8-app`, `/app/dotnetapp.exe` was the only one reporting anything, and
the whole image is clean again now.

The three outcomes are now distinct, since collapsing any two of them loses
something the caller needs: zero means there is no bundle, an error means we could
not tell, and a positive offset is the answer.

`Test_Read_DotNetDetection` gains the apphost and, more usefully, an assertion
that a well-formed binary reports no partial parse at all. That one line is what
would have caught this, and it now covers all six fixtures. The unit case that
pinned the old behavior asserted the bug, so it flips.

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

* fix(dotnet): measure readers with the shared helper and report refusals

Two things, both about not re-deriving what the campaign already has.

`internal/file.ReaderSize` exists on `main` for exactly this job, and `upx.go`
already uses it. Its doc comment is the problem statement here almost verbatim: a
declared size is worth nothing until you weigh it against a byte count the reader
can actually back, which it does by reading the last byte. This branch was
hand-rolling `Seek(0, io.SeekEnd)` in three places, one of them reimplementing the
helper's save-and-restore dance verbatim, and then absorbing the reader-lies case
downstream by tolerating short reads.

Adopting it means the bundle reader and `readDataFromRVA` now take the reader
types they were always being handed (`unionreader.UnionReader` and `io.ReaderAt`),
and the reads move to `ReadAt`, which leaves the caller's cursor alone and drops
two seeks. Net effect is a smaller diff than what it replaces, and the case the
`shortReader` fake was invented for is now caught up front instead of tolerated:
the fake was modeling a lie about length, which is not what a short squashfs read
actually does. It now models the real shape (honest length, read stops early) and
a second reader covers the lie.

Three places also still returned "there is nothing here" when what they meant was
"I could not tell", which is the thing `File.ParseErr` was added to stop:

- an ELF that `elfutil` refused to parse read as "not an ELF". `ErrDeclaredSizeExceeded`
  is exported precisely so a refusal can be told apart from a broken file, and the
  caller upstream already turns an error into an unknown.
- a `deps.json` past the 3MB cap returned "". That is the file's entire dependency
  list, so this handed back an SBOM that looked complete and silently was not.
- the marker search clamped at 512MB and then reported no bundle. It cannot claim
  there is no bundle when it declined to look everywhere one could be, so a miss
  after clamping is now an error.

`readSizeRecorder` records `ReadAt` alongside `Read`, or the allocation-bound tests
would have quietly stopped measuring anything.

Also renames `findSignatureOffset` back to `findBundleHeaderOffset`, since it
returns a header offset and reading it as "no signature" is what the last bug was,
and fixes the stale `ExtractDepsJSONFromBundle` doc comment.

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

* test(dotnet): assert allocation sizes directly in the bundle search guards

The PE and ELF search guards asserted the largest read the reader was asked for.
That happened to track the allocation, since the whole buffer goes to ReadAt, but
nothing said so. Both now measure the bytes allocated with the same `measureAlloc`
helper the resource guards use, and keep the read-size check as a separate claim.

Also names the reason a reader can report a size it cannot back: `debug/elf` and
`debug/pe` hand out `io.NewSectionReader(r, 0, 1<<63-1)`, whose `Size()` is nominal
rather than measured, so answering a size is not on its own evidence the bytes are
there. Renames `lyingReader` to `unbackedSizeReader` and regroups the two reader
implementations, which had their methods interleaved.

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

---------

Signed-off-by: Alex Goodman <wagoodman@users.noreply.github.com>
This commit is contained in:
Alex Goodman
2026-09-11 21:25:10 +00:00
committed by GitHub
parent 1ca1fa73d3
commit d85c72a08e
13 changed files with 1794 additions and 211 deletions
@@ -36,5 +36,11 @@ func parsePE(_ context.Context, _ file.Resolver, _ *generic.Environment, reader
p := newPEPackage(f.VersionResources, f.Location)
// the file was usable but not fully parsed, which for this cataloger means the package name and version
// may have come from the filename rather than from version resources. Worth reporting alongside it.
if f.ParseErr != nil {
return []pkg.Package{p}, nil, unknown.New(reader, f.ParseErr)
}
return []pkg.Package{p}, nil, nil
}
@@ -503,6 +503,11 @@ func findPEFiles(resolver file.Resolver) ([]logicalPE, error, error) {
if ldpe == nil {
continue
}
if ldpe.ParseErr != nil {
// the file parsed well enough to catalog but not completely, so record what we could not read
// and still keep the package
unknownErr = unknown.Append(unknownErr, loc, ldpe.ParseErr)
}
peFiles = append(peFiles, *ldpe)
}
@@ -1,10 +1,39 @@
package bundle
import (
"bytes"
"encoding/binary"
"errors"
"fmt"
"io"
intFile "github.com/anchore/syft/internal/file"
"github.com/anchore/syft/syft/internal/unionreader"
)
const (
// maxBundleSearchSize bounds the bytes findSignatureOffset will hold at once while looking for the
// bundle marker.
//
// The marker sits inside the executable structure, so the search legitimately covers a whole
// single-file bundle, which routinely runs past 100MB and can reach a few hundred for an app that
// embeds sizable assets. Clamping to the file length alone is not enough: a mostly empty file costs
// almost nothing inside a compressed layer, so a small artifact can still authorize a multi-gigabyte
// allocation. The trade-off is that a bundle larger than this loses its deps.json rather than being
// cataloged, which is the correct direction to fail when the alternative is OOM-killing the scan.
// This mirrors maxDeclaredSectionSize in syft/internal/elfutil.
maxBundleSearchSize = 512 * intFile.MB
// maxDepsJSONSize bounds an embedded deps.json. These are dependency manifests, so real ones are
// measured in KB even for large applications.
maxDepsJSONSize = 3 * intFile.MB
// minManifestEntrySize is the smallest a single manifest entry can be: an 8 byte offset, an 8 byte
// size, a 1 byte file type, and at least 1 byte for the length-prefixed relative path.
minManifestEntrySize = 18
// minManifestEntrySizeV6 adds the 8 byte compressed size field that V6+ bundles carry.
minManifestEntrySizeV6 = minManifestEntrySize + 8
)
// dotNetBundleSignature is the SHA-256 hash of ".net core bundle" used to identify single-file bundles.
@@ -15,6 +44,93 @@ var dotNetBundleSignature = []byte{
0xee, 0x3b, 0x2d, 0xce, 0x24, 0xb3, 0x6a, 0xae,
}
// ExtractDepsJSON returns the deps.json embedded in the .NET single-file bundle in r, or "" if r carries no
// bundle marker.
//
// searchLimit is where the caller's format parsing says the executable structure ends, which is as far into
// the file as the marker can be. It is only ever an optimization: it comes from user-controlled header
// fields, so it may describe far more than the file holds or overflow negative. A limit that makes no sense
// falls back to searching the whole file rather than searching nothing, since treating it as authoritative
// would let one bogus header field hide a bundle from us entirely.
func ExtractDepsJSON(r unionreader.UnionReader, searchLimit int64) (string, error) {
headerOffset, err := findBundleHeaderOffset(r, searchLimit)
if err != nil || headerOffset == 0 {
return "", err
}
return readDepsJSONFromBundleHeader(r, headerOffset)
}
// findBundleHeaderOffset searches the start of r for the .NET single-file bundle signature and returns the
// bundle header offset stored in the 8 bytes immediately before it.
//
// Three outcomes are distinct on purpose, because collapsing any two of them loses information the caller
// needs: 0 means there is no bundle here, an error means we could not tell, and a positive offset is the
// answer. In particular the apphost ships the signature compiled in with a zero offset placeholder and only
// gets a real one written when it is published as a single file, so a zero offset is the ordinary
// framework-dependent executable rather than a malformed one.
func findBundleHeaderOffset(r unionreader.UnionReader, searchLimit int64) (int64, error) {
// the length has to be established before anything is sized against it, and it has to be a length the
// reader can actually back: ReaderSize confirms it by reading the last byte, which is what keeps a
// reader that over-reports (a squashfs block that decompresses short does exactly that) from being
// treated as authoritative below
size, ok := intFile.ReaderSize(r)
if !ok {
return 0, errors.New("unable to determine the file's size, so the bundle marker search cannot be bounded")
}
// a limit that overflowed negative or overshoots the file tells us nothing, so fall back to the file
// itself rather than trusting it; either way the absolute cap is what bounds the allocation
limit := size
if searchLimit > 0 && searchLimit < size {
limit = searchLimit
}
// clamping is recorded rather than just logged: if the marker then turns up missing we cannot claim
// there is no bundle, only that we declined to look everywhere it could have been
var clamped bool
if limit > maxBundleSearchSize {
limit = maxBundleSearchSize
clamped = true
}
// this scans a whole executable, routinely over 100MB for a single-file bundle, so the buffer is sized
// exactly once. An append-growing read holds both arrays at its final growth and would cost well over
// twice the file's own size for the same result.
searchData := make([]byte, limit)
// a short read is not fatal here: the marker may well be in what we did get, so search the bytes we
// actually hold. ReadAt reports a short read as io.EOF, and may report a full one that way too, so the
// count is what says how much there is to search.
n, err := r.ReadAt(searchData, 0)
if err != nil && !errors.Is(err, io.EOF) {
return 0, err
}
idx := bytes.Index(searchData[:n], dotNetBundleSignature)
if idx == -1 || idx < 8 {
if clamped {
return 0, fmt.Errorf("no bundle marker in the first %d bytes and the rest of the %d byte file was not searched", maxBundleSearchSize, size)
}
return 0, nil
}
headerOffset := int64(binary.LittleEndian.Uint64(searchData[idx-8 : idx]))
if headerOffset == 0 {
// the marker is compiled into every apphost; only publishing as a single file fills in the offset
return 0, nil
}
// the offset comes straight out of the file, so it is the least trustworthy value here: everything
// downstream seeks to it and reads structures from it
if headerOffset < 0 || headerOffset >= size {
return 0, fmt.Errorf("bundle header offset %d lies outside the file (%d bytes)", headerOffset, size)
}
return headerOffset, nil
}
// dotNetBundleHeader represents the fixed portion of the bundle header (version 1+)
type dotNetBundleHeader struct {
MajorVersion uint32
@@ -45,8 +161,8 @@ const (
dotNetFileTypeSymbols
)
// ReadDepsJSONFromBundleHeader parses the bundle header at the given offset and extracts deps.json content.
func ReadDepsJSONFromBundleHeader(r io.ReadSeeker, headerOffset int64) (string, error) {
// readDepsJSONFromBundleHeader parses the bundle header at the given offset and extracts deps.json content.
func readDepsJSONFromBundleHeader(r unionreader.UnionReader, headerOffset int64) (string, error) {
if _, err := r.Seek(headerOffset, io.SeekStart); err != nil {
return "", err
}
@@ -105,20 +221,33 @@ func read7BitEncodedInt(r io.Reader) (int, error) {
return 0, errors.New("invalid 7-bit encoded int")
}
}
// the shift above can carry past int32 where int is 32 bits, and a negative length would seek callers
// backwards and let a manifest walk re-read the same bytes for every file it claims
if result < 0 {
return 0, errors.New("negative 7-bit encoded int")
}
return result, nil
}
// readDepsJSONAtOffset reads deps.json content at a specific offset using seeks (avoiding loading entire file)
func readDepsJSONAtOffset(r io.ReadSeeker, offset, size int64) (string, error) {
if size <= 0 || size > 3*1024*1024 { // 3MB max deps.json size in dotnet bundle
func readDepsJSONAtOffset(r unionreader.UnionReader, offset, size int64) (string, error) {
if size <= 0 {
return "", nil
}
if _, err := r.Seek(offset, io.SeekStart); err != nil {
return "", fmt.Errorf("failed to seek to deps.json at offset %d: %w", offset, err)
// an oversized deps.json is reported rather than dropped: this is the file's whole dependency list, so
// returning "" here would hand back an SBOM that looks complete and silently is not
if size > maxDepsJSONSize {
return "", fmt.Errorf("embedded deps.json of %d bytes is past the %d byte limit", size, maxDepsJSONSize)
}
data := make([]byte, size)
if _, err := io.ReadFull(r, data); err != nil {
return "", fmt.Errorf("failed to read deps.json (%d bytes): %w", size, err)
// ReadAt leaves the caller's cursor alone and reports a short read as io.EOF, so the count is what says
// whether the whole document was there
if n, err := r.ReadAt(data, offset); err != nil && int64(n) < size {
return "", fmt.Errorf("failed to read deps.json (%d bytes at offset %d): %w", size, offset, err)
}
return string(data), nil
}
@@ -133,10 +262,47 @@ func depsJSONFileType(majorVersion uint32) dotNetFileType {
return dotNetFileTypeDepsJSON
}
// checkManifestFits rejects a manifest whose declared entry count could not fit in the bytes left after
// the current position, which means the count came from a malformed header.
func checkManifestFits(r unionreader.UnionReader, numFiles, minEntrySize int64) error {
pos, err := r.Seek(0, io.SeekCurrent)
if err != nil {
return err
}
end, ok := intFile.ReaderSize(r)
if !ok {
return errors.New("unable to determine the file's size, so the manifest entry count cannot be weighed against it")
}
if remaining := end - pos; numFiles > remaining/minEntrySize {
return fmt.Errorf("manifest claims %d entries but only %d bytes remain", numFiles, remaining)
}
return nil
}
// findDepsJSONInManifest parses manifest entries to find deps.json (for V1 bundles or fallback)
func findDepsJSONInManifest(r io.ReadSeeker, numFiles int32, majorVersion uint32) (string, error) {
func findDepsJSONInManifest(r unionreader.UnionReader, numFiles int32, majorVersion uint32) (string, error) {
depsJSONType := depsJSONFileType(majorVersion)
if numFiles < 0 {
return "", fmt.Errorf("negative embedded file count: %d", numFiles)
}
// numFiles is a header field, so it can claim up to 2^31-1 entries. The walk terminates either way,
// since every iteration reads forward and eventually hits EOF, but a count the remaining bytes could
// not possibly hold means the header is malformed rather than describing a manifest worth hundreds of
// millions of reads.
minEntrySize := int64(minManifestEntrySize)
if majorVersion >= 6 {
minEntrySize = minManifestEntrySizeV6
}
if err := checkManifestFits(r, int64(numFiles), minEntrySize); err != nil {
return "", err
}
for i := int32(0); i < numFiles; i++ {
var offset, size int64
@@ -1,11 +1,8 @@
package bundle
import (
"bytes"
"debug/elf"
"encoding/binary"
"errors"
"io"
"github.com/anchore/syft/syft/internal/elfutil"
"github.com/anchore/syft/syft/internal/unionreader"
@@ -14,51 +11,18 @@ import (
// ExtractDepsJSONFromELFBundle extracts the deps.json content from a .net singlefile
// bundle contained within an ELF bin
func ExtractDepsJSONFromELFBundle(r unionreader.UnionReader) (string, error) {
headerOffset, err := findBundleHeaderOffsetInELF(r)
if err != nil || headerOffset == 0 {
return "", err
}
return ReadDepsJSONFromBundleHeader(r, headerOffset)
}
func findBundleHeaderOffsetInELF(r unionreader.UnionReader) (int64, error) {
elfFile, err := elfutil.NewFile(r)
if err != nil {
return 0, nil
// a refusal is not the same as "this is not an ELF": elfutil exports this error precisely so the
// gap it leaves in the SBOM can be reported rather than read as an absence of evidence
if errors.Is(err, elfutil.ErrDeclaredSizeExceeded) {
return "", err
}
// not an ELF we can parse, so not an ELF bundle
return "", nil //nolint:nilerr
}
elfEndOffset := calculateELFEndOffset(elfFile)
if elfEndOffset == 0 {
return 0, nil
}
// clamp to the actual file size so a malformed ELF (with bogus segment/section
// offsets+sizes) can't drive an arbitrarily large allocation below.
fileSize, err := r.Seek(0, io.SeekEnd)
if err != nil {
return 0, err
}
if elfEndOffset > fileSize {
elfEndOffset = fileSize
}
if _, err := r.Seek(0, io.SeekStart); err != nil {
return 0, err
}
searchData := make([]byte, elfEndOffset)
n, err := io.ReadFull(r, searchData)
if err != nil && !errors.Is(err, io.ErrUnexpectedEOF) {
return 0, err
}
searchData = searchData[:n]
idx := bytes.Index(searchData, dotNetBundleSignature)
if idx == -1 || idx < 8 {
return 0, nil
}
return int64(binary.LittleEndian.Uint64(searchData[idx-8 : idx])), nil
return ExtractDepsJSON(r, calculateELFEndOffset(elfFile))
}
func calculateELFEndOffset(f *elf.File) int64 {
@@ -2,11 +2,14 @@ package bundle
import (
"bytes"
"debug/elf"
"encoding/binary"
"testing"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
intFile "github.com/anchore/syft/internal/file"
)
// readSeekCloser adapts a *bytes.Reader to the unionreader.UnionReader interface (adds Close).
@@ -16,10 +19,44 @@ type readSeekCloser struct {
func (readSeekCloser) Close() error { return nil }
// buildELFWithHugeFilesz returns a minimal, parseable ELF64 whose single program header
// declares an absurd p_filesz. calculateELFEndOffset will compute a huge end offset; the
// clamp in findBundleHeaderOffsetInELF must keep the allocation bounded to the real file.
// buildELFWithHugeFilesz returns a minimal, parseable ELF64 whose single program header declares a
// p_filesz far past the real file. calculateELFEndOffset will compute an 8GB end offset; the bound in
// FindSignatureOffset must keep the allocation tied to the real file.
//
// note: 8GB rather than something astronomical on purpose. `make([]byte, 1<<60)` panics on its own, so a
// test built on that value passes whether or not the bound exists; `make([]byte, 8*intFile.GB)` succeeds
// on any 64-bit host from untouched anonymous mmap, so only the requested read size distinguishes them.
func buildELFWithHugeFilesz() []byte {
return buildELFWithProgHeader(0, 8*intFile.GB)
}
// readSizeRecorder records the largest single Read length requested of it, which is what tells a buffer
// sized from the file apart from one sized from the headers.
type readSizeRecorder struct {
*bytes.Reader
maxRead int
}
func (r *readSizeRecorder) Read(p []byte) (int, error) {
if len(p) > r.maxRead {
r.maxRead = len(p)
}
return r.Reader.Read(p)
}
// note: ReadAt is recorded too, since the bounded reads go through it to leave the caller's cursor alone.
func (r *readSizeRecorder) ReadAt(p []byte, off int64) (int, error) {
if len(p) > r.maxRead {
r.maxRead = len(p)
}
return r.Reader.ReadAt(p, off)
}
func (r *readSizeRecorder) Close() error { return nil }
// buildELFWithProgHeader returns a minimal, parseable ELF64 with a single PT_LOAD program header
// carrying the given p_offset and p_filesz. debug/elf does not validate either field.
func buildELFWithProgHeader(pOffset, pFilesz uint64) []byte {
const (
ehSize = 64
phSize = 56
@@ -45,19 +82,54 @@ func buildELFWithHugeFilesz() []byte {
// e_shoff/e_shnum left zero so no sections are parsed
ph := buf[phOff:]
le.PutUint32(ph[0:], 1) // p_type = PT_LOAD
le.PutUint64(ph[16:], 0) // p_offset
le.PutUint64(ph[32:], 1<<60) // p_filesz (bogus, attacker-controlled)
le.PutUint64(ph[40:], 1<<60) // p_memsz
le.PutUint32(ph[0:], 1) // p_type = PT_LOAD
le.PutUint64(ph[8:], pOffset) // p_offset (user-controlled)
le.PutUint64(ph[32:], pFilesz) // p_filesz (user-controlled)
le.PutUint64(ph[40:], pFilesz) // p_memsz
return buf
}
func TestExtractDepsJSONFromELFBundle_MalformedFileszDoesNotOverAllocate(t *testing.T) {
data := buildELFWithHugeFilesz()
func TestExtractDepsJSONFromELFBundle_EndOffsetOverflowDoesNotPanic(t *testing.T) {
// calculateELFEndOffset sums these two uint64 header fields into an int64 and adds 4096, which
// overflows to a negative search limit. A negative is not greater than the file size, so it slips
// unnoticed into the search window unless a non-positive limit reads nothing.
data := buildELFWithProgHeader(0x7FFFFFFFFFFFF000, 0)
require.Negative(t, calculateELFEndOffset(mustParseELF(t, data)),
"expected these header values to overflow the end offset; the guard below is what this test pins")
r := readSeekCloser{bytes.NewReader(data)}
// must not OOM/panic on the bogus p_filesz, and find no bundle signature
content, err := ExtractDepsJSONFromELFBundle(r)
require.NoError(t, err)
assert.Empty(t, content)
}
func mustParseELF(t *testing.T, data []byte) *elf.File {
t.Helper()
f, err := elf.NewFile(bytes.NewReader(data))
require.NoError(t, err)
return f
}
func TestExtractDepsJSONFromELFBundle_MalformedFileszDoesNotOverAllocate(t *testing.T) {
data := buildELFWithHugeFilesz()
// sanity: the headers really do describe an end offset far past the file, so the bound is what keeps
// the allocation small rather than the input being small
require.Greater(t, calculateELFEndOffset(mustParseELF(t, data)), int64(8*intFile.GB))
r := &readSizeRecorder{Reader: bytes.NewReader(data)}
var content string
var err error
allocated := measureAlloc(t, func() {
content, err = ExtractDepsJSONFromELFBundle(r)
})
require.NoError(t, err)
assert.Empty(t, content)
assert.Less(t, allocated, uint64(intFile.MB),
"the search buffer must be sized by the file, not by what the program headers claim")
assert.LessOrEqual(t, r.maxRead, len(data),
"no read may request more than the file holds")
}
@@ -0,0 +1,313 @@
package bundle
import (
"bytes"
"encoding/binary"
"io"
"math"
"runtime"
"testing"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)
// fileWithSignatureAt returns a file of the given size carrying the bundle marker (8-byte header
// offset followed by the signature) starting at sigStart.
func fileWithSignatureAt(size, sigStart int, headerOffset uint64) []byte {
data := make([]byte, size)
binary.LittleEndian.PutUint64(data[sigStart-8:sigStart], headerOffset)
copy(data[sigStart:], dotNetBundleSignature)
return data
}
func TestFindBundleHeaderOffset(t *testing.T) {
// large enough that the marker's declared header offset lands inside the file
const withMarker = 8192
const headerOffset = 0x1234
tests := []struct {
name string
data []byte
searchLimit int64
want int64
wantErr require.ErrorAssertionFunc
}{
{
name: "no signature present",
data: make([]byte, withMarker),
searchLimit: withMarker,
},
{
// the limit is only a hint about how far in the marker can be. Treating a nonsensical one as
// authoritative would let a single bogus header field hide the bundle from us.
name: "zero limit falls back to the whole file",
data: fileWithSignatureAt(withMarker, 64, headerOffset),
searchLimit: 0,
want: headerOffset,
},
{
// callers sum unsigned header fields into an int64, which overflows to a negative limit for
// some header values. That must not size a buffer, and must not hide the marker either.
name: "negative limit falls back to the whole file",
data: fileWithSignatureAt(withMarker, 64, headerOffset),
searchLimit: math.MinInt64,
want: headerOffset,
},
{
name: "limit past end of file reads only what exists",
data: fileWithSignatureAt(withMarker, 64, headerOffset),
searchLimit: math.MaxInt64 / 2,
want: headerOffset,
},
{
// the bound must not shrink the window below the real file, or a legitimate marker stops being found
name: "limit inside the file still finds the marker",
data: fileWithSignatureAt(withMarker, 64, headerOffset),
searchLimit: 128,
want: headerOffset,
},
{
name: "limit truncates the window before the marker",
data: fileWithSignatureAt(withMarker, 64, headerOffset),
searchLimit: 32,
},
{
// there is no room for the 8-byte header offset before the signature
name: "signature too close to the start to carry an offset",
data: append(append([]byte{0, 0}, dotNetBundleSignature...), make([]byte, 32)...),
searchLimit: 128,
},
{
// the offset is read straight out of the file, and everything downstream seeks to it
name: "header offset past the end of the file is rejected",
data: fileWithSignatureAt(withMarker, 64, 1<<40),
searchLimit: withMarker,
wantErr: require.Error,
},
{
// every apphost carries the signature with a zero offset placeholder; only publishing as a
// single file fills it in. Rejecting it would report a spurious parse failure against every
// ordinary framework-dependent .NET executable.
name: "header offset of zero means not bundled",
data: fileWithSignatureAt(withMarker, 64, 0),
searchLimit: withMarker,
want: 0,
},
{
// a uint64 offset read into an int64 can land negative, which would seek backwards
name: "header offset that reads negative is rejected",
data: fileWithSignatureAt(withMarker, 64, math.MaxUint64),
searchLimit: withMarker,
wantErr: require.Error,
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
if tt.wantErr == nil {
tt.wantErr = require.NoError
}
got, err := findBundleHeaderOffset(readSeekCloser{bytes.NewReader(tt.data)}, tt.searchLimit)
tt.wantErr(t, err)
if err != nil {
return
}
assert.Equal(t, tt.want, got)
})
}
}
// shortReader hands back fewer bytes than were asked for, which is what unionreader does for a squashfs
// block that decompresses short. Its size is honest; only the read comes up short, so the marker may well
// be in the bytes we did get.
type shortReader struct {
data []byte
size int64
pos int64
}
func (r *shortReader) Read(p []byte) (int, error) {
if r.pos >= int64(len(r.data)) {
return 0, io.EOF
}
n := copy(p, r.data[r.pos:])
r.pos += int64(n)
return n, nil
}
func (r *shortReader) ReadAt(p []byte, off int64) (int, error) {
if off >= int64(len(r.data)) {
return 0, io.EOF
}
n := copy(p, r.data[off:])
if n < len(p) {
return n, io.EOF
}
return n, nil
}
func (r *shortReader) Seek(offset int64, whence int) (int64, error) {
switch whence {
case io.SeekStart:
r.pos = offset
case io.SeekCurrent:
r.pos += offset
case io.SeekEnd:
// the size is what the reader reports; Read is what comes up short
r.pos = r.size + offset
}
return r.pos, nil
}
func (r *shortReader) Close() error { return nil }
// unbackedSizeReader reports a size it cannot deliver the last byte of.
//
// These readers do not all come from us. debug/elf and debug/pe hand out io.NewSectionReader(r, 0, 1<<63-1),
// whose Size() is a nominal 8EB rather than a measured one, so a reader that answers a size directly is not
// on its own evidence the bytes exist. intFile.ReaderSize confirms the count by reading the last byte, and
// the case below pins that failing that check surfaces here as an error rather than as "no bundle present".
type unbackedSizeReader struct {
*bytes.Reader
size int64
}
func (r *unbackedSizeReader) Size() int64 { return r.size }
func (r *unbackedSizeReader) ReadAt(p []byte, off int64) (int, error) {
if off >= r.Reader.Size() {
return 0, io.EOF
}
return r.Reader.ReadAt(p, off)
}
func (r *unbackedSizeReader) Close() error { return nil }
// measureAlloc reports the bytes allocated while fn runs. Only a byte count states "a small file cannot
// make us reserve a large buffer"; 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 TestFindBundleHeaderOffset_ShortReadStillSearchesWhatWasRead(t *testing.T) {
data := fileWithSignatureAt(8192, 64, 0x1234)
// honest about its length, but every read stops early
r := &shortReader{data: data, size: int64(len(data))}
got, err := findBundleHeaderOffset(r, int64(len(data)))
require.NoError(t, err)
assert.Equal(t, int64(0x1234), got,
"a short read must not abort the search; the marker was in the bytes that were returned")
}
func TestFindBundleHeaderOffset_UnbackedSizeIsReported(t *testing.T) {
data := fileWithSignatureAt(8192, 64, 0x1234)
// claims twice the bytes it holds, so nothing sized or bounds-checked against that number means anything
r := &unbackedSizeReader{Reader: bytes.NewReader(data), size: int64(len(data)) * 2}
_, err := findBundleHeaderOffset(r, int64(len(data))*2)
require.Error(t, err,
"a reader that cannot back the length it reports must be reported, not read as an absent bundle")
}
func TestRead7BitEncodedInt(t *testing.T) {
t.Run("decodes what BinaryWriter produces", func(t *testing.T) {
for _, tt := range []struct {
data []byte
want int
}{
{data: []byte{0x05}, want: 5},
{data: []byte{0x80, 0x01}, want: 128},
{data: []byte{0xFF, 0x7F}, want: 16383},
} {
got, err := read7BitEncodedInt(bytes.NewReader(tt.data))
require.NoError(t, err)
assert.Equal(t, tt.want, got)
}
})
t.Run("too many continuation bytes is rejected", func(t *testing.T) {
_, err := read7BitEncodedInt(bytes.NewReader([]byte{0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF}))
require.Error(t, err)
})
// callers seek forward by whatever this returns, so a negative value would rewind them and let a
// manifest walk re-read the same bytes for every file it claims. The shift can only carry past int32
// where int is 32 bits, so on a 64-bit build the guard is unreachable and this pins the invariant
// rather than the branch.
t.Run("never returns a negative length", func(t *testing.T) {
for _, data := range [][]byte{
{0xFF, 0xFF, 0xFF, 0xFF, 0x0F},
{0xFF, 0xFF, 0xFF, 0xFF, 0x7F},
{0x80, 0x80, 0x80, 0x80, 0x08},
} {
got, err := read7BitEncodedInt(bytes.NewReader(data))
if err != nil {
continue
}
assert.GreaterOrEqual(t, got, 0, "a length that seeks callers backwards must be rejected")
}
})
}
func TestFindDepsJSONInManifest_ImpossibleEntryCountIsRejected(t *testing.T) {
tests := []struct {
name string
numFiles int32
majorVersion uint32
}{
{
// 2^31-1 entries cannot fit in a handful of bytes, so the header is malformed rather than
// describing a manifest worth hundreds of millions of reads
name: "max int32 entries in a tiny file",
numFiles: math.MaxInt32,
},
{
name: "negative entry count",
numFiles: -1,
},
{
name: "max int32 entries in a v6 bundle",
numFiles: math.MaxInt32,
majorVersion: 6,
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
r := bytes.NewReader(make([]byte, 512))
_, err := findDepsJSONInManifest(readSeekCloser{r}, tt.numFiles, tt.majorVersion)
require.Error(t, err)
})
}
}
func TestFindDepsJSONInManifest_PlausibleEntryCountIsWalked(t *testing.T) {
// a count the remaining bytes could hold must still be walked, or the guard would reject real bundles
var buf bytes.Buffer
for range 2 {
require.NoError(t, binary.Write(&buf, binary.LittleEndian, int64(0))) // offset
require.NoError(t, binary.Write(&buf, binary.LittleEndian, int64(0))) // size
require.NoError(t, binary.Write(&buf, binary.LittleEndian, uint8(1))) // type: assembly
require.NoError(t, binary.Write(&buf, binary.LittleEndian, uint8(1))) // path length
require.NoError(t, binary.Write(&buf, binary.LittleEndian, uint8('a'))) // path
}
got, err := findDepsJSONInManifest(readSeekCloser{bytes.NewReader(buf.Bytes())}, 2, 1)
require.NoError(t, err)
assert.Empty(t, got, "no deps.json entry in this manifest")
}
@@ -1,24 +1,13 @@
package pe
import (
"bytes"
"debug/pe"
"encoding/binary"
"errors"
"io"
"github.com/anchore/syft/syft/internal/unionreader"
"github.com/anchore/syft/syft/pkg/cataloger/internal/dotnet/bundle"
)
// dotNetBundleSignature is the SHA-256 hash of ".net core bundle" used to identify single-file bundles.
var dotNetBundleSignature = []byte{
0x8b, 0x12, 0x02, 0xb9, 0x6a, 0x61, 0x20, 0x38,
0x72, 0x7b, 0x93, 0x02, 0x14, 0xd7, 0xa0, 0x32,
0x13, 0xf5, 0xb9, 0xe6, 0xef, 0xae, 0x33, 0x18,
0xee, 0x3b, 0x2d, 0xce, 0x24, 0xb3, 0x6a, 0xae,
}
// ExtractDepsJSONFromBundle searches for an embedded deps.json file in a .NET single-file bundle.
// extractDepsJSONFromBundle searches for an embedded deps.json file in a .NET single-file bundle.
// When built with PublishSingleFile=true, .NET embeds the application and all dependencies into
// the AppHost executable. The bundle marker (8-byte header offset + 32-byte signature) is placed
// in a placeholder location within the PE structure, pointing to the bundle header which contains
@@ -50,42 +39,8 @@ var dotNetBundleSignature = []byte{
// - https://github.com/dotnet/runtime/blob/main/src/native/corehost/bundle/header.h
// - https://github.com/dotnet/runtime/blob/main/src/native/corehost/bundle/file_entry.h
// - https://github.com/dotnet/runtime/blob/main/src/native/corehost/bundle/file_type.h
func extractDepsJSONFromBundle(r io.ReadSeeker, sections []pe.SectionHeader32) (string, error) {
headerOffset, err := findBundleHeaderOffset(r, sections)
if err != nil {
return "", err
}
if headerOffset == 0 {
return "", nil // not a .NET single-file bundle
}
return bundle.ReadDepsJSONFromBundleHeader(r, headerOffset)
}
// findBundleHeaderOffset locates the bundle marker within the PE structure and returns the header offset.
// Returns 0 if no bundle marker is found (not a single-file bundle).
func findBundleHeaderOffset(r io.ReadSeeker, sections []pe.SectionHeader32) (int64, error) {
peEndOffset := calculatePEEndOffset(sections)
if _, err := r.Seek(0, io.SeekStart); err != nil {
return 0, err
}
peData := make([]byte, peEndOffset)
n, err := io.ReadFull(r, peData)
if err != nil && !errors.Is(err, io.ErrUnexpectedEOF) {
return 0, err
}
peData = peData[:n]
idx := bytes.Index(peData, dotNetBundleSignature)
if idx == -1 || idx < 8 {
return 0, nil
}
// the header offset is stored in the 8 bytes immediately before the signature
headerOffset := int64(binary.LittleEndian.Uint64(peData[idx-8 : idx]))
return headerOffset, nil
func extractDepsJSONFromBundle(r unionreader.UnionReader, sections []pe.SectionHeader32) (string, error) {
return bundle.ExtractDepsJSON(r, calculatePEEndOffset(sections))
}
// calculatePEEndOffset determines where the PE structure ends based on section headers,
@@ -1,10 +1,14 @@
package pe
import (
"bytes"
"debug/pe"
"testing"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
intFile "github.com/anchore/syft/internal/file"
)
func Test_extractDepsJSONFromBundle_Versions(t *testing.T) {
@@ -65,3 +69,57 @@ func Test_extractDepsJSONFromBundle_Versions(t *testing.T) {
})
}
}
// readSizeRecorder records the largest single Read length requested of it. The worst end offset PE
// headers can describe is about 8.6GB, and `make([]byte, 8.6e9)` succeeds on any 64-bit host from fresh
// anonymous mmap without touching the pages, so asserting "no panic" would pass with or without the
// bound. The requested read size is what actually distinguishes them.
type readSizeRecorder struct {
*bytes.Reader
maxRead int
}
func (r *readSizeRecorder) Read(p []byte) (int, error) {
if len(p) > r.maxRead {
r.maxRead = len(p)
}
return r.Reader.Read(p)
}
// note: ReadAt is recorded too, since the bounded reads go through it to leave the caller's cursor alone.
func (r *readSizeRecorder) ReadAt(p []byte, off int64) (int, error) {
if len(p) > r.maxRead {
r.maxRead = len(p)
}
return r.Reader.ReadAt(p, off)
}
func (r *readSizeRecorder) Close() error { return nil }
func TestExtractDepsJSONFromBundle_MalformedSectionSizesDoNotOverAllocate(t *testing.T) {
// PointerToRawData and SizeOfRawData are user-controlled and unrelated to the real file size
sections := []pe.SectionHeader32{{PointerToRawData: 0xFFFFFFFF, SizeOfRawData: 0xFFFFFFFF}}
// sanity: the headers really do describe an end offset far past the file, so the bound is what keeps
// the allocation small rather than the input being small
require.Greater(t, calculatePEEndOffset(sections), int64(8*intFile.GB))
const fileSize = 512 // a small "file" with no bundle signature
r := &readSizeRecorder{Reader: bytes.NewReader(make([]byte, fileSize))}
var content string
var err error
allocated := measureAlloc(t, func() {
content, err = extractDepsJSONFromBundle(r, sections)
})
require.NoError(t, err)
assert.Empty(t, content)
// the byte count is the assertion that matters. make([]byte, 8.6e9) does not crash on a 64-bit host:
// it comes back from fresh anonymous mmap and never touches the pages, so a test that only checked for
// a panic would pass with the bound removed.
assert.Less(t, allocated, uint64(intFile.MB),
"the search buffer must be sized by the file, not by what the section headers claim")
assert.LessOrEqual(t, r.maxRead, fileSize,
"no read may request more than the file holds")
}
+373 -99
View File
@@ -9,15 +9,142 @@ import (
"io"
"unicode/utf16"
"github.com/scylladb/go-set/strset"
"github.com/scylladb/go-set/u32set"
intFile "github.com/anchore/syft/internal/file"
"github.com/anchore/syft/internal/log"
"github.com/anchore/syft/syft/file"
"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
// peMaxResourceDirectoryDepth bounds how deep the resource tree walk will recurse.
//
// The format uses exactly three levels (type, then name, then language), so anything past a handful
// is malformed. The bound matters because Go grows a goroutine stack until it hits the process limit
// and then dies with a fatal error rather than a recoverable panic: a chain of directories at
// distinct RVAs, each naming the next, would take the whole scan down with it. Distinct RVAs are why
// the dirs set below cannot stand in for this, and a byte budget cannot either, since the budget
// scales with the section while the stack does not.
peMaxResourceDirectoryDepth = 32
// maxDirectorySectionSize bounds the bytes any single PE data directory may declare.
//
// Only the resource and COM descriptor directories are read, and both are metadata: real ones run
// from a few KB to a few MB even for applications that embed sizable assets. Clamping to the bytes
// remaining in the file (which the caller does as well) is not on its own enough, because a mostly
// empty file compresses to almost nothing in a layer, so an attacker can hand us a small artifact
// that still authorizes a multi-gigabyte allocation. The trade-off is that a binary declaring a
// larger directory loses its version resources rather than being cataloged; that is the correct
// direction to fail, since the alternative is OOM-killing the whole scan. This mirrors
// maxDeclaredSectionSize in syft/internal/elfutil.
maxDirectorySectionSize = 128 * intFile.MB
// 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 total tree walk together.
//
// note: this bounds total work, not recursion depth. The budget scales with the section size, so a
// large section still affords a very deep chain of directory headers; depth is bounded separately by
// peMaxResourceDirectoryDepth.
budget int64
// depth is how many directory levels the current recursion is into the tree.
depth int
}
// newResourceWalk starts a walk over one resource section's bytes, fixing the origin every RVA is
// measured against. Taking both up front is what keeps reader.Size() the one authoritative bound on
// every offset the walk derives, since there is no window in which a walk exists without them.
func newResourceWalk(reader *bytes.Reader, baseRVA uint32) *resourceWalk {
return &resourceWalk{
reader: reader,
baseRVA: baseRVA,
dirs: u32set.New(),
fields: make(map[string]string),
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
}
// offsetWithin validates a section-relative offset the file supplied directly (rather than as an RVA)
// before it is used to seek. bytes.Reader happily seeks past its end and only fails on the later read, so
// checking here is what turns a bogus offset into a legible error instead of a downstream EOF.
func (w *resourceWalk) offsetWithin(offset uint32) (int64, error) {
if int64(offset) >= w.reader.Size() {
return 0, fmt.Errorf("offset 0x%x lies past its section end 0x%x", offset, w.reader.Size())
}
return int64(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")
// errResourceDepth stops the walk for the same reason as errResourceBudget: the format nests three levels,
// so a tree past the cap is crafted rather than unusual, and its siblings are the same structure.
var errResourceDepth = errors.New("resource directory nested deeper than a well-formed one could justify")
// stopsWalk reports whether an error is about the walk as a whole rather than the one entry that raised it,
// which is what tells the log-and-continue loops to propagate instead.
func stopsWalk(err error) bool {
return errors.Is(err, errResourceBudget) || errors.Is(err, errResourceDepth)
}
// 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
@@ -40,6 +167,14 @@ type File struct {
// VersionResources is a map of version resource keys to their values found in the VERSIONINFO resource directory.
VersionResources map[string]string
// ParseErr records the structures that could not be parsed out of an otherwise usable PE file.
//
// A malformed resource directory, an unreadable data directory, or a bundle marker pointing nowhere
// each cost us one piece of evidence, not the whole file, so Read reports them here and still returns
// what it did find. Callers that track partial results should join this into their unknowns rather
// than discarding the package.
ParseErr error
}
// CLREvidence is basic info about the CLR (common language runtime) version from the COM descriptor.
@@ -135,6 +270,11 @@ type extractedSection struct {
BaseRVA uint32
Size uint32
Reader *bytes.Reader
// Err records why this section's bytes could not be read, leaving Reader nil. Recording it rather
// than failing the file keeps one malformed directory from dropping the package entirely; the
// downstream parsers all treat a nil Reader as "nothing to say about this".
Err error
}
func (s extractedSection) exists() bool {
@@ -162,29 +302,43 @@ 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
// every structure below is optional evidence: losing one costs us a field, not the package. They are
// collected rather than returned so a single malformed directory cannot drop the file from the SBOM,
// and so callers can still see what went wrong instead of it only reaching a trace log.
var parseErrs []error
for _, i := range imageDirectoryEntryIndexes {
if sec := sections[i]; sec != nil && sec.Err != nil {
parseErrs = append(parseErrs, sec.Err)
}
}
c, err := parseCLR(sections[pe.IMAGE_DIRECTORY_ENTRY_COM_DESCRIPTOR], resourceNames)
walk, err := parseResourceDirectory(sections[pe.IMAGE_DIRECTORY_ENTRY_RESOURCE])
if err != nil {
return nil, fmt.Errorf("unable to parse PE CLR directory: %w", err)
parseErrs = append(parseErrs, fmt.Errorf("unable to fully parse PE resource directory: %w", err))
}
c, err := parseCLR(sections[pe.IMAGE_DIRECTORY_ENTRY_COM_DESCRIPTOR], walk.hasCLRDebugInfo)
if err != nil {
parseErrs = append(parseErrs, fmt.Errorf("unable to parse PE CLR directory: %w", err))
c = &CLREvidence{HasClrResourceNames: walk.hasCLRDebugInfo}
}
embeddedDepsJSON, err := extractDepsJSONFromBundle(r, sectionHeaders)
if err != nil {
return nil, fmt.Errorf("unable to extract embedded deps.json: %w", err)
parseErrs = append(parseErrs, fmt.Errorf("unable to extract embedded deps.json: %w", err))
}
parseErr := errors.Join(parseErrs...)
if parseErr != nil {
log.Tracef("partially parsed PE file %s: %v", f.RealPath, parseErr)
}
return &File{
Location: f.Location,
CLR: c,
EmbeddedDepsJSON: embeddedDepsJSON,
VersionResources: versionResources,
VersionResources: walk.fields,
ParseErr: parseErr,
}, nil
}
@@ -206,7 +360,10 @@ func parsePEFile(file unionreader.UnionReader) (map[int]*extractedSection, []pe.
}
data, err := readDataFromRVA(file, sec.RVA, sec.Size, headers)
if err != nil {
return nil, nil, fmt.Errorf("error reading %q section data: %w", directoryName(i), err)
// one unreadable directory says nothing about the others or about the rest of the file, so
// record it and carry on rather than dropping the package over it
sec.Err = fmt.Errorf("error reading %q section data: %w", directoryName(i), err)
continue
}
sec.Reader = data
}
@@ -297,12 +454,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 +470,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,
@@ -342,7 +501,7 @@ func rvaToFileOffset(rva uint32, sections []pe.SectionHeader32) (uint32, error)
}
// readDataFromRVA will read data from a specific RVA in the PE file
func readDataFromRVA(file io.ReadSeeker, rva, size uint32, sections []pe.SectionHeader32) (*bytes.Reader, error) {
func readDataFromRVA(file io.ReaderAt, rva, size uint32, sections []pe.SectionHeader32) (*bytes.Reader, error) {
if size == 0 {
return nil, fmt.Errorf("zero size specified")
}
@@ -352,12 +511,31 @@ func readDataFromRVA(file io.ReadSeeker, rva, size uint32, sections []pe.Section
return nil, err
}
if _, err := file.Seek(int64(offset), io.SeekStart); err != nil {
return nil, fmt.Errorf("error seeking to data: %w", err)
// size is a user-controlled uint32, so sizing the buffer from it alone lets a small file reserve up to
// 4GB. Two bounds apply before it sizes anything: the bytes that actually remain in the file, and the
// absolute cap, which is what keeps a sparse multi-gigabyte file (cheap to ship inside a compressed
// layer) from authorizing a multi-gigabyte allocation. Both checks precede the allocation, and the
// allocation is 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.
if size > maxDirectorySectionSize {
return nil, fmt.Errorf("error reading data: %d bytes declared at offset %d exceeds the %d byte limit", size, offset, maxDirectorySectionSize)
}
// the length has to come from something the reader can back up rather than from what it claims, since
// the whole point here is weighing a declared size against the bytes that are really present
end, ok := intFile.ReaderSize(file)
if !ok {
return nil, errors.New("error measuring file")
}
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 {
// ReadAt may report a full read as io.EOF when it lands on the end of the file, so the count is what
// says whether the whole section was there
if n, err := file.ReadAt(data, int64(offset)); err != nil && n < len(data) {
return nil, fmt.Errorf("error reading data: %w", err)
}
@@ -388,13 +566,15 @@ 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 {
// parseResourceDirectory walks a resource section and returns the walk that collected from it. The walk is
// always non-nil, even on error: a tree that stops partway still tells us about the fields it did yield.
func parseResourceDirectory(sec *extractedSection) (*resourceWalk, error) {
if sec == nil || sec.Size <= 0 {
return nil
return newResourceWalk(bytes.NewReader(nil), 0), nil
}
if sec.Reader == nil {
return errors.New("resource section not found")
return newResourceWalk(bytes.NewReader(nil), 0), errors.New("resource section not found")
}
baseRVA := sec.BaseRVA
@@ -402,40 +582,74 @@ 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 := newResourceWalk(sec.Reader, baseRVA)
return w, parseResourceDirectoryAt(sec.RVA, w)
}
func parseResourceDirectoryAt(rva uint32, w *resourceWalk) error {
// a resource tree is three levels deep by spec, so anything past the cap is malformed. This has to be
// its own bound: the dirs set only catches a directory naming an RVA already seen, and a chain of
// distinct RVAs each naming the next would otherwise recurse until the goroutine stack gives out,
// which is a fatal error no caller can recover from.
w.depth++
defer func() { w.depth-- }()
if w.depth > peMaxResourceDirectoryDepth {
return fmt.Errorf("%w: %d levels", errResourceDepth, peMaxResourceDirectoryDepth)
}
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 or depth limit hit 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 stopsWalk(err) {
return err
}
log.Tracef("error processing resource entry: %v", err)
continue
}
@@ -444,7 +658,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 +670,95 @@ 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)
nameAt, err := w.offsetWithin(nameOffset)
if err != nil {
return fmt.Errorf("resource name: %w", err)
}
name, err := readUTF16WithLength(sec.Reader)
if err == nil {
names.Add(name)
if _, err := w.reader.Seek(nameAt, io.SeekStart); err != nil {
return fmt.Errorf("error seeking to resource name: %w", err)
}
if _, err := sec.Reader.Seek(currentPos, io.SeekStart); err != nil {
// only one name matters downstream, so compare in place rather than retaining every name a
// crafted file cares to declare
name, err := w.readUTF16WithLength()
switch {
case stopsWalk(err):
return err
case err == nil && name == clrDebugInfoResourceName:
w.hasCLRDebugInfo = true
}
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)
if err != nil {
return err
}
return nil
w.dirs.Add(targetRVA)
return parseResourceDirectoryAt(targetRVA, w)
}
return parseResourceDataEntry(sec.Reader, baseRVA, baseRVA+entryOffsetToData, sec.Size, fields)
return parseResourceDataEntry(targetRVA, w)
}
func parseResourceDataEntry(reader *bytes.Reader, baseRVA, rva, remainingSize uint32, fields map[string]string) error {
var dataEntry peImageResourceDataEntry
offset := int64(rva - baseRVA)
func parseResourceDataEntry(rva uint32, w *resourceWalk) error {
offset, err := w.offsetOf(rva)
if err != nil {
return fmt.Errorf("resource data entry: %w", err)
}
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.
@@ -607,6 +841,11 @@ func parseVersionResourceSection(reader *bytes.Reader, fields map[string]string)
var sfiHeader peStringFileInfo
if szKey, err := readIntoStructAndSzKey(reader, &sfiHeader, &offset); err != nil {
if isTruncated(err) {
// a well-formed version resource whose last child is VarFileInfo ends right here, so stop
// and let the FileVersion fallback below still run
break
}
return fmt.Errorf("error reading PE string file info header: %v", err)
} else if szKey != "StringFileInfo" {
// we only care about extracting strings from any string tables, skip this
@@ -619,31 +858,14 @@ func parseVersionResourceSection(reader *bytes.Reader, fields map[string]string)
// note: the szKey for the prStringTable is the language
var stHeader peStringTable
if _, err := readIntoStructAndSzKey(reader, &stHeader, &offset, &stOffset); err != nil {
if isTruncated(err) {
break
}
return fmt.Errorf("error reading PE string table header: %v", err)
}
for stOffset < int(stHeader.Length) {
var stringHeader peString
if err := readIntoStruct(reader, &stringHeader, &offset, &stOffset); err != nil {
break
}
key := readUTF16(reader, &offset, &stOffset)
if err := alignAndSeek(reader, &offset, &stOffset); err != nil {
return fmt.Errorf("error aligning to next PE string table value: %w", err)
}
var value string
if stringHeader.ValueLength > 0 {
value = readUTF16(reader, &offset, &stOffset)
}
fields[key] = value
if err := alignAndSeek(reader, &offset, &stOffset); err != nil {
return fmt.Errorf("error aligning to next PE string table key: %w", err)
}
if err := parseStringTable(reader, int(stHeader.Length), &offset, &stOffset, fields); err != nil {
return err
}
}
@@ -657,6 +879,40 @@ func parseVersionResourceSection(reader *bytes.Reader, fields map[string]string)
return nil
}
// parseStringTable reads the key/value pairs of a single string table into fields. length is what the
// string table header claims it holds, and stOffset tracks how much of that has actually been consumed.
func parseStringTable(reader *bytes.Reader, length int, offset, stOffset *int, fields map[string]string) error {
for *stOffset < length {
var stringHeader peString
if err := readIntoStruct(reader, &stringHeader, offset, stOffset); err != nil {
if isTruncated(err) {
// the table claims more content than the resource carries; stop rather than re-reading a
// reader that is not advancing
break
}
return fmt.Errorf("error reading PE string table entry: %w", err)
}
key := readUTF16(reader, offset, stOffset)
if err := alignAndSeek(reader, offset, stOffset); err != nil {
return fmt.Errorf("error aligning to next PE string table value: %w", err)
}
var value string
if stringHeader.ValueLength > 0 {
value = readUTF16(reader, offset, stOffset)
}
fields[key] = value
if err := alignAndSeek(reader, offset, stOffset); err != nil {
return fmt.Errorf("error aligning to next PE string table key: %w", err)
}
}
return nil
}
// readIntoStructAndSzKey reads a struct from the reader and updates the offsets if provided, returning the szKey value.
// This is only useful in the context of the resource directory parsing in narrow cases (this is invalid to use outside of that context).
func readIntoStructAndSzKey[T any](reader *bytes.Reader, data *T, offsets ...*int) (string, error) {
@@ -666,12 +922,19 @@ func readIntoStructAndSzKey[T any](reader *bytes.Reader, data *T, offsets ...*in
return readUTF16(reader, offsets...), nil
}
// isTruncated reports whether err means the resource simply ran out of bytes. binary.Read gives io.EOF when
// nothing was left and io.ErrUnexpectedEOF when a struct was cut in half; both say the same thing about a
// version resource, and neither should cost us the fields already collected.
func isTruncated(err error) bool {
return errors.Is(err, io.EOF) || errors.Is(err, io.ErrUnexpectedEOF)
}
// readIntoStruct reads a struct from the reader and updates the offsets if provided.
//
// 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 {
if errors.Is(err, io.EOF) {
return nil
}
return err
}
@@ -726,20 +989,31 @@ func readUTF16(reader *bytes.Reader, offsets ...*int) string {
return string(result)
}
// readUTF16WithLength reads a length-prefixed UTF-16 string from reader.
// readUTF16WithLength reads a length-prefixed UTF-16 string from the walk's current position.
// The first 2 bytes represent the number of UTF-16 code units.
func readUTF16WithLength(reader *bytes.Reader) (string, error) {
func (w *resourceWalk) readUTF16WithLength() (string, error) {
var length uint16
if err := binary.Read(reader, binary.LittleEndian, &length); err != nil {
if err := binary.Read(w.reader, binary.LittleEndian, &length); err != nil {
return "", err
}
if length == 0 {
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(w.reader.Len()) {
return "", fmt.Errorf("declared name length %d exceeds the %d bytes remaining", length, w.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 {
if err := binary.Read(w.reader, binary.LittleEndian, &codes); err != nil {
return "", err
}
return string(utf16.Decode(codes)), nil
@@ -0,0 +1,132 @@
package pe
import (
"bytes"
"debug/pe"
"encoding/binary"
"testing"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
syftFile "github.com/anchore/syft/syft/file"
)
// buildPE32 assembles a minimal but genuinely parseable PE32 with one section, wiring the resource and COM
// descriptor data directories to the RVAs given. A zero size leaves that directory absent.
func buildPE32(t *testing.T, resourceRVA, resourceSize, clrRVA, clrSize uint32, sectionData []byte) []byte {
t.Helper()
const peAt = 0x80
const sectionRVA = 0x1000
const sectionRaw = 0x400
buf := make([]byte, sectionRaw)
copy(buf, "MZ")
binary.LittleEndian.PutUint32(buf[60:], peAt)
copy(buf[peAt:], "PE\x00\x00")
fh := pe.FileHeader{
Machine: 0x14c,
NumberOfSections: 1,
SizeOfOptionalHeader: uint16(binary.Size(pe.OptionalHeader32{})),
}
hdr := new(bytes.Buffer)
require.NoError(t, binary.Write(hdr, binary.LittleEndian, fh))
opt := pe.OptionalHeader32{Magic: 0x10B}
opt.DataDirectory[pe.IMAGE_DIRECTORY_ENTRY_RESOURCE] = pe.DataDirectory{VirtualAddress: resourceRVA, Size: resourceSize}
opt.DataDirectory[pe.IMAGE_DIRECTORY_ENTRY_COM_DESCRIPTOR] = pe.DataDirectory{VirtualAddress: clrRVA, Size: clrSize}
require.NoError(t, binary.Write(hdr, binary.LittleEndian, opt))
sec := pe.SectionHeader32{
VirtualAddress: sectionRVA,
VirtualSize: 0x1000,
SizeOfRawData: uint32(sectionRaw),
PointerToRawData: uint32(sectionRaw),
}
copy(sec.Name[:], ".rsrc")
require.NoError(t, binary.Write(hdr, binary.LittleEndian, sec))
copy(buf[peAt+4:], hdr.Bytes())
return append(buf, sectionData...)
}
func readPE(t *testing.T, data []byte) (*File, error) {
t.Helper()
loc := syftFile.NewLocation("test.dll")
return Read(syftFile.NewLocationReadCloser(loc, readSeekCloser{bytes.NewReader(data)}))
}
type readSeekCloser struct{ *bytes.Reader }
func (readSeekCloser) Close() error { return nil }
func TestRead_MalformedResourceDirectoryStillYieldsAFile(t *testing.T) {
// one malformed directory costs us the fields it would have held, not the package. Failing the whole
// file here used to drop it from the SBOM entirely, and the pe-binary cataloger then reported an error
// instead of a package.
section := make([]byte, 0x400)
// a root directory claiming counts that sum past 0xFFFF, which is rejected outright
binary.LittleEndian.PutUint16(section[12:], 0x8000)
binary.LittleEndian.PutUint16(section[14:], 0x8001)
// a valid CLR header lives further into the same section, so there is still evidence to recover
clr := peImageCore20{Cb: 72, MajorRuntimeVersion: 2, MinorRuntimeVersion: 5}
clrBytes := new(bytes.Buffer)
require.NoError(t, binary.Write(clrBytes, binary.LittleEndian, clr))
copy(section[0x200:], clrBytes.Bytes())
data := buildPE32(t, 0x1000, 0x400, 0x1200, uint32(clrBytes.Len()), section)
f, err := readPE(t, data)
require.NoError(t, err, "a malformed resource directory must not fail the whole file")
require.NotNil(t, f)
// the partial parse has to be reported rather than only reaching a trace log, or the unknowns task
// cannot see that this file was cataloged on incomplete evidence
require.Error(t, f.ParseErr, "the resource directory failure must be recorded")
assert.Contains(t, f.ParseErr.Error(), "resource directory")
// and the evidence that did parse has to survive
assert.True(t, f.CLR.HasEvidenceOfCLR(), "the CLR header parsed fine and must still be reported")
assert.Equal(t, uint16(2), f.CLR.MajorVersion)
}
func TestRead_UnreadableDataDirectoryStillYieldsAFile(t *testing.T) {
// the same rule one layer down: a directory whose bytes cannot be read at all is recorded and skipped.
// This used to be fatal, which was inconsistent with the resource walk being non-fatal a line away.
section := make([]byte, 0x400)
// a resource directory declaring far more than the file holds
data := buildPE32(t, 0x1000, 0xFFFFFFF, 0, 0, section)
f, err := readPE(t, data)
require.NoError(t, err)
require.NotNil(t, f)
require.Error(t, f.ParseErr)
assert.Contains(t, f.ParseErr.Error(), "Resource")
assert.Empty(t, f.VersionResources)
}
func TestRead_WellFormedFileReportsNoParseError(t *testing.T) {
// the guard against the above turning every file into a partial parse
section := make([]byte, 0x400)
putResourceDirN(section, 0x000, 1, 0x100, false) // root -> data entry
binary.LittleEndian.PutUint32(section[0x100:], 0x1000+0x200)
binary.LittleEndian.PutUint32(section[0x104:], 0x100)
copy(section[0x200:], buildVersionResource(true))
data := buildPE32(t, 0x1000, 0x400, 0, 0, section)
f, err := readPE(t, data)
require.NoError(t, err)
require.NotNil(t, f)
assert.NoError(t, f.ParseErr, "a file that parsed cleanly must not report a partial parse")
assert.Equal(t, "9.9.9.9", f.VersionResources["FileVersion"])
}
@@ -0,0 +1,514 @@
package pe
import (
"bytes"
"encoding/binary"
"fmt"
"runtime"
"testing"
"time"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
intFile "github.com/anchore/syft/internal/file"
)
const (
testSectionRVA = 0x1000
testSectionSize = 0x400
)
// putResourceDirN writes a resource directory header declaring n ID entries, all of them pointing at the
// same target. isDir sets the high bit, which marks that target as a subdirectory. Aliasing every entry
// onto one target 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 over data as its resource section, as parseResourceDirectory would.
func boundWalk(data []byte) *resourceWalk {
return newResourceWalk(bytes.NewReader(data), testSectionRVA)
}
// 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*intFile.MB) // a size far past the 1KB section
err := parseResourceDataEntry(testSectionRVA+0x100, boundWalk(buf))
// note: this pins the bound, not an allocation. A single-level entry was already rejected before this
// change (by comparing against the section size), it just said so differently, so an allocation
// assertion here would pass with or without the fix. TestParseResourceDirectory_NestedOffsetsCannotUnderflow
// is the one that holds the allocation property, because that is the shape that used to slip through.
require.ErrorContains(t, err, "extends past its section end")
}
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)
putResourceDirN(buf, 0x000, 1, 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 * intFile.KB
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 := boundWalk(buf)
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.
// expressed in terms of the factor so raising the budget cannot silently widen what this accepts
assert.Less(t, allocated, uint64(8*peResourceBudgetFactor*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.
func buildVersionResource(withFileVersionString bool) []byte {
buf := new(bytes.Buffer)
le := binary.LittleEndian
putHeader := func(length, valueLength, typ uint16) {
_ = binary.Write(buf, le, [3]uint16{length, valueLength, typ})
}
putUTF16 := func(s string) {
for _, r := range s {
_ = binary.Write(buf, le, uint16(r))
}
_ = binary.Write(buf, le, uint16(0))
}
pad := func() {
for buf.Len()%4 != 0 {
buf.WriteByte(0)
}
}
putHeader(0, 52, 0)
putUTF16("VS_VERSION_INFO")
pad()
// peVsFixedFileInfo, with FileVersionMS/LS encoding 1.2.3.4
ffi := make([]byte, 52)
le.PutUint32(ffi[0:], 0xFEEF04BD) // signature
le.PutUint32(ffi[4:], 0x00010000) // strucVersion
le.PutUint32(ffi[8:], 0x00010002) // FileVersionMS -> 1.2
le.PutUint32(ffi[12:], 0x00030004) // FileVersionLS -> 3.4
buf.Write(ffi)
if withFileVersionString {
pad()
putHeader(0, 0, 1)
putUTF16("StringFileInfo")
pad()
putHeader(38, 0, 1)
putUTF16("040904b0")
pad()
putHeader(0, 8, 1)
putUTF16("FileVersion")
pad()
putUTF16("9.9.9.9")
}
// the final child: VarFileInfo -> Var("Translation") with a 4 byte value
pad()
putHeader(0, 0, 1)
putUTF16("VarFileInfo")
pad()
putHeader(0, 4, 0)
putUTF16("Translation")
pad()
_ = binary.Write(buf, le, uint32(0x04b00409))
return buf.Bytes()
}
func TestParseVersionResourceSection_FileVersionFallback(t *testing.T) {
// a resource ending on a struct boundary is normal termination, not a parse failure. Treating it as
// an error skips the VS_FIXEDFILEINFO fallback below, which is the only source of a version for
// binaries that carry no FileVersion string entry.
tests := []struct {
name string
withFileVersionString bool
want string
}{
{
name: "derived from fixed file info when no string entry exists",
want: "1.2.3.4",
},
{
name: "string entry wins when present",
withFileVersionString: true,
want: "9.9.9.9",
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
fields := map[string]string{}
require.NoError(t, parseVersionResourceSection(bytes.NewReader(buildVersionResource(tt.withFileVersionString)), fields))
assert.Equal(t, tt.want, fields["FileVersion"])
})
}
}
// buildTruncatedStringTable returns version resource bytes that end exactly on a struct boundary while
// the string table header still claims 0xFFFF bytes remain. Landing precisely at EOF is one of two cases
// that matter; trailing bytes that cut a struct in half are the other, covered by
// TestParseVersionResourceSection_TrailingBytesKeepFileVersion.
func buildTruncatedStringTable() []byte {
buf := new(bytes.Buffer)
putHeader := func() {
_ = binary.Write(buf, binary.LittleEndian, [3]uint16{}) // Length, ValueLength, Type
}
putUTF16 := func(s string) {
for _, r := range s {
_ = binary.Write(buf, binary.LittleEndian, uint16(r))
}
_ = binary.Write(buf, binary.LittleEndian, uint16(0)) // null terminator
}
putHeader()
putUTF16("VS_VERSION_INFO") // offset 38
buf.Write([]byte{0, 0}) // pad to the DWORD boundary at 40
buf.Write(make([]byte, 52)) // peVsFixedFileInfo -> 92
putHeader()
putUTF16("StringFileInfo") // -> 128
// the string table header claims far more content than the file carries
_ = binary.Write(buf, binary.LittleEndian, [3]uint16{0xFFFF, 0, 0})
putUTF16("040904b0") // -> 152, then EOF
return buf.Bytes()
}
func TestParseVersionResourceSection_TruncatedStringTableTerminates(t *testing.T) {
data := buildTruncatedStringTable()
done := make(chan error, 1)
go func() {
done <- parseVersionResourceSection(bytes.NewReader(data), map[string]string{})
}()
select {
case <-done:
case <-time.After(5 * time.Second):
// a reader parked at EOF used to be reported as a successful read of a zeroed struct, so the
// string table loop consumed nothing, advanced nothing, and never reached its length bound
require.FailNow(t, "parseVersionResourceSection did not terminate",
"a %d-byte resource blob must not spin forever", len(data))
}
}
func TestParseResourceDirectory_DeepChainCannotOverflowTheStack(t *testing.T) {
// a chain of directories at distinct RVAs, each naming the next. Every offset is inside the section and
// every RVA is distinct, so neither the dirs set (which only catches a repeated RVA) nor the byte budget
// (which scales with the section) stops it. Unbounded, this recurses until the goroutine stack hits the
// process limit, and Go answers that with a fatal error no recover() can catch: the whole scan dies.
//
// the stride is 12 bytes, the tightest packing where each directory's counts and its single entry do not
// collide with its neighbours' (dir k reads counts at 12k+12 and its entry at 12k+16, while dir k+1 reads
// counts at 12k+24).
const stride = 12
const levels = 200
buf := make([]byte, stride*(levels+4))
le := binary.LittleEndian
for k := range levels {
at := stride * k
le.PutUint16(buf[at+14:], 1) // one ID entry
le.PutUint32(buf[at+16:], uint32(k)) // Name (distinct ID)
le.PutUint32(buf[at+20:], uint32(stride*(k+1))|0x80000000) // -> next directory
}
w := boundWalk(buf)
err := parseResourceDirectoryAt(testSectionRVA, w)
require.ErrorIs(t, err, errResourceDepth,
"a chain deeper than the cap must be rejected rather than recursed into")
assert.LessOrEqual(t, w.depth, peMaxResourceDirectoryDepth,
"the depth counter must unwind as the walk returns")
}
func TestParseResourceDirectory_NestedOffsetsCannotUnderflow(t *testing.T) {
// the shape that used to authorize a ~4.29GB allocation out of a 1KB section. Each level's offset is
// individually smaller than the section, so each directory header reads fine, but the old walk tracked a
// uint32 "remaining size" and subtracted each level's offset from it. By the third level that subtraction
// underflowed to near 4GB, and a leaf declaring a size just under it then passed the bounds check and
// sized the buffer from it.
buf := make([]byte, testSectionSize)
le := binary.LittleEndian
putResourceDirN(buf, 0x000, 1, 0x300, true) // root -> level 2
putResourceDirN(buf, 0x300, 1, 0x300, true) // level 2 -> level 3 (same offset, deeper)
putResourceDirN(buf, 0x300, 1, 0x200, false) // level 3 -> data entry
// the leaf claims almost 4GB, which the underflowed remaining size used to permit
le.PutUint32(buf[0x200:], testSectionRVA) // OffsetToData -> section offset 0
le.PutUint32(buf[0x204:], 0xFFFFFF00) // Size
w := boundWalk(buf)
var err error
allocated := measureAlloc(t, func() {
err = parseResourceDirectoryAt(testSectionRVA, w)
})
require.NoError(t, err, "bad children are logged and skipped, so the walk itself completes")
assert.Less(t, allocated, uint64(intFile.MB),
"a 1KB section must never authorize a multi-gigabyte read, however deeply it is nested")
assert.Empty(t, w.fields, "nothing in this tree is a real version resource")
}
func TestParseResourceDirectory_SelfReferentialEntryTerminates(t *testing.T) {
// a directory whose entry names its own RVA. The dirs set is what catches this, and it is the guard the
// budget and depth bounds cannot express, since one level of aliasing costs almost nothing.
buf := make([]byte, testSectionSize)
putResourceDirN(buf, 0x000, 1, 0, true) // root -> itself (offset 0)
w := boundWalk(buf)
done := make(chan error, 1)
go func() { done <- parseResourceDirectoryAt(testSectionRVA, w) }()
select {
case err := <-done:
require.NoError(t, err, "the self-reference is logged and skipped, not fatal")
assert.Empty(t, w.fields)
case <-time.After(5 * time.Second):
require.FailNow(t, "a self-referential resource directory did not terminate")
}
}
func TestProcessResourceEntry_HostileNameLengthIsRejected(t *testing.T) {
// a named entry's length prefix is a user-controlled uint16, and binary.Read allocates a buffer of its
// own on top of ours, so a name the section cannot satisfy has to be rejected before either is sized.
//
// note: a name that cannot be read is not fatal to the entry. The name is optional evidence (only
// CLRDEBUGINFO matters downstream), so the walk carries on and simply does not learn it. That is why
// these assert on the allocation and on the name not being recorded, rather than on an error: only the
// allocation states the property these guards exist for.
tests := []struct {
name string
nameOffset uint32
length uint16
}{
{
name: "declared length past the end of the section",
nameOffset: testSectionSize - 4,
length: 0xFFFF,
},
{
name: "name offset past the end of the section",
nameOffset: testSectionSize,
length: 1,
},
{
name: "declared length of MaxUint16 at the section start",
nameOffset: 0,
length: 0xFFFF,
},
{
// a zero-length name is legal and must not drive a read
name: "zero length name",
nameOffset: 0x100,
length: 0,
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
buf := make([]byte, testSectionSize)
if int(tt.nameOffset) < len(buf)-2 {
binary.LittleEndian.PutUint16(buf[tt.nameOffset:], tt.length)
}
// a data entry (high bit clear on OffsetToData) with a string name (high bit set on Name)
entry := peImageResourceDirectoryEntry{
Name: tt.nameOffset | 0x80000000,
OffsetToData: 0x100,
}
w := boundWalk(buf)
allocated := measureAlloc(t, func() {
_ = processResourceEntry(entry, w)
})
assert.Less(t, allocated, uint64(intFile.MB),
"a declared name length must never size a buffer the section cannot back")
assert.False(t, w.hasCLRDebugInfo,
"no name was readable, so none should have been recorded")
})
}
}
func TestProcessResourceEntry_CLRDebugInfoNameIsRecorded(t *testing.T) {
// the one name that matters downstream, so the happy path through the length-prefixed reader is pinned
// alongside the hostile ones above
buf := make([]byte, testSectionSize)
le := binary.LittleEndian
const nameAt = 0x100
le.PutUint16(buf[nameAt:], uint16(len(clrDebugInfoResourceName)))
for i, r := range clrDebugInfoResourceName {
le.PutUint16(buf[nameAt+2+i*2:], uint16(r))
}
w := boundWalk(buf)
entry := peImageResourceDirectoryEntry{Name: nameAt | 0x80000000, OffsetToData: 0x200}
_ = processResourceEntry(entry, w)
assert.True(t, w.hasCLRDebugInfo, "a CLRDEBUGINFO resource name must be recorded")
}
func TestParseVersionResourceSection_TrailingBytesKeepFileVersion(t *testing.T) {
// the case the isTruncated guard exists for. A version resource whose last child leaves 1 to 5 trailing
// bytes cuts the next struct in half, which binary.Read reports as io.ErrUnexpectedEOF rather than
// io.EOF. That used to propagate out as a parse failure, and because the failure returned early it also
// skipped the VS_FIXEDFILEINFO fallback, so the binary lost its version entirely.
for trailing := 0; trailing <= 6; trailing++ {
t.Run(fmt.Sprintf("%d trailing bytes", trailing), func(t *testing.T) {
data := append(buildVersionResource(false), make([]byte, trailing)...)
fields := map[string]string{}
require.NoError(t, parseVersionResourceSection(bytes.NewReader(data), fields))
assert.Equal(t, "1.2.3.4", fields["FileVersion"],
"a resource that runs out mid-struct must still yield the fields already collected")
})
}
}
func TestParseResourceDataEntry_ZeroSizeIsHandled(t *testing.T) {
// a zero-size data entry is degenerate rather than hostile, and must not be reported as a version
// resource or drive a read
buf := make([]byte, testSectionSize)
le := binary.LittleEndian
le.PutUint32(buf[0x100:], testSectionRVA) // OffsetToData
le.PutUint32(buf[0x104:], 0) // Size
w := boundWalk(buf)
err := parseResourceDataEntry(testSectionRVA+0x100, w)
require.Error(t, err, "an empty blob carries no version info")
assert.Empty(t, w.fields)
}
func TestParseResourceDirectory_EmptySectionIsRejected(t *testing.T) {
// a zero-length section means every offset is out of range, including the root's
w := newResourceWalk(bytes.NewReader(nil), testSectionRVA)
err := parseResourceDirectoryAt(testSectionRVA, w)
require.ErrorContains(t, err, "lies past its section end")
}
@@ -0,0 +1,98 @@
package pe
import (
"bytes"
"debug/pe"
"testing"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
intFile "github.com/anchore/syft/internal/file"
)
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, so the shortfall has to be caught before the allocation: the read stays
// a single exactly-sized one, and a size the file cannot back is an error rather than a mostly-zero
// buffer returned 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(intFile.MB),
"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 the
// read 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")
}
func TestReadDataFromRVA_SizePastTheAbsoluteCapIsRejected(t *testing.T) {
// clamping to the bytes remaining in the file is not enough on its own: a mostly empty file costs
// almost nothing inside a compressed layer, so a cheap artifact can still declare a directory large
// enough to OOM the scan. The absolute cap is what bounds that, and it has to run before the
// allocation like every other bound here.
const size = maxDirectorySectionSize + 1
sections := []pe.SectionHeader32{{VirtualAddress: 0x1000, VirtualSize: 0x1000, PointerToRawData: 0}}
// a sparse file that really is large enough to satisfy the declared size
r := &readSizeRecorder{Reader: bytes.NewReader(make([]byte, size+1))}
var err error
allocated := measureAlloc(t, func() {
_, err = readDataFromRVA(r, 0x1000, size, sections)
})
require.ErrorContains(t, err, "exceeds the")
assert.Less(t, allocated, uint64(intFile.MB),
"the cap has to be checked before the buffer is sized, not after")
}
func TestReadDataFromRVA_SizeAtTheAbsoluteCapIsAllowed(t *testing.T) {
// the cap must not reject what it is meant to permit, so pin the boundary from the other side. Only the
// requested read length is checked here; allocating the full cap would make the test cost of this the
// same as the bug it guards against.
sections := []pe.SectionHeader32{{VirtualAddress: 0x1000, VirtualSize: 0x1000, PointerToRawData: 0}}
// one byte short of the cap, in a file that cannot back it: the size check has to pass and the
// remaining-bytes check has to be what rejects it
r := &readSizeRecorder{Reader: bytes.NewReader(make([]byte, 512))}
_, err := readDataFromRVA(r, 0x1000, maxDirectorySectionSize-1, sections)
require.ErrorContains(t, err, "only 512 remain",
"a size under the cap must fall through to the remaining-bytes check, not be capped away")
}
@@ -106,6 +106,28 @@ func Test_Read_DotNetDetection(t *testing.T) {
},
wantErr: require.NoError,
},
{
// the framework-dependent apphost, which carries the bundle signature with a zero offset
// placeholder because it was never published as a single file. This is the most common shape a
// .NET executable takes, so anything it reports as a parse failure is reported against nearly
// every .NET binary that exists.
name: "framework dependent apphost",
path: "/app/dotnetapp.exe",
fixture: "image-net8-app",
wantCLR: false, // the CLR metadata lives in the sibling dotnetapp.dll
wantVR: map[string]string{
"Assembly Version": "1.0.0.0",
"CompanyName": "dotnetapp",
"FileDescription": "dotnetapp",
"FileVersion": "1.0.0.0",
"InternalName": "dotnetapp.dll",
"LegalCopyright": " ",
"OriginalFilename": "dotnetapp.dll",
"ProductName": "dotnetapp",
"ProductVersion": "1.0.0",
},
wantErr: require.NoError,
},
{
name: "single file deployment",
path: "/app/dotnetapp.exe",
@@ -149,6 +171,10 @@ func Test_Read_DotNetDetection(t *testing.T) {
return
}
// a real binary must parse completely. ParseErr is how a partial parse reaches the unknowns
// channel, so anything non-nil here is a field we silently failed to read on stock input.
require.NoError(t, got.ParseErr, "a well-formed binary must not report a partial parse")
if d := cmp.Diff(tt.wantVR, got.VersionResources); d != "" {
t.Errorf("unexpected version resources (-want +got): %s", d)
}