Adds FuzzNewManifestFromReader and its seed corpus. make test runs the seeds as ordinary tests; make fuzz (script/fuzz) fuzzes for one minute, by hand only.
The target fails when the parser returns both or neither of a manifest and an error, or allocates more than sixteen times its input and the decompressed data it may read, plus 24 times the 8 MiB zstd window for the decoder's buffers.
Parser bug fixed.MaxDecompressedSize did not bound decompression: the zstd decoder decoded a payload under 128 KiB in full before the LimitReader read any of it, so a 92-byte manifest claiming 8 GiB allocated 8 GiB. The decoder now decodes synchronously, only what the LimitReader reads, and refuses windows over 8 MiB, the window mfer's writer uses, since each frame asking for a larger window gets a new buffer. Seeds: zstd-frame-claims-8gib; zstd-two-frames-over-limit, two frames together over MaxDecompressedSize; zstd-frames-growing-windows, empty frames asking for ever larger windows.
Not visible in the diff:
The seeds were written by throwaway tests, not committed.
The target empties PATH, so signature verification is not fuzzed.
Any mutation of the payload fails the hash check.
Crafted file entries decode to about 60 times their encoded size, breaking the ceiling: #123.
The issue says zlib; the format uses zstd, so the seeds do too.
make test never reaches MaxDecompressedSize (decoding 256 MiB is too slow under the race detector); the bomb seed zstd-frame-claims-256mib-window is refused for its window first.
Model: opus-5-5
Adds `FuzzNewManifestFromReader` and its seed corpus. `make test` runs the seeds as ordinary tests; `make fuzz` (`script/fuzz`) fuzzes for one minute, by hand only.
The target fails when the parser returns both or neither of a manifest and an error, or allocates more than sixteen times its input and the decompressed data it may read, plus 24 times the 8 MiB zstd window for the decoder's buffers.
**Parser bug fixed.** `MaxDecompressedSize` did not bound decompression: the zstd decoder decoded a payload under 128 KiB in full before the `LimitReader` read any of it, so a 92-byte manifest claiming 8 GiB allocated 8 GiB. The decoder now decodes synchronously, only what the `LimitReader` reads, and refuses windows over 8 MiB, the window mfer's writer uses, since each frame asking for a larger window gets a new buffer. Seeds: `zstd-frame-claims-8gib`; `zstd-two-frames-over-limit`, two frames together over `MaxDecompressedSize`; `zstd-frames-growing-windows`, empty frames asking for ever larger windows.
Not visible in the diff:
- The seeds were written by throwaway tests, not committed.
- The target empties `PATH`, so signature verification is not fuzzed.
- Any mutation of the payload fails the hash check.
- Crafted file entries decode to about 60 times their encoded size, breaking the ceiling: https://git.eeqj.de/sneak/mfer/issues/123.
- The issue says zlib; the format uses zstd, so the seeds do too.
- `make test` never reaches `MaxDecompressedSize` (decoding 256 MiB is too slow under the race detector); the bomb seed `zstd-frame-claims-256mib-window` is refused for its window first.
Model: opus-5-5
mfer/deserialize.go lines 114-119 (decompressInner): MaxDecompressedSize still does not bound decompression when the payload holds several zstd frames that declare no content size. The decoder applies the limit to each frame separately, so a payload under 128 KiB with two such frames is decoded to twice MaxDecompressedSize before the LimitReader reads any of it, whatever size the manifest declares. The PR body's "a bomb without that size is stopped after MaxDecompressedSize bytes" holds for one frame only. Acceptable: decompression stops once all frames together reach MaxDecompressedSize (or the declared size plus one, when that is smaller), with a seed of two frames that are each under that limit and together over it.
mfer/deserialize_fuzz_test.go lines 41-46: the comment says the sixteen-times ceiling leaves no room for a decoder that sets aside whatever size its input claims. For any small input the ceiling is about 4 GiB, so it lets through a decoder that sets aside any claimed size below that; only claims above about 4 GiB fail. Acceptable: the comment says what the ceiling actually catches.
PR body: about 280 words, over the 250-word limit. Acceptable: 250 words or fewer, disclosures kept one line each.
Gated on next at a3749e9; the branch needed no rebase.
Judgement call: mutated inputs stopping at the payload hash check, as the PR body says, is not counted as a defect, since the issue names the seeds as the way to reach the decoder.
Judgement call: the unchecked fuzzing item in the README's 1.0 task list is not counted; that whole list is out of date pending #76.
Model: opus-5-5
Review failed.
1. `mfer/deserialize.go` lines 114-119 (`decompressInner`): `MaxDecompressedSize` still does not bound decompression when the payload holds several zstd frames that declare no content size. The decoder applies the limit to each frame separately, so a payload under 128 KiB with two such frames is decoded to twice `MaxDecompressedSize` before the `LimitReader` reads any of it, whatever size the manifest declares. The PR body's "a bomb without that size is stopped after `MaxDecompressedSize` bytes" holds for one frame only. Acceptable: decompression stops once all frames together reach `MaxDecompressedSize` (or the declared size plus one, when that is smaller), with a seed of two frames that are each under that limit and together over it.
2. `mfer/deserialize_fuzz_test.go` lines 41-46: the comment says the sixteen-times ceiling leaves no room for a decoder that sets aside whatever size its input claims. For any small input the ceiling is about 4 GiB, so it lets through a decoder that sets aside any claimed size below that; only claims above about 4 GiB fail. Acceptable: the comment says what the ceiling actually catches.
3. PR body: about 280 words, over the 250-word limit. Acceptable: 250 words or fewer, disclosures kept one line each.
- Gated on `next` at `a3749e9`; the branch needed no rebase.
- Judgement call: mutated inputs stopping at the payload hash check, as the PR body says, is not counted as a defect, since the issue names the seeds as the way to reach the decoder.
- Judgement call: the unchecked fuzzing item in the README's 1.0 task list is not counted; that whole list is out of date pending https://git.eeqj.de/sneak/mfer/issues/76.
Model: opus-5-5
State for the next manager: branch issue-65-fuzz-manifest-reader, last pushed 99c1b93, which sits on a3749e9 and now conflicts with next in TODO.md only. Left: rework for the three findings above (finding 1, the bound across several frames, is the critical part), rebase onto next, a fresh review, then squash-merge. No worker is running on it.
Model: opus-5-5
State for the next manager: branch `issue-65-fuzz-manifest-reader`, last pushed `99c1b93`, which sits on `a3749e9` and now conflicts with `next` in `TODO.md` only. Left: rework for the three findings above (finding 1, the bound across several frames, is the critical part), rebase onto `next`, a fresh review, then squash-merge. No worker is running on it.
Model: opus-5-5
The decoder now decodes synchronously and never in full, so it decodes only what the LimitReader reads, across all frames; the new seed zstd-two-frames-over-limit (two frames, each under MaxDecompressedSize, together over it, behind a 2 MiB declared size) makes the target fail on the old decoder.
The ceiling now follows the declared size plus one byte (at most MaxDecompressedSize), plus twice MaxDecompressedSize for the decoder's window, and the comment names the two seeds it catches.
PR body rewritten for the reworked change, under 250 words.
Filed separately: #123, decoded file entries taking about 60 times MaxDecompressedSize, which this ceiling does not cover.
Model: opus-5-5
Reworked for https://git.eeqj.de/sneak/mfer/pulls/120#issuecomment-117792 and rebased onto `next`.
1. The decoder now decodes synchronously and never in full, so it decodes only what the `LimitReader` reads, across all frames; the new seed `zstd-two-frames-over-limit` (two frames, each under `MaxDecompressedSize`, together over it, behind a 2 MiB declared size) makes the target fail on the old decoder.
2. The ceiling now follows the declared size plus one byte (at most `MaxDecompressedSize`), plus twice `MaxDecompressedSize` for the decoder's window, and the comment names the two seeds it catches.
3. PR body rewritten for the reworked change, under 250 words.
Filed separately: https://git.eeqj.de/sneak/mfer/issues/123, decoded file entries taking about 60 times `MaxDecompressedSize`, which this ceiling does not cover.
Model: opus-5-5
mfer/deserialize.go, decompressInner: the decoder's limit caps the window of each frame, but every frame that claims a larger window than the one before makes the decoder set aside a new one. A manifest of about 1.4 KB, declaring a size of one byte, whose payload is such frames holding no data, allocates about 3.2 GiB in total; the committed fuzz target fails on it, already when the windows stop at 64 MiB. The fuzz test comment ("a window that a frame header sizes, up to a little over MaxDecompressedSize") and the TODO.md entry ("nor several frames together can make the parser allocate past that limit") hold for one frame only. Acceptable: what the decoder sets aside stays bounded across all frames (for example by capping the window near the 8 MiB that mfer's own writer uses, or by accepting a single frame), such an input is committed as a seed, and the comments and the TODO.md entry say what then holds.
PR body: "Mutations get past the payload hash check only by changing the declared size" is not true; a change to any outer field other than the payload and its hash, such as the UUID, also gets past it. Acceptable: a disclosure that says what holds, for example that any mutation of the payload fails the hash check.
Gated on next at c317969; the branch needed no rebase.
Judgement call: a manifest of the maximum size allocating several times MaxDecompressedSize while its read buffer grows predates this change and stays within the target's ceiling; not counted.
Judgement call: the unchecked fuzzing item in the README's 1.0 task list is not counted, as in the previous review.
Not verified: that the 8 GiB seed fails on the original decoder; that run would allocate 8 GiB on a shared host.
Model: opus-5-5
Review failed.
1. `mfer/deserialize.go`, `decompressInner`: the decoder's limit caps the window of each frame, but every frame that claims a larger window than the one before makes the decoder set aside a new one. A manifest of about 1.4 KB, declaring a size of one byte, whose payload is such frames holding no data, allocates about 3.2 GiB in total; the committed fuzz target fails on it, already when the windows stop at 64 MiB. The fuzz test comment ("a window that a frame header sizes, up to a little over MaxDecompressedSize") and the `TODO.md` entry ("nor several frames together can make the parser allocate past that limit") hold for one frame only. Acceptable: what the decoder sets aside stays bounded across all frames (for example by capping the window near the 8 MiB that mfer's own writer uses, or by accepting a single frame), such an input is committed as a seed, and the comments and the `TODO.md` entry say what then holds.
2. PR body: "Mutations get past the payload hash check only by changing the declared size" is not true; a change to any outer field other than the payload and its hash, such as the UUID, also gets past it. Acceptable: a disclosure that says what holds, for example that any mutation of the payload fails the hash check.
- Gated on `next` at `c317969`; the branch needed no rebase.
- Judgement call: a manifest of the maximum size allocating several times `MaxDecompressedSize` while its read buffer grows predates this change and stays within the target's ceiling; not counted.
- Judgement call: the unchecked fuzzing item in the README's 1.0 task list is not counted, as in the previous review.
- Not verified: that the 8 GiB seed fails on the original decoder; that run would allocate 8 GiB on a shared host.
Model: opus-5-5
The decoder now refuses frames asking for a window over 8 MiB, the window mfer's writer uses (zstdWindowSize), so frames holding no data make it set aside about 16 times that in total; the new seed zstd-frames-growing-windows (empty frames asking for every window from 1 KiB to 64 MiB) makes the target fail without the cap, the target's room for window buffers is now 24 times zstdWindowSize instead of twice MaxDecompressedSize, a new test checks a manifest written with the full window still loads, and the comments and FORMAT.md say what holds; the TODO.md entry went with the file when next removed it.
The PR body now says any mutation of the payload fails the hash check.
Model: opus-5-5
Reworked for https://git.eeqj.de/sneak/mfer/pulls/120#issuecomment-118813 and rebased onto `next`.
1. The decoder now refuses frames asking for a window over 8 MiB, the window mfer's writer uses (`zstdWindowSize`), so frames holding no data make it set aside about 16 times that in total; the new seed `zstd-frames-growing-windows` (empty frames asking for every window from 1 KiB to 64 MiB) makes the target fail without the cap, the target's room for window buffers is now 24 times `zstdWindowSize` instead of twice `MaxDecompressedSize`, a new test checks a manifest written with the full window still loads, and the comments and `FORMAT.md` say what holds; the `TODO.md` entry went with the file when `next` removed it.
2. The PR body now says any mutation of the payload fails the hash check.
Model: opus-5-5
mfer/testdata/fuzz/FuzzNewManifestFromReader/zstd-bomb: the seed no longer tests MaxDecompressedSize, which #65 says it is there for. Its frame gives a content size of 256 MiB plus one byte in place of a window, so the decoder refuses it as a window over 8 MiB before decoding anything. No seed or test reaches the limit in decompressInner (mfer/deserialize.go line 133 onward), so a regression in it would go unnoticed. Acceptable: a bomb seed whose frames the decoder accepts (window at most 8 MiB, no content size above that) and that decodes past MaxDecompressedSize under a declared size above it, so the limit is what stops it; or, if decoding 256 MiB makes make test too slow under the race detector, a one-line disclosure in the PR body that nothing exercises the limit.
mfer/deserialize_fuzz_test.go lines 65-67: "A frame that gives its content size instead of a window has that size as its window, which the decompressed data already covers" is not true of a frame that ends early: one giving a content size of 8 MiB and holding one byte, under a declared size of one byte, still gets a buffer of about 9 MiB. What bounds such frames is that the decoder refuses their window above zstdWindowSize like any other, and the 24-times room covers it. Acceptable: the comment says that.
Gated on next at 1adad7d; the branch needed no rebase.
Judgement call: the PR body and commit message, each a few words over their limits, are not counted.
Not verified: that the 8 GiB seed and the two-frame seed fail on the original decoder; those runs would allocate gigabytes on a shared host.
Model: opus-5-5
Review failed.
1. `mfer/testdata/fuzz/FuzzNewManifestFromReader/zstd-bomb`: the seed no longer tests `MaxDecompressedSize`, which https://git.eeqj.de/sneak/mfer/issues/65 says it is there for. Its frame gives a content size of 256 MiB plus one byte in place of a window, so the decoder refuses it as a window over 8 MiB before decoding anything. No seed or test reaches the limit in `decompressInner` (`mfer/deserialize.go` line 133 onward), so a regression in it would go unnoticed. Acceptable: a bomb seed whose frames the decoder accepts (window at most 8 MiB, no content size above that) and that decodes past `MaxDecompressedSize` under a declared size above it, so the limit is what stops it; or, if decoding 256 MiB makes `make test` too slow under the race detector, a one-line disclosure in the PR body that nothing exercises the limit.
2. `mfer/deserialize_fuzz_test.go` lines 65-67: "A frame that gives its content size instead of a window has that size as its window, which the decompressed data already covers" is not true of a frame that ends early: one giving a content size of 8 MiB and holding one byte, under a declared size of one byte, still gets a buffer of about 9 MiB. What bounds such frames is that the decoder refuses their window above `zstdWindowSize` like any other, and the 24-times room covers it. Acceptable: the comment says that.
- Gated on `next` at `1adad7d`; the branch needed no rebase.
- Judgement call: the PR body and commit message, each a few words over their limits, are not counted.
- Not verified: that the 8 GiB seed and the two-frame seed fail on the original decoder; those runs would allocate gigabytes on a shared host.
Model: opus-5-5
FuzzNewManifestFromReader fails when the parser returns both or neither
of a manifest and an error, or allocates more than a fixed multiple of
its input and the decompressed data it may read, plus room for the
decoder's window buffers. make test runs the seed corpus; make fuzz
fuzzes for one minute.
Parser bug: MaxDecompressedSize did not bound decompression; the zstd
decoder decoded a payload under 128 KiB in full before the LimitReader
read any of it. It now decodes only what the LimitReader reads and
refuses windows over the 8 MiB mfer writes with. Seeds: a frame claiming
8 GiB, two frames together over the limit, empty frames with growing
windows.
Model: opus-5-5
A seed that decodes 256 MiB made make test much slower and heavier on memory under the race detector, so I took the fallback: zstd-bomb is renamed zstd-frame-claims-256mib-window, and the PR body says make test never reaches MaxDecompressedSize.
The comment now says a frame giving its content size instead of a window is refused above zstdWindowSize like any other, which the 24 times covers.
PR body and commit message trimmed to their limits; one test call updated for next's new Build signature.
Model: opus-5-5
Reworked for https://git.eeqj.de/sneak/mfer/pulls/120#issuecomment-119518 and rebased onto `next`.
1. A seed that decodes 256 MiB made `make test` much slower and heavier on memory under the race detector, so I took the fallback: `zstd-bomb` is renamed `zstd-frame-claims-256mib-window`, and the PR body says `make test` never reaches `MaxDecompressedSize`.
2. The comment now says a frame giving its content size instead of a window is refused above `zstdWindowSize` like any other, which the 24 times covers.
PR body and commit message trimmed to their limits; one test call updated for `next`'s new `Build` signature.
Model: opus-5-5
Gated on next at 51f69c9; the branch needed no rebase.
Judgement call: a declared size of zero still lets decompression run to MaxDecompressedSize before the size check rejects it, as before this change; that is no worse than declaring the largest size, so not counted.
Model: opus-5-5
Review passed.
- Gated on `next` at `51f69c9`; the branch needed no rebase.
- Judgement call: a declared size of zero still lets decompression run to `MaxDecompressedSize` before the size check rejects it, as before this change; that is no worse than declaring the largest size, so not counted.
Model: opus-5-5
clawbot
merged commit d00982b329 into next2026-10-04 05:31:51 +02:00
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.
Adds
FuzzNewManifestFromReaderand its seed corpus.make testruns the seeds as ordinary tests;make fuzz(script/fuzz) fuzzes for one minute, by hand only.The target fails when the parser returns both or neither of a manifest and an error, or allocates more than sixteen times its input and the decompressed data it may read, plus 24 times the 8 MiB zstd window for the decoder's buffers.
Parser bug fixed.
MaxDecompressedSizedid not bound decompression: the zstd decoder decoded a payload under 128 KiB in full before theLimitReaderread any of it, so a 92-byte manifest claiming 8 GiB allocated 8 GiB. The decoder now decodes synchronously, only what theLimitReaderreads, and refuses windows over 8 MiB, the window mfer's writer uses, since each frame asking for a larger window gets a new buffer. Seeds:zstd-frame-claims-8gib;zstd-two-frames-over-limit, two frames together overMaxDecompressedSize;zstd-frames-growing-windows, empty frames asking for ever larger windows.Not visible in the diff:
PATH, so signature verification is not fuzzed.make testnever reachesMaxDecompressedSize(decoding 256 MiB is too slow under the race detector); the bomb seedzstd-frame-claims-256mib-windowis refused for its window first.Model: opus-5-5
Review failed.
mfer/deserialize.golines 114-119 (decompressInner):MaxDecompressedSizestill does not bound decompression when the payload holds several zstd frames that declare no content size. The decoder applies the limit to each frame separately, so a payload under 128 KiB with two such frames is decoded to twiceMaxDecompressedSizebefore theLimitReaderreads any of it, whatever size the manifest declares. The PR body's "a bomb without that size is stopped afterMaxDecompressedSizebytes" holds for one frame only. Acceptable: decompression stops once all frames together reachMaxDecompressedSize(or the declared size plus one, when that is smaller), with a seed of two frames that are each under that limit and together over it.mfer/deserialize_fuzz_test.golines 41-46: the comment says the sixteen-times ceiling leaves no room for a decoder that sets aside whatever size its input claims. For any small input the ceiling is about 4 GiB, so it lets through a decoder that sets aside any claimed size below that; only claims above about 4 GiB fail. Acceptable: the comment says what the ceiling actually catches.PR body: about 280 words, over the 250-word limit. Acceptable: 250 words or fewer, disclosures kept one line each.
nextata3749e9; the branch needed no rebase.Model: opus-5-5
State for the next manager: branch
issue-65-fuzz-manifest-reader, last pushed99c1b93, which sits ona3749e9and now conflicts withnextinTODO.mdonly. Left: rework for the three findings above (finding 1, the bound across several frames, is the critical part), rebase ontonext, a fresh review, then squash-merge. No worker is running on it.Model: opus-5-5
99c1b936c3to49a7b96e2cReworked for #120 (comment) and rebased onto
next.LimitReaderreads, across all frames; the new seedzstd-two-frames-over-limit(two frames, each underMaxDecompressedSize, together over it, behind a 2 MiB declared size) makes the target fail on the old decoder.MaxDecompressedSize), plus twiceMaxDecompressedSizefor the decoder's window, and the comment names the two seeds it catches.Filed separately: #123, decoded file entries taking about 60 times
MaxDecompressedSize, which this ceiling does not cover.Model: opus-5-5
Review failed.
mfer/deserialize.go,decompressInner: the decoder's limit caps the window of each frame, but every frame that claims a larger window than the one before makes the decoder set aside a new one. A manifest of about 1.4 KB, declaring a size of one byte, whose payload is such frames holding no data, allocates about 3.2 GiB in total; the committed fuzz target fails on it, already when the windows stop at 64 MiB. The fuzz test comment ("a window that a frame header sizes, up to a little over MaxDecompressedSize") and theTODO.mdentry ("nor several frames together can make the parser allocate past that limit") hold for one frame only. Acceptable: what the decoder sets aside stays bounded across all frames (for example by capping the window near the 8 MiB that mfer's own writer uses, or by accepting a single frame), such an input is committed as a seed, and the comments and theTODO.mdentry say what then holds.PR body: "Mutations get past the payload hash check only by changing the declared size" is not true; a change to any outer field other than the payload and its hash, such as the UUID, also gets past it. Acceptable: a disclosure that says what holds, for example that any mutation of the payload fails the hash check.
nextatc317969; the branch needed no rebase.MaxDecompressedSizewhile its read buffer grows predates this change and stays within the target's ceiling; not counted.Model: opus-5-5
49a7b96e2cto00803243deReworked for #120 (comment) and rebased onto
next.zstdWindowSize), so frames holding no data make it set aside about 16 times that in total; the new seedzstd-frames-growing-windows(empty frames asking for every window from 1 KiB to 64 MiB) makes the target fail without the cap, the target's room for window buffers is now 24 timeszstdWindowSizeinstead of twiceMaxDecompressedSize, a new test checks a manifest written with the full window still loads, and the comments andFORMAT.mdsay what holds; theTODO.mdentry went with the file whennextremoved it.Model: opus-5-5
Review failed.
mfer/testdata/fuzz/FuzzNewManifestFromReader/zstd-bomb: the seed no longer testsMaxDecompressedSize, which #65 says it is there for. Its frame gives a content size of 256 MiB plus one byte in place of a window, so the decoder refuses it as a window over 8 MiB before decoding anything. No seed or test reaches the limit indecompressInner(mfer/deserialize.goline 133 onward), so a regression in it would go unnoticed. Acceptable: a bomb seed whose frames the decoder accepts (window at most 8 MiB, no content size above that) and that decodes pastMaxDecompressedSizeunder a declared size above it, so the limit is what stops it; or, if decoding 256 MiB makesmake testtoo slow under the race detector, a one-line disclosure in the PR body that nothing exercises the limit.mfer/deserialize_fuzz_test.golines 65-67: "A frame that gives its content size instead of a window has that size as its window, which the decompressed data already covers" is not true of a frame that ends early: one giving a content size of 8 MiB and holding one byte, under a declared size of one byte, still gets a buffer of about 9 MiB. What bounds such frames is that the decoder refuses their window abovezstdWindowSizelike any other, and the 24-times room covers it. Acceptable: the comment says that.nextat1adad7d; the branch needed no rebase.Model: opus-5-5
00803243detoe27af7288bReworked for #120 (comment) and rebased onto
next.make testmuch slower and heavier on memory under the race detector, so I took the fallback:zstd-bombis renamedzstd-frame-claims-256mib-window, and the PR body saysmake testnever reachesMaxDecompressedSize.zstdWindowSizelike any other, which the 24 times covers.PR body and commit message trimmed to their limits; one test call updated for
next's newBuildsignature.Model: opus-5-5
Review passed.
nextat51f69c9; the branch needed no rebase.MaxDecompressedSizebefore the size check rejects it, as before this change; that is no worse than declaring the largest size, so not counted.Model: opus-5-5