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

Parser fix: before decoding the manifest, the parser walks its file
entries and adds up what decoding sets aside for each entry, hash,
timestamp and MIME type, however short its encoding. It refuses the
manifest once that sum passes 8 times the decompressed size; manifests
mfer writes come to at most about 7.15 times. Empty entries decoded to
about 50 times their size, so a manifest of under 1 KB allocated about
500 MB. A test refuses entries counted at just over 8 times and loads
them at just under. The fuzz target's ceiling rises from 16 to 20 times
the input and decompressed data, and seeds of empty entries and of
empty hashes fail it without the fix.

Model: opus-5-5

FORMAT.md

@@ -49,7 +49,9 @@ The `innerMessage` field is compressed with
 enforce a decompression size limit to prevent decompression bombs. The reference
 implementation limits decompressed size to 256 MB. It writes zstd frames with a
 window of at most 8 MiB, the largest window the zstd format recommends decoders
-support, and refuses frames that ask for a larger one.
+support, and refuses frames that ask for a larger one. It also refuses an inner
+message whose file entries, hashes, timestamps and MIME types, counted at 160,
+112, 64 and 16 bytes each, add up to more than 8 times its size.
 
 ## Inner Message (`MFFile`)
 

mfer/constants.go

@@ -17,4 +17,27 @@ const (
 
 	// uuidLength is the length in bytes of a binary UUID.
 	uuidLength = 16
+
+	// Numbers in mf.proto of MFFile.files and of the MFFilePath fields
+	// that decoding sets aside a fixed amount of memory for.
+	filesFieldNumber    = 101
+	hashesFieldNumber   = 3
+	mimeTypeFieldNumber = 301
+	mtimeFieldNumber    = 302
+	ctimeFieldNumber    = 303
+
+	// Bytes decoding sets aside for each file entry, hash, timestamp and
+	// MIME type, however short its encoding. checkDecodedSize refuses an
+	// inner message for which these add up to more than maxDecodedGrowth
+	// times its size.
+	decodedFileEntrySize = 160
+	decodedHashSize      = 112
+	decodedTimestampSize = 64
+	decodedMIMETypeSize  = 16
+
+	// Each file entry mfer writes holds a path of at least one byte, a
+	// 34-byte SHA-256 multihash and a modification time: at least 47 bytes,
+	// counted at 336. So its manifests add up to at most about 7.15 times
+	// their size, and this limit is about 12% above that.
+	maxDecodedGrowth = 8
 )

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")
+	errDecodedTooLarge      = errors.New(
+		"manifest would take too much memory to decode")
 )
 
 // validateUUID checks that the byte slice is a valid UUID (16 bytes, parseable).
@@ -154,6 +157,85 @@ func (m *manifest) decompressInner() ([]byte, error) {
 	return dat, nil
 }
 
