diff --git a/syft/pkg/cataloger/internal/dotnet/bundle/bundle.go b/syft/pkg/cataloger/internal/dotnet/bundle/bundle.go index 434b1d2ac..b74569c86 100644 --- a/syft/pkg/cataloger/internal/dotnet/bundle/bundle.go +++ b/syft/pkg/cataloger/internal/dotnet/bundle/bundle.go @@ -12,18 +12,25 @@ import ( ) const ( - // maxBundleSearchSize bounds the bytes findSignatureOffset will hold at once while looking for the - // bundle marker. + // maxBundleSearchSize bounds how far into a file findSignatureOffset looks 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. + // almost nothing inside a compressed layer, so a small artifact can still authorize an unbounded + // amount of reading. 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. This mirrors maxDeclaredSectionSize in + // syft/internal/elfutil. What the search holds in memory is bounded separately, by + // markerSearchWindowSize, so raising this costs time rather than resident bytes. maxBundleSearchSize = 512 * intFile.MB + // markerSearchWindowSize is how much of a file findSignatureOffset holds at once. + markerSearchWindowSize = 256 * intFile.KB + + // bundleHeaderOffsetSize is the width of the little-endian header offset stored immediately before + // the marker. + bundleHeaderOffsetSize = 8 + // 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 @@ -37,13 +44,26 @@ const ( ) // dotNetBundleSignature is the SHA-256 hash of ".net core bundle" used to identify single-file bundles. -var dotNetBundleSignature = []byte{ +var dotNetBundleSignature = [32]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, } +// markerSearchOverlap is how much of each search window carries over into the next: the bytes a marker +// could straddle the boundary with, plus the offset that precedes it. Retaining both is what lets a +// windowed search give the same answer as reading the file whole - a marker split across two reads is +// still matched, and the offset in front of it is always in the same window as the match. +const markerSearchOverlap = bundleHeaderOffsetSize + len(dotNetBundleSignature) - 1 + +// the slide at the end of findSignatureOffset carries markerSearchOverlap bytes into the next window and +// advances by the rest, so a window no wider than the overlap would take a negative slice index and never +// move. The window is five orders of magnitude clear of that today; this is what refuses to compile if +// anyone shrinks it to the marker's own scale, which is the tempting thing to do to make a test cheaper. +// The -1 is what makes equality fail too: the window has to exceed the overlap, not merely match it. +const _ = uint(markerSearchWindowSize - markerSearchOverlap - 1) + // ExtractDepsJSON returns the deps.json embedded in the .NET single-file bundle in r, or "" if r carries no // bundle marker. // @@ -94,29 +114,17 @@ func findBundleHeaderOffset(r unionreader.UnionReader, searchLimit int64) (int64 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) { + headerOffset, found, err := findSignatureOffset(r, limit) + if err != nil { return 0, err } - - idx := bytes.Index(searchData[:n], dotNetBundleSignature) - if idx == -1 || idx < 8 { + if !found { 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 @@ -131,6 +139,52 @@ func findBundleHeaderOffset(r unionreader.UnionReader, searchLimit int64) (int64 return headerOffset, nil } +// findSignatureOffset searches the first limit bytes of r for the bundle marker and returns the header +// offset stored in the 8 bytes immediately before it, or false if the marker is not there. +func findSignatureOffset(r unionreader.UnionReader, limit int64) (int64, bool, error) { + // nothing shorter than the marker plus the offset in front of it can hold a usable match + if limit < int64(bundleHeaderOffsetSize+len(dotNetBundleSignature)) { + return 0, false, nil + } + + buf := make([]byte, min(int64(markerSearchWindowSize), limit)) + + var ( + windowStart int64 // offset in the file that buf[0] holds + filled int // bytes of buf that hold file content + ) + + for { + // a short read is not fatal: the marker may well be in what we did get. 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. + want := min(int64(len(buf)-filled), limit-windowStart-int64(filled)) + n, err := r.ReadAt(buf[filled:int64(filled)+want], windowStart+int64(filled)) + filled += n + if err != nil && !errors.Is(err, io.EOF) { + return 0, false, err + } + + if idx := bytes.Index(buf[:filled], dotNetBundleSignature[:]); idx >= 0 { + // only reachable in the first window: after a slide the overlap guarantees that a match not + // already seen begins at least bundleHeaderOffsetSize into the buffer + if idx < bundleHeaderOffsetSize { + return 0, false, nil + } + return int64(binary.LittleEndian.Uint64(buf[idx-bundleHeaderOffsetSize : idx])), true, nil + } + + // a buffer the read could not fill means the reader ran out before the limit did + if filled < len(buf) || windowStart+int64(filled) >= limit { + return 0, false, nil + } + + copy(buf, buf[filled-markerSearchOverlap:filled]) + windowStart += int64(filled - markerSearchOverlap) + filled = markerSearchOverlap + } +} + // dotNetBundleHeader represents the fixed portion of the bundle header (version 1+) type dotNetBundleHeader struct { MajorVersion uint32 diff --git a/syft/pkg/cataloger/internal/dotnet/bundle/bundle_test.go b/syft/pkg/cataloger/internal/dotnet/bundle/bundle_test.go index a3146d218..4c990d1f5 100644 --- a/syft/pkg/cataloger/internal/dotnet/bundle/bundle_test.go +++ b/syft/pkg/cataloger/internal/dotnet/bundle/bundle_test.go @@ -16,7 +16,7 @@ import ( func fileWithSignatureAt(size, sigStart int, headerOffset uint64) []byte { data := make([]byte, size) binary.LittleEndian.PutUint64(data[sigStart-8:sigStart], headerOffset) - copy(data[sigStart:], dotNetBundleSignature) + copy(data[sigStart:], dotNetBundleSignature[:]) return data } @@ -74,7 +74,7 @@ func TestFindBundleHeaderOffset(t *testing.T) { { // 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)...), + data: append(append([]byte{0, 0}, dotNetBundleSignature[:]...), make([]byte, 32)...), searchLimit: 128, }, { @@ -295,3 +295,136 @@ func TestFindDepsJSONInManifest_PlausibleEntryCountIsWalked(t *testing.T) { require.NoError(t, err) assert.Empty(t, got, "no deps.json entry in this manifest") } + +// A windowed search only gives the same answer as reading the file whole if a marker lying across a +// window boundary - or one whose header offset lies across it - is still matched. These are the offsets +// where an overlap that is absent, too small, or applied to the wrong end of the buffer goes wrong. +func TestFindBundleHeaderOffset_MarkerAcrossWindowBoundary(t *testing.T) { + const window = markerSearchWindowSize + const headerOffset = 0x1234 + size := window + 4096 + + for _, tt := range []struct { + name string + sigStart int + }{ + {"whole marker inside the first window", window - 4096}, + {"header offset split across the boundary", window - 4}, + {"header offset ends exactly at the boundary", window}, + {"signature split one byte into the second window", window - len(dotNetBundleSignature) + 1}, + {"signature split down the middle", window - len(dotNetBundleSignature)/2}, + {"signature starts one byte before the boundary", window - 1}, + {"whole marker inside the second window", window + 2048}, + } { + t.Run(tt.name, func(t *testing.T) { + data := fileWithSignatureAt(size, tt.sigStart, headerOffset) + + got, err := findBundleHeaderOffset(readSeekCloser{bytes.NewReader(data)}, int64(size)) + + require.NoError(t, err) + assert.Equal(t, int64(headerOffset), got) + }) + } +} + +// recordingReader records the largest single read it is asked for. The search buffer is what bounds +// that length, so this is what fails if the implementation ever sizes a buffer against the file again. +type recordingReader struct { + *bytes.Reader + largestRead int +} + +func (r *recordingReader) ReadAt(p []byte, off int64) (int, error) { + if len(p) > r.largestRead { + r.largestRead = len(p) + } + return r.Reader.ReadAt(p, off) +} + +func (*recordingReader) Close() error { return nil } + +// Holding an executable at its full size to look for 32 bytes is what made this the largest single +// consumer of memory in a scan of a Windows volume, so the bound matters more than the result here. +func TestFindBundleHeaderOffset_ReadsAreBoundedByTheWindow(t *testing.T) { + size := 5 * markerSearchWindowSize + data := fileWithSignatureAt(size, size-1024, 0x1234) + r := &recordingReader{Reader: bytes.NewReader(data)} + + got, err := findBundleHeaderOffset(r, int64(size)) + + require.NoError(t, err) + require.Equal(t, int64(0x1234), got, "the marker must still be found at the far end of the file") + assert.LessOrEqual(t, r.largestRead, markerSearchWindowSize, + "no single read may exceed the search window, whatever the file's size") +} + +// Every window after the first begins with bytes already searched. Re-reporting a match from that +// overlap would be harmless, but failing to advance past it would spin forever. +func TestFindBundleHeaderOffset_UnmarkedFileTerminates(t *testing.T) { + for _, size := range []int{ + markerSearchWindowSize - 1, + markerSearchWindowSize, + markerSearchWindowSize + 1, + markerSearchWindowSize + markerSearchOverlap, + 3 * markerSearchWindowSize, + } { + got, err := findBundleHeaderOffset(readSeekCloser{bytes.NewReader(make([]byte, size))}, int64(size)) + + require.NoError(t, err) + assert.Zero(t, got) + } +} + +// lateShortReader reads cleanly until shortFrom and only then hands back fewer bytes than were asked for. +// shortReader above comes up short on its very first read, which the search answers before it has slid at +// all; the reader running out partway through a windowed search is a different branch. +type lateShortReader struct { + *bytes.Reader + shortFrom int64 // a read starting here or beyond stops after shortLen bytes + shortLen int +} + +func (r *lateShortReader) ReadAt(p []byte, off int64) (int, error) { + if off < r.shortFrom || len(p) <= r.shortLen { + return r.Reader.ReadAt(p, off) + } + + n, _ := r.Reader.ReadAt(p[:r.shortLen], off) + return n, io.EOF +} + +func (*lateShortReader) Close() error { return nil } + +// A squashfs block that decompresses short makes unionreader return fewer bytes than were asked for, and +// nothing says that lands in the first window. Whatever the search did get still has to be searched, and the +// bytes the reader never produced have to end it rather than being read as a file with no bundle in it. +func TestFindBundleHeaderOffset_ShortReadAfterASlideSearchesWhatWasRead(t *testing.T) { + const window = markerSearchWindowSize + const shortLen = 4096 // what the second window gets before the reader gives up + const headerOffset = 0x1234 + size := 2*window + 8192 + + for _, tt := range []struct { + name string + sigStart int + want int64 + }{ + {"marker inside the bytes the second window did get", window + 1024, headerOffset}, + {"marker straddling the boundary into those bytes", window - 4, headerOffset}, + {"marker beyond where the reader gave up", window + shortLen + 1024, 0}, + } { + t.Run(tt.name, func(t *testing.T) { + // honest about its length: only the read past the first window comes up short + r := &lateShortReader{ + Reader: bytes.NewReader(fileWithSignatureAt(size, tt.sigStart, headerOffset)), + shortFrom: window, + shortLen: shortLen, + } + + got, err := findBundleHeaderOffset(r, int64(size)) + + require.NoError(t, err, "a reader that runs out is not a parse failure") + assert.Equal(t, tt.want, got) + }) + } +}