MaxDecompressedSize (256 MiB, mfer/constants.go) bounds the decompressed inner message, but not what proto.Unmarshal allocates for it in deserializeInner (mfer/deserialize.go). An empty file entry is 3 bytes encoded and about 170 bytes decoded, and every entry is decoded before any path is checked.
Measured while reworking #120: a 174-byte manifest with a valid hash, whose payload decompresses to 768 KiB of empty file entries, makes NewManifestFromReader allocate 46 MB. At MaxDecompressedSize of such entries that is about 15 GiB, from a manifest of roughly 25 KB (both extrapolated, not run). Any command that reads a manifest (check, fetch, list) can be run out of memory that way, the same kind of problem as #65.
Definition of done
A manifest whose decoded entries would take much more memory than its decompressed size is rejected before that memory is allocated, with a regression test or seed showing it.
make check passes; TODO.md updated in the same commit.
Model: opus-5-5
## Context
`MaxDecompressedSize` (256 MiB, `mfer/constants.go`) bounds the decompressed inner message, but not what `proto.Unmarshal` allocates for it in `deserializeInner` (`mfer/deserialize.go`). An empty file entry is 3 bytes encoded and about 170 bytes decoded, and every entry is decoded before any path is checked.
Measured while reworking https://git.eeqj.de/sneak/mfer/pulls/120: a 174-byte manifest with a valid hash, whose payload decompresses to 768 KiB of empty file entries, makes `NewManifestFromReader` allocate 46 MB. At `MaxDecompressedSize` of such entries that is about 15 GiB, from a manifest of roughly 25 KB (both extrapolated, not run). Any command that reads a manifest (`check`, `fetch`, `list`) can be run out of memory that way, the same kind of problem as https://git.eeqj.de/sneak/mfer/issues/65.
## Definition of done
- A manifest whose decoded entries would take much more memory than its decompressed size is rejected before that memory is allocated, with a regression test or seed showing it.
- `make check` passes; `TODO.md` updated in the same commit.
Model: opus-5-5
Plan (labelled critical: the same kind of problem as #65, now fixed on next).
The format already says every file entry has a non-empty path and at least one hash, so no valid entry is as small as the 3-byte empty entries that blow up 60 times when decoded. Before proto.Unmarshal in deserializeInner, the reader walks the inner message's top-level fields with protowire (already a dependency) and rejects the manifest if any file entry is shorter than the smallest entry the format allows, or if the entries could not fit in the decompressed size at that minimum. What an entry may then cost when decoded is bounded by a small multiple of its encoded size. A regression seed (or test) with many empty entries shows it, and the fuzz target's allocation ceiling is checked against it. No TODO.md entry (removed by #76).
Model: opus-5-5
Plan (labelled critical: the same kind of problem as https://git.eeqj.de/sneak/mfer/issues/65, now fixed on `next`).
The format already says every file entry has a non-empty path and at least one hash, so no valid entry is as small as the 3-byte empty entries that blow up 60 times when decoded. Before `proto.Unmarshal` in `deserializeInner`, the reader walks the inner message's top-level fields with `protowire` (already a dependency) and rejects the manifest if any file entry is shorter than the smallest entry the format allows, or if the entries could not fit in the decompressed size at that minimum. What an entry may then cost when decoded is bounded by a small multiple of its encoded size. A regression seed (or test) with many empty entries shows it, and the fuzz target's allocation ceiling is checked against it. No `TODO.md` entry (removed by https://git.eeqj.de/sneak/mfer/issues/76).
Model: opus-5-5
clawbot
self-assigned this 2026-10-04 05:32:16 +02:00
Built in #128: before decoding, the parser rejects a manifest holding a file entry or hash shorter than the smallest the format allows. Hashes inside each entry are checked as well as the entries, since one entry of many empty hashes decodes as large as many empty entries.
Model: opus-5-5
Built in https://git.eeqj.de/sneak/mfer/pulls/128: before decoding, the parser rejects a manifest holding a file entry or hash shorter than the smallest the format allows. Hashes inside each entry are checked as well as the entries, since one entry of many empty hashes decodes as large as many empty entries.
Model: opus-5-5
Blocking a user prevents them from interacting with repositories, such as opening or commenting on pull requests or issues. Learn more about blocking a user.
Context
MaxDecompressedSize(256 MiB,mfer/constants.go) bounds the decompressed inner message, but not whatproto.Unmarshalallocates for it indeserializeInner(mfer/deserialize.go). An empty file entry is 3 bytes encoded and about 170 bytes decoded, and every entry is decoded before any path is checked.Measured while reworking #120: a 174-byte manifest with a valid hash, whose payload decompresses to 768 KiB of empty file entries, makes
NewManifestFromReaderallocate 46 MB. AtMaxDecompressedSizeof such entries that is about 15 GiB, from a manifest of roughly 25 KB (both extrapolated, not run). Any command that reads a manifest (check,fetch,list) can be run out of memory that way, the same kind of problem as #65.Definition of done
make checkpasses;TODO.mdupdated in the same commit.Model: opus-5-5
Plan (labelled critical: the same kind of problem as #65, now fixed on
next).The format already says every file entry has a non-empty path and at least one hash, so no valid entry is as small as the 3-byte empty entries that blow up 60 times when decoded. Before
proto.UnmarshalindeserializeInner, the reader walks the inner message's top-level fields withprotowire(already a dependency) and rejects the manifest if any file entry is shorter than the smallest entry the format allows, or if the entries could not fit in the decompressed size at that minimum. What an entry may then cost when decoded is bounded by a small multiple of its encoded size. A regression seed (or test) with many empty entries shows it, and the fuzz target's allocation ceiling is checked against it. NoTODO.mdentry (removed by #76).Model: opus-5-5
Built in #128: before decoding, the parser rejects a manifest holding a file entry or hash shorter than the smallest the format allows. Hashes inside each entry are checked as well as the entries, since one entry of many empty hashes decodes as large as many empty entries.
Model: opus-5-5