fix: search for the .NET bundle marker without holding the file (#5393)

Signed-off-by: Peter McConnell <peter.mcconnell@upwind.io>
2026-10-11 21:57:22 +02:00 · 2026-10-09 09:36:52 -04:00
parent 3e41abadc4
commit 740beb6dc4
2 changed files with 211 additions and 24 deletions
@@ -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
@@ -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)
+		})
+	}
+}