diff --git a/FORMAT.md b/FORMAT.md index 22e842e..e634a80 100644 --- a/FORMAT.md +++ b/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`) diff --git a/mfer/constants.go b/mfer/constants.go index e8c73b6..519d21b 100644 --- a/mfer/constants.go +++ b/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 ) diff --git a/mfer/deserialize.go b/mfer/deserialize.go index 1fd842f..db0b35d 100644 --- a/mfer/deserialize.go +++ b/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,10 +259,16 @@ 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) - err = proto.Unmarshal(dat, m.pbInner) + // Unknown fields would cost memory; mfer never writes a loaded manifest out. + err = proto.UnmarshalOptions{DiscardUnknown: true}.Unmarshal(dat, m.pbInner) if err != nil { return fmt.Errorf("deserialize: unmarshal inner: %w", err) } @@ -249,7 +337,8 @@ func NewManifestFromReader(input io.Reader) (*manifest, error) { // deserialize outer: m.pbOuter = new(MFFileOuter) - err = proto.Unmarshal(dat, m.pbOuter) + // Unknown fields would cost memory; mfer never writes a loaded manifest out. + err = proto.UnmarshalOptions{DiscardUnknown: true}.Unmarshal(dat, m.pbOuter) if err != nil { return nil, err } diff --git a/mfer/deserialize_fuzz_test.go b/mfer/deserialize_fuzz_test.go index c1e6f43..5767082 100644 --- a/mfer/deserialize_fuzz_test.go +++ b/mfer/deserialize_fuzz_test.go @@ -54,9 +54,15 @@ 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 drops fields it does not know. + // The strings and bytes it copies out of it, such as many one-byte + // values in one hash, take up to about five times more under the + // race detector, which pads every small copy to 16 bytes, and about + // half that without it. 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 +77,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 { diff --git a/mfer/deserialize_path_test.go b/mfer/deserialize_path_test.go index f2abacc..8b7286b 100644 --- a/mfer/deserialize_path_test.go +++ b/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,113 @@ 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) + } + }) + } +} + +// Fields the decoder does not know are dropped, in the outer message, the inner +// message and a file entry, so that they take no memory once loaded. +func TestDeserializeDropsUnknownFields(t *testing.T) { + t.Parallel() + + unknown := protowire.AppendTag(nil, 99, protowire.BytesType) // in no message + unknown = protowire.AppendBytes(unknown, []byte("not known")) + + entry := protowire.AppendTag(nil, 1, protowire.BytesType) // MFFilePath.path + entry = protowire.AppendString(entry, "a") + entry = append(entry, unknown...) + + id := uuid.New() + inner := protowire.AppendTag(nil, 101, protowire.BytesType) // MFFile.files + inner = protowire.AppendBytes(inner, entry) + inner = protowire.AppendTag(inner, 102, protowire.BytesType) // MFFile.uuid + inner = protowire.AppendBytes(inner, id[:]) + inner = append(inner, unknown...) + + data := wrapInner(t, id, inner) + data = append(data, unknown...) // the outer message ends the file + + m, err := NewManifestFromReader(bytes.NewReader(data)) + require.NoError(t, err) + require.Len(t, m.Files(), 1) + assert.Empty(t, m.pbOuter.ProtoReflect().GetUnknown()) + assert.Empty(t, m.pbInner.ProtoReflect().GetUnknown()) + assert.Empty(t, m.Files()[0].ProtoReflect().GetUnknown()) +} + +// 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() diff --git a/mfer/testdata/fuzz/FuzzNewManifestFromReader/empty-file-entries b/mfer/testdata/fuzz/FuzzNewManifestFromReader/empty-file-entries new file mode 100644 index 0000000..b22c95d --- /dev/null +++ b/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") diff --git a/mfer/testdata/fuzz/FuzzNewManifestFromReader/file-entry-of-empty-hashes b/mfer/testdata/fuzz/FuzzNewManifestFromReader/file-entry-of-empty-hashes new file mode 100644 index 0000000..8c6c8c6 --- /dev/null +++ b/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")