+// checkDecodedSize refuses an encoded inner message whose file entries,
+// hashes, timestamps and MIME types would take more than maxDecodedGrowth
+// times its size to decode. Decoding sets aside a fixed amount for each,
+// however short its encoding, so a message of empty ones would take about
+// 50 times its size.
+func checkDecodedSize(inner []byte) error {
+	limit := maxDecodedGrowth * int64(len(inner))
+
+	var decoded int64
+
+	add := func(size int64) error {
+		decoded += size
+		if decoded > limit {
+			return errDecodedTooLarge
+		}
+
+		return nil
+	}
+
+	return forEachBytesField(inner, func(num protowire.Number, entry []byte) error {
+		if num != filesFieldNumber {
+			return nil
+		}
+
+		err := add(decodedFileEntrySize)
+		if err != nil {
+			return err
+		}
+
+		return forEachBytesField(entry, func(num protowire.Number, _ []byte) error {
+			if num == hashesFieldNumber {
+				return add(decodedHashSize)
+			}
+
+			if num == mtimeFieldNumber || num == ctimeFieldNumber {
+				return add(decodedTimestampSize)
+			}
+
+			if num == mimeTypeFieldNumber {
+				return add(decodedMIMETypeSize)
+			}
+
+			return nil
+		})
+	})
+}
+
+// forEachBytesField calls fn with the number and value of each
+// length-delimited field in the encoded message msg, and fails if msg is
+// malformed.
+func forEachBytesField(
+	msg []byte, fn func(num protowire.Number, value []byte) error,
+) error {
+	for len(msg) > 0 {
+		num, wireType, tagLen := protowire.ConsumeTag(msg)
+		if tagLen < 0 {
+			return protowire.ParseError(tagLen)
+		}
+
+		valueLen := protowire.ConsumeFieldValue(num, wireType, msg[tagLen:])
+		if valueLen < 0 {
+			return protowire.ParseError(valueLen)
+		}
+
+		if wireType == protowire.BytesType {
+			value, _ := protowire.ConsumeBytes(msg[tagLen:])
+
+			err := fn(num, 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 +259,11 @@ func (m *manifest) deserializeInner() error {
 		return bork.ErrFileTruncated
 	}
 
+	err = checkDecodedSize(dat)
+	if err != nil {
+		return fmt.Errorf("deserialize: unmarshal inner: %w", err)
+	}
+
 	// Deserialize inner message
 	m.pbInner = new(MFFile)
 

mfer/deserialize_fuzz_test.go

@@ -54,9 +54,14 @@ 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.
+		// copying, so reaching those sizes allocates up to about six times
+		// them in total. Decoding the decompressed data takes up to
+		// maxDecodedGrowth times its size for file entries, hashes,
+		// timestamps and MIME types, and up to about five times more for
+		// the bytes it copies out of it, such as fields it does not know,
+		// which it keeps in buffers that also grow by copying. Twenty
+		// times the input and the decompressed data leaves room for all of
+		// that.
 		//
 		// 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 +76,9 @@ 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 them.
+		limit := 20*(uint64(len(data))+decompressed) + 24*zstdWindowSize
 
 		allocated := after.TotalAlloc - before.TotalAlloc
 		if allocated > limit {

mfer/deserialize_path_test.go

@@ -6,7 +6,10 @@ import (
 	"context"
 	"crypto/sha256"
 	"fmt"
+	"strconv"
+	"strings"
 	"testing"
+	"time"
 
 	"github.com/google/uuid"
 	"github.com/klauspost/compress/zstd"
@@ -114,6 +117,83 @@ func TestDeserializeRejectsInvalidEntryPaths(t *testing.T) {
 	}
 }
 
+// Entries of a path, an empty hash, an empty MIME type and empty modification
+// and change times are counted at 416 bytes each (160 + 112 + 16 + 64 + 64)
+// and take 16 bytes plus the path to encode. A 35-character path makes that
+// 51 bytes, about 8.2 times: refused, and leaving any one of the five
+// uncounted, even the MIME type, brings it under 8. A 37-character path makes
+// it 53 bytes, about 7.8 times: loaded.
+func TestDeserializeRefusesEntriesThatDecodeTooLarge(t *testing.T) {
+	t.Parallel()
+
+	tests := []struct {
+		pathLen int
+		refused bool
+	}{
+		{35, true},
+		{37, false},
+	}
+
+	for _, tt := range tests {
+		t.Run(strconv.Itoa(tt.pathLen), func(t *testing.T) {
+			t.Parallel()
+
+			entry := protowire.AppendTag(nil, 1, protowire.BytesType) // MFFilePath.path
+			entry = protowire.AppendString(entry, strings.Repeat("a", tt.pathLen))
+			entry = protowire.AppendTag(entry, 3, protowire.BytesType) // MFFilePath.hashes
+			entry = protowire.AppendBytes(entry, nil)
+			entry = protowire.AppendTag(entry, 301, protowire.BytesType) // MFFilePath.mimeType
+			entry = protowire.AppendBytes(entry, nil)
+			entry = protowire.AppendTag(entry, 302, protowire.BytesType) // MFFilePath.mtime
+			entry = protowire.AppendBytes(entry, nil)
+			entry = protowire.AppendTag(entry, 303, protowire.BytesType) // MFFilePath.ctime
+			entry = protowire.AppendBytes(entry, nil)
+
+			id := uuid.New()
+			inner := protowire.AppendTag(nil, 102, protowire.BytesType) // MFFile.uuid
+			inner = protowire.AppendBytes(inner, id[:])
+
+			for range 1000 {
+				inner = protowire.AppendTag(inner, 101, protowire.BytesType) // MFFile.files
+				inner = protowire.AppendBytes(inner, entry)
+			}
+
+			_, err := NewManifestFromReader(bytes.NewReader(wrapInner(t, id, inner)))
+			if tt.refused {
+				require.ErrorIs(t, err, errDecodedTooLarge)
+			} else {
+				require.NoError(t, err)
+			}
+		})
+	}
+}
+
+// Many empty files with names of at most three characters and modification
+// times at the epoch make about the densest manifest mfer writes: it takes
+// about 7 times its size to decode, and still loads. A signature would not
+// change the inner message, so none is added.
+func TestDeserializeLoadsDensestManifest(t *testing.T) {
+	t.Parallel()
+
+	hash := make([]byte, 34) // multihash: 2-byte prefix + 32-byte SHA-256
+
+	b := NewBuilder()
+	b.SetIncludeTimestamps(true)
+
+	const files = 10000
+	for i := range files {
+		name := RelFilePath(strconv.FormatInt(int64(i), 36))
+		require.NoError(t, b.AddFileWithHash(name, 0, ModTime(time.Unix(0, 0)), hash))
+	}
+
+	var buf bytes.Buffer
+	require.NoError(t, b.Build(context.Background(), &buf))
+
+	m, err := NewManifestFromReader(&buf)
+	require.NoError(t, err)
+	assert.Len(t, m.Files(), files)
+}
+
 func TestDeserializeValidManifestRoundTrips(t *testing.T) {
 	t.Parallel()
 

mfer/testdata/fuzz/FuzzNewManifestFromReader/empty-file-entries

@@ -0,0 +1,2 @@
+go test fuzz v1
+[]byte("ZNAVSRFG\xa8\x06\x01\xb0\x06\x01\xb8\x06\xff\xff\xff\x03\xc2\x06 {\x16\xbdu\xa0\xa2\x11\xfcH\xef*\x1b7\r\x99\xefb\x04\x02g\n\xa9\xf3B5\xe5p\x96\x8c\x8c\xac\x0e\xca\x06\x10\x03Q\xb2\xd0\x19`F\xc1\xb1\xc0Z\xf4x\xf4g^\xba\f\xa1\x06(\xb5/\xfd\x04h\x04\x01\x00d\x01\xb2\x06\x10\x03Q\xb2\xd0\x19`F\xc1\xb1\xc0Z\xf4x\xf4g^\xaa\x06\x00\x01T\x13\x024\xce\xff\rL\x00\x00\x00\x01T\x00\x044\xfd\xff\x15L\x00\x00\x00\x01T\x00\x044\xfd\xff\x15L\x00\x00\x00\x01T\x00\x044\xfd\xff\x15L\x00\x00\x00\x01T\x00\x044\xfd\xff\x15L\x00\x00\x00\x01T\x00\x044\xfd\xff\x15L\x00\x00\x00\x01T\x00\x044\xfd\xff\x15L\x00\x00\x00\x01T\x00\x044\xfd\xff\x15L\x00\x00\x00\x01T\x00\x044\xfd\xff\x15L\x00\x00\x00\x01T\x00\x044\xfd\xff\x15L\x00\x00\x00\x01T\x00\x044\xfd\xff\x15L\x00\x00\x00\x01T\x00\x044\xfd\xff\x15L\x00\x00\x00\x01T\x00\x044\xfd\xff\x15L\x00\x00\x00\x01T\x00\x044\xfd\xff\x15L\x00\x00\x00\x01T\x00\x044\xfd\xff\x15L\x00\x00\x00\x01T\x00\x044\xfd\xff\x15L\x00\x00\x00\x01T\x00\x044\xfd\xff\x15L\x00\x00\x00\x01T\x00\x044\xfd\xff\x15L\x00\x00\x00\x01T\x00\x044\xfd\xff\x15L\x00\x00\x00\x01T\x00\x044\xfd\xff\x15L\x00\x00\x00\x01T\x00\x044\xfd\xff\x15L\x00\x00\x00\x01T\x00\x044\xfd\xff\x15L\x00\x00\x00\x01T\x00\x044\xfd\xff\x15L\x00\x00\x00\x01T\x00\x044\xfd\xff\x15L\x00\x00\x00\x01T\x00\x044\xfd\xff\x15L\x00\x00\x00\x01T\x00\x044\xfd\xff\x15L\x00\x00\x00\x01T\x00\x044\xfd\xff\x15L\x00\x00\x00\x01T\x00\x044\xfd\xff\x15L\x00\x00\x00\x01T\x00\x044\xfd\xff\x15L\x00\x00\x00\x01T\x00\x044\xfd\xff\x15L\x00\x00\x00\x01T\x00\x044\xfd\xff\x15L\x00\x00\x00\x01T\x00\x044\xfd\xff\x15L\x00\x00\x00\x01T\x00\x044\xfd\xff\x15L\x00\x00\x00\x01T\x00\x044\xfd\xff\x15L\x00\x00\x00\x01T\x00\x044\xfd\xff\x15L\x00\x00\x00\x01T\x00\x044\xfd\xff\x15L\x00\x00\x00\x01T\x00\x044\xfd\xff\x15L\x00\x00\x00\x01T\x00\x044\xfd\xff\x15L\x00\x00\x00\x01T\x00\x044\xfd\xff\x15L\x00\x00\x00\x01T\x00\x044\xfd\xff\x15L\x00\x00\x00\x01T\x00\x044\xfd\xff\x15L\x00\x00\x00\x01T\x00\x044\xfd\xff\x15L\x00\x00\x00\x01T\x00\x044\xfd\xff\x15L\x00\x00\x00\x01T\x00\x044\xfd\xff\x15L\x00\x00\x00\x01T\x00\x044\xfd\xff\x15L\x00\x00\x00\x01T\x00\x044\xfd\xff\x15L\x00\x00\x00\x01T\x00\x044\xfd\xff\x15L\x00\x00\x00\x01T\x00\x044\xfd\xff\x15L\x00\x00\x00\x01T\x00\x044\xfd\xff\x15L\x00\x00\x00\x01T\x00\x044\xfd\xff\x15L\x00\x00\x00\x01T\x00\x044\xfd\xff\x15L\x00\x00\x00\x01T\x00\x044\xfd\xff\x15L\x00\x00\x00\x01T\x00\x044\xfd\xff\x15L\x00\x00\x00\x01T\x00\x044\xfd\xff\x15L\x00\x00\x00\x01T\x00\x044\xfd\xff\x15L\x00\x00\x00\x01T\x00\x044\xfd\xff\x15L\x00\x00\x00\x01T\x00\x044\xfd\xff\x15L\x00\x00\x00\x01T\x00\x044\xfd\xff\x15L\x00\x00\x00\x01T\x00\x044\xfd\xff\x15L\x00\x00\x00\x01T\x00\x044\xfd\xff\x15L\x00\x00\x00\x01T\x00\x044\xfd\xff\x15L\x00\x00\x00\x01T\x00\x044\xfd\xff\x15L\x00\x00\x00\x01T\x00\x044\xfd\xff\x15M\x00\x00\x00\x01T\x00\x044\xfc\xff\x153\xea\a\xb4")

mfer/testdata/fuzz/FuzzNewManifestFromReader/file-entry-of-empty-hashes

@@ -0,0 +1,2 @@
+go test fuzz v1
+[]byte("ZNAVSRFG\xa8\x06\x01\xb0\x06\x01\xb8\x06\xff\xff\xff\x03\xc2\x06 .\xcd\x11|0\xfcP\xe5\x1b\xe3\xc6Ӡ\xcdڤx\xcd\x169t\x1a9~ǽB\xc9\xe8G`\x05\xca\x06\x10\x11*!\x0e\x95EF\xb8\xbd\x9f\xde\x12MF\r\x99\xba\f\xa6\x06(\xb5/\xfd\x04h,\x01\x00\xb4\x01\xb2\x06\x10\x11*!\x0e\x95EF\xb8\xbd\x9f\xde\x12MF\r\x99\xaa\x06\xe6\xff\xff\x03\x1a\x00\x01T\x14\x024\x8b\xff\x17L\x00\x00\x00\x01T\x00\x044\xfd\xff\x15L\x00\x00\x00\x01T\x00\x044\xfd\xff\x15L\x00\x00\x00\x01T\x00\x044\xfd\xff\x15L\x00\x00\x00\x01T\x00\x044\xfd\xff\x15L\x00\x00\x00\x01T\x00\x044\xfd\xff\x15L\x00\x00\x00\x01T\x00\x044\xfd\xff\x15L\x00\x00\x00\x01T\x00\x044\xfd\xff\x15L\x00\x00\x00\x01T\x00\x044\xfd\xff\x15L\x00\x00\x00\x01T\x00\x044\xfd\xff\x15L\x00\x00\x00\x01T\x00\x044\xfd\xff\x15L\x00\x00\x00\x01T\x00\x044\xfd\xff\x15L\x00\x00\x00\x01T\x00\x044\xfd\xff\x15L\x00\x00\x00\x01T\x00\x044\xfd\xff\x15L\x00\x00\x00\x01T\x00\x044\xfd\xff\x15L\x00\x00\x00\x01T\x00\x044\xfd\xff\x15L\x00\x00\x00\x01T\x00\x044\xfd\xff\x15L\x00\x00\x00\x01T\x00\x044\xfd\xff\x15L\x00\x00\x00\x01T\x00\x044\xfd\xff\x15L\x00\x00\x00\x01T\x00\x044\xfd\xff\x15L\x00\x00\x00\x01T\x00\x044\xfd\xff\x15L\x00\x00\x00\x01T\x00\x044\xfd\xff\x15L\x00\x00\x00\x01T\x00\x044\xfd\xff\x15L\x00\x00\x00\x01T\x00\x044\xfd\xff\x15L\x00\x00\x00\x01T\x00\x044\xfd\xff\x15L\x00\x00\x00\x01T\x00\x044\xfd\xff\x15L\x00\x00\x00\x01T\x00\x044\xfd\xff\x15L\x00\x00\x00\x01T\x00\x044\xfd\xff\x15L\x00\x00\x00\x01T\x00\x044\xfd\xff\x15L\x00\x00\x00\x01T\x00\x044\xfd\xff\x15L\x00\x00\x00\x01T\x00\x044\xfd\xff\x15L\x00\x00\x00\x01T\x00\x044\xfd\xff\x15L\x00\x00\x00\x01T\x00\x044\xfd\xff\x15L\x00\x00\x00\x01T\x00\x044\xfd\xff\x15L\x00\x00\x00\x01T\x00\x044\xfd\xff\x15L\x00\x00\x00\x01T\x00\x044\xfd\xff\x15L\x00\x00\x00\x01T\x00\x044\xfd\xff\x15L\x00\x00\x00\x01T\x00\x044\xfd\xff\x15L\x00\x00\x00\x01T\x00\x044\xfd\xff\x15L\x00\x00\x00\x01T\x00\x044\xfd\xff\x15L\x00\x00\x00\x01T\x00\x044\xfd\xff\x15L\x00\x00\x00\x01T\x00\x044\xfd\xff\x15L\x00\x00\x00\x01T\x00\x044\xfd\xff\x15L\x00\x00\x00\x01T\x00\x044\xfd\xff\x15L\x00\x00\x00\x01T\x00\x044\xfd\xff\x15L\x00\x00\x00\x01T\x00\x044\xfd\xff\x15L\x00\x00\x00\x01T\x00\x044\xfd\xff\x15L\x00\x00\x00\x01T\x00\x044\xfd\xff\x15L\x00\x00\x00\x01T\x00\x044\xfd\xff\x15L\x00\x00\x00\x01T\x00\x044\xfd\xff\x15L\x00\x00\x00\x01T\x00\x044\xfd\xff\x15L\x00\x00\x00\x01T\x00\x044\xfd\xff\x15L\x00\x00\x00\x01T\x00\x044\xfd\xff\x15L\x00\x00\x00\x01T\x00\x044\xfd\xff\x15L\x00\x00\x00\x01T\x00\x044\xfd\xff\x15L\x00\x00\x00\x01T\x00\x044\xfd\xff\x15L\x00\x00\x00\x01T\x00\x044\xfd\xff\x15L\x00\x00\x00\x01T\x00\x044\xfd\xff\x15L\x00\x00\x00\x01T\x00\x044\xfd\xff\x15L\x00\x00\x00\x01T\x00\x044\xfd\xff\x15L\x00\x00\x00\x01T\x00\x044\xfd\xff\x15L\x00\x00\x00\x01T\x00\x044\xfd\xff\x15L\x00\x00\x00\x01T\x00\x044\xfd\xff\x15M\x00\x00\x00\x01T\x00\x044\xfc\xff\x15\x02\xd1.\xe3")