sneak/mfer
1
0
Fork 0
Code Issues 28 Pull Requests 5 Actions Releases Wiki Activity

Add fuzz coverage for NewManifestFromReader #65

New Issue
Open
opened 2026-08-09 03:39:10 +02:00 by clawbot · 2 comments
clawbot commented 2026-08-09 03:39:10 +02:00
Collaborator
Copy Link
Copy Source

Context

grep -rn "^func Fuzz" . returns nothing. There is no fuzz coverage
anywhere in the repo and no testdata/fuzz corpus.

NewManifestFromReader is the function that parses attacker-controlled
bytes: a magic prefix, a protobuf outer message, a zlib-compressed inner
payload, and an embedded signature. That is a deserializer for untrusted
input in a tool whose entire purpose is to be pointed at files fetched from
the network. It is the single highest-value fuzz target in the codebase and
currently has none.

Definition of done

  • FuzzNewManifestFromReader exists and exercises the full parse path from
    raw bytes.
  • A seed corpus is committed under mfer/testdata/fuzz/ containing at
    minimum: a valid minimal manifest, a valid signed manifest, truncated
    input at several offsets, correct magic with garbage after it, wrong
    magic, empty input, a zlib bomb (small compressed, huge declared size),
    and a declared size that disagrees with the actual decompressed length.
  • The fuzz target asserts the real invariant: the parser must either return
    a manifest or an error, and must never panic, never hang, and never
    allocate unboundedly.
  • Any crash the fuzzer finds during development is fixed in the same PR, and
    its input is added to the committed corpus as a regression seed.
  • make test runs the seed corpus as ordinary unit tests (Go does this
    automatically for Fuzz* without -fuzz) and stays within the test
    timeout.
  • make check passes. TODO.md updated in the same commit.

Implementation requirements

  • Do not add -fuzz to script/test. Continuous fuzzing must not run in
    CI on every push — the seed corpus regression run is what belongs in
    make test. If a longer fuzz run is worth having, add a separate,
    explicitly-invoked make fuzz target with a bounded -fuzztime, and say
    so in the README Entrypoints section.
  • The target must be fast and deterministic on the seed corpus. No network,
    no gpg subprocess, no real filesystem writes outside t.TempDir().
  • Do not weaken the parser to make the fuzzer pass. If the fuzzer finds
    input that causes a panic or an unbounded allocation, that is a bug in the
    parser and gets fixed as one, with a note in the commit message.
  • Confirm the existing decompression bound (MaxDecompressedSize,
    mfer/constants.go:13) actually holds under fuzzing — the zlib-bomb seed
    is there specifically to test it.
  • Commit title must end with (closes #65).
## Context `grep -rn "^func Fuzz" .` returns nothing. There is no fuzz coverage anywhere in the repo and no `testdata/fuzz` corpus. `NewManifestFromReader` is the function that parses attacker-controlled bytes: a magic prefix, a protobuf outer message, a zlib-compressed inner payload, and an embedded signature. That is a deserializer for untrusted input in a tool whose entire purpose is to be pointed at files fetched from the network. It is the single highest-value fuzz target in the codebase and currently has none. ## Definition of done - `FuzzNewManifestFromReader` exists and exercises the full parse path from raw bytes. - A seed corpus is committed under `mfer/testdata/fuzz/` containing at minimum: a valid minimal manifest, a valid signed manifest, truncated input at several offsets, correct magic with garbage after it, wrong magic, empty input, a zlib bomb (small compressed, huge declared size), and a declared size that disagrees with the actual decompressed length. - The fuzz target asserts the real invariant: the parser must either return a manifest or an error, and must never panic, never hang, and never allocate unboundedly. - Any crash the fuzzer finds during development is fixed in the same PR, and its input is added to the committed corpus as a regression seed. - `make test` runs the seed corpus as ordinary unit tests (Go does this automatically for `Fuzz*` without `-fuzz`) and stays within the test timeout. - `make check` passes. `TODO.md` updated in the same commit. ## Implementation requirements - Do not add `-fuzz` to `script/test`. Continuous fuzzing must not run in CI on every push — the seed corpus regression run is what belongs in `make test`. If a longer fuzz run is worth having, add a separate, explicitly-invoked `make fuzz` target with a bounded `-fuzztime`, and say so in the README Entrypoints section. - The target must be fast and deterministic on the seed corpus. No network, no gpg subprocess, no real filesystem writes outside `t.TempDir()`. - Do not weaken the parser to make the fuzzer pass. If the fuzzer finds input that causes a panic or an unbounded allocation, that is a bug in the parser and gets fixed as one, with a note in the commit message. - Confirm the existing decompression bound (`MaxDecompressedSize`, `mfer/constants.go:13`) actually holds under fuzzing — the zlib-bomb seed is there specifically to test it. - Commit title must end with ` (closes #65)`.
clawbot added this to the 1.0.0 milestone 2026-08-09 03:39:10 +02:00
clawbot commented 2026-10-03 17:47:52 +02:00
Author
Collaborator
Copy Link
Copy Source

Built in #120: the fuzz target, the seed corpus, and make fuzz.

The decompression limit did not hold. The zstd decoder set aside whatever size a frame header claimed, up to its own 64 GiB limit, and decoded small payloads in full before MaxDecompressedSize was checked. The decoder now has MaxDecompressedSize as its limit, and the input that showed it is in the seed corpus.

Model: opus-5-5

Built in https://git.eeqj.de/sneak/mfer/pulls/120: the fuzz target, the seed corpus, and `make fuzz`. The decompression limit did not hold. The zstd decoder set aside whatever size a frame header claimed, up to its own 64 GiB limit, and decoded small payloads in full before `MaxDecompressedSize` was checked. The decoder now has `MaxDecompressedSize` as its limit, and the input that showed it is in the seed corpus. Model: opus-5-5
clawbot added the critical label 2026-10-03 17:58:37 +02:00
clawbot commented 2026-10-03 17:58:37 +02:00
Author
Collaborator
Copy Link
Copy Source

Labelled critical: the fuzzing for this issue found that a 92-byte manifest makes any mfer command that reads it (check, fetch, list) try to allocate 8 GiB, so anyone who opens a manifest from an untrusted source can have mfer run the machine out of memory; the fix is in #120.

Model: opus-5-5

Labelled critical: the fuzzing for this issue found that a 92-byte manifest makes any `mfer` command that reads it (`check`, `fetch`, `list`) try to allocate 8 GiB, so anyone who opens a manifest from an untrusted source can have `mfer` run the machine out of memory; the fix is in https://git.eeqj.de/sneak/mfer/pulls/120. Model: opus-5-5
Sign in to join this conversation.
No Branch/Tag Specified
Branches Tags
Labels
Clear labels
critical
Extremely important: data loss, a security hole, production down, or basic use impossible
 merge-ready
All checks pass, reviewed, rebased, ready for merge
 needs-checks
make check does not pass cleanly
 needs-rebase
PR has merge conflicts or is behind main
 needs-review
Code review not yet done
 needs-rework
Code review found issues to fix
No labels critical
Milestone
No items
No Milestone 1.0.0
Assignees
Clear assignees
clawbot sneak (Jeffrey Paul)
No Assignees
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: sneak/mfer#65