diff --git a/syft/pkg/cataloger/internal/dotnet/bundle/bundle.go b/syft/pkg/cataloger/internal/dotnet/bundle/bundle.go index facb70e73..672e9eb8e 100644 --- a/syft/pkg/cataloger/internal/dotnet/bundle/bundle.go +++ b/syft/pkg/cataloger/internal/dotnet/bundle/bundle.go @@ -1,6 +1,7 @@ package bundle import ( + "bytes" "encoding/binary" "errors" "fmt" @@ -15,6 +16,54 @@ 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 comes from user-controlled header fields, so it may describe far more +// than the file holds or overflow negative; it is clamped to the real file length before anything is sized +// from it. A non-positive limit reads nothing. +func ExtractDepsJSON(r io.ReadSeeker, searchLimit int64) (string, error) { + headerOffset, err := findSignatureOffset(r, searchLimit) + if err != nil || headerOffset == 0 { + return "", err + } + + return readDepsJSONFromBundleHeader(r, headerOffset) +} + +// findSignatureOffset 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, or 0 if the signature is not found. +func findSignatureOffset(r io.ReadSeeker, searchLimit int64) (int64, error) { + if searchLimit <= 0 { + return 0, nil + } + + end, err := r.Seek(0, io.SeekEnd) + if err != nil { + return 0, err + } + + if _, err := r.Seek(0, io.SeekStart); err != nil { + return 0, err + } + + // 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, min(searchLimit, end)) + if _, err := io.ReadFull(r, searchData); err != nil { + return 0, err + } + + idx := bytes.Index(searchData, dotNetBundleSignature) + if idx == -1 || idx < 8 { + return 0, nil + } + + return int64(binary.LittleEndian.Uint64(searchData[idx-8 : idx])), nil +} + // dotNetBundleHeader represents the fixed portion of the bundle header (version 1+) type dotNetBundleHeader struct { MajorVersion uint32 @@ -44,7 +93,7 @@ const ( ) // ReadDepsJSONFromBundleHeader parses the bundle header at the given offset and extracts deps.json content. -func ReadDepsJSONFromBundleHeader(r io.ReadSeeker, headerOffset int64) (string, error) { +func readDepsJSONFromBundleHeader(r io.ReadSeeker, headerOffset int64) (string, error) { if _, err := r.Seek(headerOffset, io.SeekStart); err != nil { return "", err } @@ -103,6 +152,13 @@ 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 } diff --git a/syft/pkg/cataloger/internal/dotnet/bundle/bundle_elf.go b/syft/pkg/cataloger/internal/dotnet/bundle/bundle_elf.go index b65cd173e..7a9984d88 100644 --- a/syft/pkg/cataloger/internal/dotnet/bundle/bundle_elf.go +++ b/syft/pkg/cataloger/internal/dotnet/bundle/bundle_elf.go @@ -1,11 +1,7 @@ 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 +10,13 @@ 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 + // not an ELF, 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..23ac2e6dc 100644 --- a/syft/pkg/cataloger/internal/dotnet/bundle/bundle_elf_test.go +++ b/syft/pkg/cataloger/internal/dotnet/bundle/bundle_elf_test.go @@ -2,6 +2,7 @@ package bundle import ( "bytes" + "debug/elf" "encoding/binary" "testing" @@ -16,10 +17,36 @@ 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<<30)` 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<<30) +} + +// 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) +} + +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 +72,47 @@ 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*1024*1024*1024)) + + r := &readSizeRecorder{Reader: bytes.NewReader(data)} + + content, err := ExtractDepsJSONFromELFBundle(r) + require.NoError(t, err) + assert.Empty(t, content) + assert.LessOrEqual(t, r.maxRead, len(data), + "the search must be bounded by the file, not by what the program headers claim") +} 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..479711590 --- /dev/null +++ b/syft/pkg/cataloger/internal/dotnet/bundle/bundle_test.go @@ -0,0 +1,81 @@ +package bundle + +import ( + "bytes" + "encoding/binary" + "math" + "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 TestFindSignatureOffset(t *testing.T) { //nolint:funlen + const withMarker = 256 + + tests := []struct { + name string + data []byte + searchLimit int64 + want int64 + }{ + { + name: "no signature present", + data: make([]byte, 512), + searchLimit: 512, + }, + { + name: "zero limit searches nothing", + data: fileWithSignatureAt(withMarker, 64, 0x1234), + searchLimit: 0, + }, + { + // callers sum unsigned header fields into an int64, which overflows to a negative limit for + // some header values. This must read nothing rather than sizing a buffer from it. + name: "negative limit reads nothing", + data: fileWithSignatureAt(withMarker, 64, 0x1234), + searchLimit: math.MinInt64, + }, + { + name: "limit past end of file reads only what exists", + data: fileWithSignatureAt(withMarker, 64, 0x1234), + searchLimit: math.MaxInt64 / 2, + want: 0x1234, + }, + { + // 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, 0x1234), + searchLimit: 128, + want: 0x1234, + }, + { + name: "limit truncates the window before the marker", + data: fileWithSignatureAt(withMarker, 64, 0x1234), + 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, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got, err := findSignatureOffset(bytes.NewReader(tt.data), tt.searchLimit) + require.NoError(t, err) + assert.Equal(t, tt.want, got) + }) + } +} diff --git a/syft/pkg/cataloger/internal/dotnet/pe/bundle.go b/syft/pkg/cataloger/internal/dotnet/pe/bundle.go index 791b476b2..faf3b7824 100644 --- a/syft/pkg/cataloger/internal/dotnet/pe/bundle.go +++ b/syft/pkg/cataloger/internal/dotnet/pe/bundle.go @@ -1,23 +1,12 @@ package pe import ( - "bytes" "debug/pe" - "encoding/binary" - "errors" "io" "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. // 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 @@ -51,41 +40,7 @@ var dotNetBundleSignature = []byte{ // - 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 + 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 d25b33f7c..514b43c35 100644 --- a/syft/pkg/cataloger/internal/dotnet/pe/bundle_test.go +++ b/syft/pkg/cataloger/internal/dotnet/pe/bundle_test.go @@ -1,6 +1,8 @@ package pe import ( + "bytes" + "debug/pe" "testing" "github.com/stretchr/testify/assert" @@ -56,3 +58,37 @@ 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) +} + +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*1024*1024*1024)) + + const fileSize = 512 // a small "file" with no bundle signature + r := &readSizeRecorder{Reader: bytes.NewReader(make([]byte, fileSize))} + + content, err := extractDepsJSONFromBundle(r, sections) + require.NoError(t, err) + assert.Empty(t, content) + assert.LessOrEqual(t, r.maxRead, fileSize, + "the search must be bounded by the file, not by what the section headers claim") +}