diff --git a/syft/pkg/cataloger/binary/pe_package_cataloger.go b/syft/pkg/cataloger/binary/pe_package_cataloger.go index 74abb379d..05410ab97 100644 --- a/syft/pkg/cataloger/binary/pe_package_cataloger.go +++ b/syft/pkg/cataloger/binary/pe_package_cataloger.go @@ -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 } diff --git a/syft/pkg/cataloger/dotnet/deps_binary_cataloger.go b/syft/pkg/cataloger/dotnet/deps_binary_cataloger.go index 15f19de22..cde56fd44 100644 --- a/syft/pkg/cataloger/dotnet/deps_binary_cataloger.go +++ b/syft/pkg/cataloger/dotnet/deps_binary_cataloger.go @@ -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) } diff --git a/syft/pkg/cataloger/internal/dotnet/bundle/bundle.go b/syft/pkg/cataloger/internal/dotnet/bundle/bundle.go index baf00bd1a..3bead6fb9 100644 --- a/syft/pkg/cataloger/internal/dotnet/bundle/bundle.go +++ b/syft/pkg/cataloger/internal/dotnet/bundle/bundle.go @@ -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 diff --git a/syft/pkg/cataloger/internal/dotnet/bundle/bundle_elf.go b/syft/pkg/cataloger/internal/dotnet/bundle/bundle_elf.go index b65cd173e..257e7b3be 100644 --- a/syft/pkg/cataloger/internal/dotnet/bundle/bundle_elf.go +++ b/syft/pkg/cataloger/internal/dotnet/bundle/bundle_elf.go @@ -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 { diff --git a/syft/pkg/cataloger/internal/dotnet/bundle/bundle_elf_test.go b/syft/pkg/cataloger/internal/dotnet/bundle/bundle_elf_test.go index 7a49dbea0..8521b8425 100644 --- a/syft/pkg/cataloger/internal/dotnet/bundle/bundle_elf_test.go +++ b/syft/pkg/cataloger/internal/dotnet/bundle/bundle_elf_test.go @@ -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") +} diff --git a/syft/pkg/cataloger/internal/dotnet/bundle/bundle_test.go b/syft/pkg/cataloger/internal/dotnet/bundle/bundle_test.go new file mode 100644 index 000000000..70d2afedc --- /dev/null +++ b/syft/pkg/cataloger/internal/dotnet/bundle/bundle_test.go @@ -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") +} diff --git a/syft/pkg/cataloger/internal/dotnet/pe/bundle.go b/syft/pkg/cataloger/internal/dotnet/pe/bundle.go index 791b476b2..2032c83fa 100644 --- a/syft/pkg/cataloger/internal/dotnet/pe/bundle.go +++ b/syft/pkg/cataloger/internal/dotnet/pe/bundle.go @@ -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, diff --git a/syft/pkg/cataloger/internal/dotnet/pe/bundle_test.go b/syft/pkg/cataloger/internal/dotnet/pe/bundle_test.go index 6bd1f110d..4a9f4e536 100644 --- a/syft/pkg/cataloger/internal/dotnet/pe/bundle_test.go +++ b/syft/pkg/cataloger/internal/dotnet/pe/bundle_test.go @@ -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") +} diff --git a/syft/pkg/cataloger/internal/dotnet/pe/pe.go b/syft/pkg/cataloger/internal/dotnet/pe/pe.go index c389d55bd..9939e153e 100644 --- a/syft/pkg/cataloger/internal/dotnet/pe/pe.go +++ b/syft/pkg/cataloger/internal/dotnet/pe/pe.go @@ -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 diff --git a/syft/pkg/cataloger/internal/dotnet/pe/pe_partial_test.go b/syft/pkg/cataloger/internal/dotnet/pe/pe_partial_test.go new file mode 100644 index 000000000..bf8e48e1b --- /dev/null +++ b/syft/pkg/cataloger/internal/dotnet/pe/pe_partial_test.go @@ -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"]) +} diff --git a/syft/pkg/cataloger/internal/dotnet/pe/pe_resource_test.go b/syft/pkg/cataloger/internal/dotnet/pe/pe_resource_test.go new file mode 100644 index 000000000..bb074e3e2 --- /dev/null +++ b/syft/pkg/cataloger/internal/dotnet/pe/pe_resource_test.go @@ -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") +} diff --git a/syft/pkg/cataloger/internal/dotnet/pe/pe_rva_test.go b/syft/pkg/cataloger/internal/dotnet/pe/pe_rva_test.go new file mode 100644 index 000000000..fbccb6eb3 --- /dev/null +++ b/syft/pkg/cataloger/internal/dotnet/pe/pe_rva_test.go @@ -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") +} diff --git a/syft/pkg/cataloger/internal/dotnet/pe/pe_test.go b/syft/pkg/cataloger/internal/dotnet/pe/pe_test.go index 4804de444..b612e7bc0 100644 --- a/syft/pkg/cataloger/internal/dotnet/pe/pe_test.go +++ b/syft/pkg/cataloger/internal/dotnet/pe/pe_test.go @@ -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) }