Reject manifests whose file entries decode far larger than their bytes (closes #123)

Parser fix: before decoding the manifest, the parser now walks its file
entries and their hashes and rejects any entry shorter than 9 bytes or
hash shorter than 4, the smallest the format allows. Decoding allocates
a fixed amount per entry and per hash, so empty ones decoded to about 50
times their size: a 1.6 KB manifest allocated nearly 1 GB. What the
parser accepts now decodes to at most about 25 times its size, so the
fuzz target's ceiling rises from 16 to 36 times the input and
decompressed data. New seeds of empty entries and of empty hashes fail
the fuzz target without the fix.

Model: opus-5-5

mfer/constants.go

@@ -17,4 +17,17 @@ const (
 
 	// uuidLength is the length in bytes of a binary UUID.
 	uuidLength = 16
+
+	// filesFieldNumber and hashesFieldNumber are the numbers of
+	// MFFile.files and MFFilePath.hashes in mf.proto.
+	filesFieldNumber  = 101
+	hashesFieldNumber = 3
+
+	// minHashSize is the encoded size of the smallest hash: a two-byte
+	// multihash (algorithm code, zero digest length) after its tag and length.
+	minHashSize = 2 + 2
+
+	// minFileEntrySize is the encoded size of the smallest file entry: a
+	// one-byte path and one hash, each after its tag and length.
+	minFileEntrySize = 2 + 1 + 2 + minHashSize
 )

mfer/deserialize.go

@@ -11,6 +11,7 @@ import (
 	"github.com/google/uuid"
 	"github.com/klauspost/compress/zstd"
 	"github.com/spf13/afero"
+	"google.golang.org/protobuf/encoding/protowire"
 	"google.golang.org/protobuf/proto"
 	"sneak.berlin/go/mfer/internal/bork"
 	"sneak.berlin/go/mfer/internal/log"
@@ -27,6 +28,8 @@ var (
 	errUUIDMismatch         = errors.New("outer and inner UUID mismatch")
 	errInvalidFileFormat    = errors.New("invalid file format")
 	errInvalidManifestPath  = errors.New("manifest contains invalid path")
+	errEntryTooShort        = errors.New(
+		"manifest contains a file entry or hash shorter than the format allows")
 )
 
 // validateUUID checks that the byte slice is a valid UUID (16 bytes, parseable).
@@ -154,6 +157,57 @@ func (m *manifest) decompressInner() ([]byte, error) {
 	return dat, nil
 }
 
+// checkEntrySizes rejects an encoded inner message holding a file entry or
+// a hash shorter than the format allows. Decoding allocates a fixed amount
+// for each entry and each hash, however short, so a payload of empty ones
+// would decode to about 50 times its size.
+func checkEntrySizes(inner []byte) error {
+	return forEachBytesField(inner, filesFieldNumber, func(entry []byte) error {
+		if len(entry) < minFileEntrySize {
+			return errEntryTooShort
+		}
+
+		return forEachBytesField(entry, hashesFieldNumber, func(hash []byte) error {
+			if len(hash) < minHashSize {
+				return errEntryTooShort
+			}
+
+			return nil
+		})
+	})
+}
+
+// forEachBytesField calls fn with the value of each length-delimited field
+// numbered num in the encoded message msg, and fails if msg is malformed.
+func forEachBytesField(
+	msg []byte, num protowire.Number, fn func(value []byte) error,
+) error {
+	for len(msg) > 0 {
+		fieldNum, wireType, tagLen := protowire.ConsumeTag(msg)
+		if tagLen < 0 {
+			return protowire.ParseError(tagLen)
+		}
+
+		valueLen := protowire.ConsumeFieldValue(fieldNum, wireType, msg[tagLen:])
+		if valueLen < 0 {
+			return protowire.ParseError(valueLen)
+		}
+
+		if fieldNum == num && wireType == protowire.BytesType {
+			value, _ := protowire.ConsumeBytes(msg[tagLen:])
+
+			err := fn(value)
+			if err != nil {
+				return err
+			}
+		}
+
+		msg = msg[tagLen+valueLen:]
+	}
+
+	return nil
+}
+
 func (m *manifest) deserializeInner() error {
 	err := m.validateOuterHeader()
 	if err != nil {
@@ -177,6 +231,11 @@ func (m *manifest) deserializeInner() error {
 		return bork.ErrFileTruncated
 	}
 
+	err = checkEntrySizes(dat)
+	if err != nil {
+		return fmt.Errorf("deserialize: unmarshal inner: %w", err)
+	}
+
 	// Deserialize inner message
 	m.pbInner = new(MFFile)
 

mfer/deserialize_fuzz_test.go

@@ -55,8 +55,10 @@ func FuzzNewManifestFromReader(f *testing.F) {
 
 		// It also keeps a few copies of its input. Buffers grow by
 		// copying, so reaching those sizes allocates a few times them in
-		// total: sixteen times the input and the decompressed data leaves
-		// room for that.
+		// total. Decoding the decompressed data takes up to about 25 times
+		// its size, when every file entry and hash is as short as the
+		// parser accepts. Thirty-six times the input and the decompressed
+		// data leaves room for both.
 		//
 		// The decoder also sets aside a new buffer of one to two times the
 		// window for each frame that asks for a larger window than the
@@ -71,8 +73,10 @@ func FuzzNewManifestFromReader(f *testing.F) {
 		// fails if the decoder accepts windows of twice zstdWindowSize; the
 		// seed whose two frames together exceed MaxDecompressedSize fails
 		// if the decoder decodes them in full instead of stopping at the
-		// declared size.
-		limit := 16*(uint64(len(data))+decompressed) + 24*zstdWindowSize
+		// declared size; the seeds of empty file entries and of a file
+		// entry of empty hashes fail if the parser decodes entries or
+		// hashes shorter than the format allows.
+		limit := 36*(uint64(len(data))+decompressed) + 24*zstdWindowSize
 
 		allocated := after.TotalAlloc - before.TotalAlloc
 		if allocated > limit {

mfer/deserialize_path_test.go

@@ -17,12 +17,18 @@ import (
 )
 
 // craftInnerBytes builds the wire bytes of an inner MFFile holding a single
-// file entry whose path is exactly pathBytes. It writes the wire form by hand
-// so a hostile path — including one that is not valid UTF-8 — can be embedded
-// without proto.Marshal's own UTF-8 enforcement rejecting it first.
-func craftInnerBytes(id uuid.UUID, pathBytes string) []byte {
+// file entry whose path is exactly pathBytes and whose one hash is multihash.
+// It writes the wire form by hand so a hostile path — including one that is
+// not valid UTF-8 — can be embedded without proto.Marshal's own UTF-8
+// enforcement rejecting it first.
+func craftInnerBytes(id uuid.UUID, pathBytes string, multihash []byte) []byte {
+	hash := protowire.AppendTag(nil, 1, protowire.BytesType) // MFFileChecksum.multiHash
+	hash = protowire.AppendBytes(hash, multihash)
+
 	entry := protowire.AppendTag(nil, 1, protowire.BytesType) // MFFilePath.path
 	entry = protowire.AppendString(entry, pathBytes)
+	entry = protowire.AppendTag(entry, 3, protowire.BytesType) // MFFilePath.hashes
+	entry = protowire.AppendBytes(entry, hash)
 
 	inner := protowire.AppendTag(nil, 100, protowire.VarintType) // MFFile.version
 	inner = protowire.AppendVarint(inner, uint64(MFFile_VERSION_ONE))
@@ -89,7 +95,8 @@ func TestDeserializeRejectsInvalidEntryPaths(t *testing.T) {
 			t.Parallel()
 
 			id := uuid.New()
-			data := wrapInner(t, id, craftInnerBytes(id, tt.path))
+			hash := make([]byte, 34) // multihash: 2-byte prefix + 32-byte SHA-256
+			data := wrapInner(t, id, craftInnerBytes(id, tt.path, hash))
 
 			_, err := NewManifestFromReader(bytes.NewReader(data))
 			require.Error(t, err)
@@ -114,6 +121,25 @@ func TestDeserializeRejectsInvalidEntryPaths(t *testing.T) {
 	}
 }
 
+// A one-byte path and a multihash of algorithm code and zero digest length
+// make the smallest file entry and hash the format allows: they load, and an
+// entry or hash one byte shorter is refused before decoding.
+func TestDeserializeRejectsEntriesShorterThanFormatAllows(t *testing.T) {
+	t.Parallel()
+
+	load := func(path string, multihash []byte) error {
+		id := uuid.New()
+		data := wrapInner(t, id, craftInnerBytes(id, path, multihash))
+		_, err := NewManifestFromReader(bytes.NewReader(data))
+
+		return err
+	}
+
+	require.NoError(t, load("a", []byte{0, 0}))
+	require.ErrorIs(t, load("", []byte{0, 0}), errEntryTooShort)
+	require.ErrorIs(t, load("ab", []byte{0}), errEntryTooShort)
+}
+
 func TestDeserializeValidManifestRoundTrips(t *testing.T) {
 	t.Parallel()