Fuzz NewManifestFromReader and cap the zstd decoder (closes #65)
check / check (push) Successful in 1m46s
check / check (push) Successful in 1m46s
FuzzNewManifestFromReader 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 room for the decoder's window. make test runs the committed seed corpus; make fuzz fuzzes for one minute, by hand only. Parser bug: MaxDecompressedSize did not bound decompression. The zstd decoder decoded a payload under 128 KiB in full, each frame up to its own 64 GiB limit, before the LimitReader read any of it. It now decodes synchronously, only what the LimitReader reads, with MaxDecompressedSize as its limit. Regression seeds: a frame claiming 8 GiB, and two frames each under the limit and together over it. Model: opus-5-5
This commit is contained in:
@@ -13,7 +13,7 @@ GOLDFLAGS += -X main.Version=$(VERSION)
|
|||||||
GOLDFLAGS += -X main.Gitrev=$(GITREV_BUILD)
|
GOLDFLAGS += -X main.Gitrev=$(GITREV_BUILD)
|
||||||
GOFLAGS := -ldflags "$(GOLDFLAGS)"
|
GOFLAGS := -ldflags "$(GOLDFLAGS)"
|
||||||
|
|
||||||
.PHONY: bootstrap setup docker default run ci test check lint fmt fmt-check fmt-check-go fmt-check-md hooks fixme
|
.PHONY: bootstrap setup docker default run ci test fuzz check lint fmt fmt-check fmt-check-go fmt-check-md hooks fixme
|
||||||
|
|
||||||
default: fmt test
|
default: fmt test
|
||||||
|
|
||||||
@@ -32,6 +32,9 @@ ci: test
|
|||||||
test:
|
test:
|
||||||
@script/test
|
@script/test
|
||||||
|
|
||||||
|
fuzz:
|
||||||
|
@script/fuzz
|
||||||
|
|
||||||
$(PROTOC_GEN_GO):
|
$(PROTOC_GEN_GO):
|
||||||
test -e $(PROTOC_GEN_GO) || go install -v google.golang.org/protobuf/cmd/protoc-gen-go@v1.28.1
|
test -e $(PROTOC_GEN_GO) || go install -v google.golang.org/protobuf/cmd/protoc-gen-go@v1.28.1
|
||||||
|
|
||||||
|
|||||||
@@ -44,6 +44,9 @@ provide:
|
|||||||
such as `script/docker`
|
such as `script/docker`
|
||||||
- `script/test` — run the test suite (`go test`), regenerating the protobuf code
|
- `script/test` — run the test suite (`go test`), regenerating the protobuf code
|
||||||
first if it is stale
|
first if it is stale
|
||||||
|
- `script/fuzz` — fuzz the manifest parser for one minute; run by hand
|
||||||
|
(`make fuzz`), never by CI, while `script/test` runs its committed seed corpus
|
||||||
|
as ordinary tests
|
||||||
- `script/lint` — run `golangci-lint` and verify `gofmt` cleanliness
|
- `script/lint` — run `golangci-lint` and verify `gofmt` cleanliness
|
||||||
- `script/fmt` — format all code and docs (writes): `gofumpt`,
|
- `script/fmt` — format all code and docs (writes): `gofumpt`,
|
||||||
`golangci-lint run --fix`, and `script/prettier --write`
|
`golangci-lint run --fix`, and `script/prettier --write`
|
||||||
|
|||||||
@@ -24,6 +24,11 @@ only thing left of the `chore/align-repo-policies` branch is the list below.
|
|||||||
|
|
||||||
# Completed Steps
|
# Completed Steps
|
||||||
|
|
||||||
|
- 2026-10-03: added `FuzzNewManifestFromReader` and its seed corpus, which
|
||||||
|
`make test` runs, plus `make fuzz` for a one-minute run by hand; the zstd
|
||||||
|
decoder now decodes only as many bytes as the parser reads, and has
|
||||||
|
`MaxDecompressedSize` as its limit, so neither a frame claiming a large size
|
||||||
|
nor several frames together can make the parser allocate past that limit (#65)
|
||||||
- 2026-10-03: pinned the CLI error messages by driving the functions that emit
|
- 2026-10-03: pinned the CLI error messages by driving the functions that emit
|
||||||
them in `internal/cli/errmsg_test.go`, and made the freshen mtime-presence
|
them in `internal/cli/errmsg_test.go`, and made the freshen mtime-presence
|
||||||
test distinguish an absent mtime from the epoch (#87)
|
test distinguish an absent mtime from the epoch (#87)
|
||||||
@@ -126,7 +131,7 @@ only thing left of the `chore/align-repo-policies` branch is the list below.
|
|||||||
rate-limit Checker progress output; add --deterministic flag or default;
|
rate-limit Checker progress output; add --deterministic flag or default;
|
||||||
wire top-level --version properly
|
wire top-level --version properly
|
||||||
- Testing:
|
- Testing:
|
||||||
- Fuzz NewManifestFromReader; end-to-end tests for freshen and fetch
|
- End-to-end tests for freshen and fetch
|
||||||
- Documentation:
|
- Documentation:
|
||||||
- Promote docs/FORMAT.md as primary spec reference; audit error messages;
|
- Promote docs/FORMAT.md as primary spec reference; audit error messages;
|
||||||
document the signature scheme fully
|
document the signature scheme fully
|
||||||
|
|||||||
+9
-1
@@ -111,7 +111,15 @@ func (m *manifest) verifyOuterIntegrity() error {
|
|||||||
func (m *manifest) decompressInner() ([]byte, error) {
|
func (m *manifest) decompressInner() ([]byte, error) {
|
||||||
bb := bytes.NewBuffer(m.pbOuter.GetInnerMessage())
|
bb := bytes.NewBuffer(m.pbOuter.GetInnerMessage())
|
||||||
|
|
||||||
zr, err := zstd.NewReader(bb)
|
// By default the decoder decodes a payload under 128 KiB in full,
|
||||||
|
// each frame up to the decoder's limit, before the LimitReader below
|
||||||
|
// reads any of it. Decoding synchronously and never in full makes it decode
|
||||||
|
// only what the LimitReader asks for. Its limit caps the window that
|
||||||
|
// a frame header can make it set aside.
|
||||||
|
zr, err := zstd.NewReader(bb,
|
||||||
|
zstd.WithDecoderConcurrency(1),
|
||||||
|
zstd.WithDecodeBuffersBelow(0),
|
||||||
|
zstd.WithDecoderMaxMemory(uint64(MaxDecompressedSize)))
|
||||||
if err != nil {
|
if err != nil {
|
||||||
return nil, fmt.Errorf("deserialize: zstd reader: %w", err)
|
return nil, fmt.Errorf("deserialize: zstd reader: %w", err)
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -0,0 +1,75 @@
|
|||||||
|
//nolint:testpackage // white-box tests exercise unexported internals
|
||||||
|
package mfer
|
||||||
|
|
||||||
|
import (
|
||||||
|
"bytes"
|
||||||
|
"runtime"
|
||||||
|
"testing"
|
||||||
|
|
||||||
|
"google.golang.org/protobuf/proto"
|
||||||
|
)
|
||||||
|
|
||||||
|
// FuzzNewManifestFromReader feeds arbitrary bytes to the manifest parser.
|
||||||
|
// `make test` runs it on the seed corpus in
|
||||||
|
// testdata/fuzz/FuzzNewManifestFromReader; `make fuzz` searches for new
|
||||||
|
// inputs.
|
||||||
|
//
|
||||||
|
// For every input the parser must return a manifest or an error, not both
|
||||||
|
// and not neither, and must not allocate more than a fixed multiple of its
|
||||||
|
// input and of the decompressed data it may read, plus room for the
|
||||||
|
// decoder's window. A panic or a hang fails the test on its own.
|
||||||
|
func FuzzNewManifestFromReader(f *testing.F) {
|
||||||
|
// A signed manifest makes the parser write the key and signature to a
|
||||||
|
// temporary directory and run gpg on them. With gpg off the PATH and
|
||||||
|
// temporary files kept in the test's own directory, no process is
|
||||||
|
// started and nothing is written elsewhere; such input ends in an
|
||||||
|
// error instead.
|
||||||
|
f.Setenv("PATH", "")
|
||||||
|
f.Setenv("TMPDIR", f.TempDir())
|
||||||
|
|
||||||
|
f.Fuzz(func(t *testing.T, data []byte) {
|
||||||
|
var before, after runtime.MemStats
|
||||||
|
|
||||||
|
runtime.ReadMemStats(&before)
|
||||||
|
|
||||||
|
m, err := NewManifestFromReader(bytes.NewReader(data))
|
||||||
|
|
||||||
|
runtime.ReadMemStats(&after)
|
||||||
|
|
||||||
|
if (m == nil) == (err == nil) {
|
||||||
|
t.Fatalf("got manifest %p and error %v, want exactly one", m, err)
|
||||||
|
}
|
||||||
|
|
||||||
|
// The parser reads at most the declared size plus one byte of
|
||||||
|
// decompressed data, and never more than MaxDecompressedSize.
|
||||||
|
decompressed := uint64(MaxDecompressedSize)
|
||||||
|
|
||||||
|
outer := new(MFFileOuter)
|
||||||
|
if validateMagic(data) &&
|
||||||
|
proto.Unmarshal(data[len(MAGIC):], outer) == nil {
|
||||||
|
size := outer.GetSize()
|
||||||
|
if size > 0 && size < MaxDecompressedSize {
|
||||||
|
decompressed = uint64(size) + 1
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// It also keeps a few copies of its input, and the decoder sets
|
||||||
|
// aside a window that a frame header sizes, up to a little over
|
||||||
|
// MaxDecompressedSize. Buffers grow by copying, so reaching those
|
||||||
|
// sizes allocates a few times them in total: sixteen times the
|
||||||
|
// input and the decompressed data, plus twice MaxDecompressedSize
|
||||||
|
// for the window, leaves room for that. The seed whose frame
|
||||||
|
// claims 8 GiB fails if the decoder sets that size aside; 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) +
|
||||||
|
2*uint64(MaxDecompressedSize)
|
||||||
|
|
||||||
|
allocated := after.TotalAlloc - before.TotalAlloc
|
||||||
|
if allocated > limit {
|
||||||
|
t.Fatalf("allocated %d bytes for %d bytes of input, limit %d",
|
||||||
|
allocated, len(data), limit)
|
||||||
|
}
|
||||||
|
})
|
||||||
|
}
|
||||||
@@ -0,0 +1,2 @@
|
|||||||
|
go test fuzz v1
|
||||||
|
[]byte("")
|
||||||
@@ -0,0 +1,2 @@
|
|||||||
|
go test fuzz v1
|
||||||
|
[]byte("ZNAVSRFGy[i\x04\xe5O\x82A\x1d\xf4\xb0\xe2z7:U\xee\xa3\xf9\xd6m\xacZ\x9b\xce\x1d\xd9/{@\x1d\xa5y[i\x04\xe5O\x82A\x1d\xf4\xb0\xe2z7:U\xee\xa3\xf9\xd6m\xacZ\x9b\xce\x1d\xd9/{@\x1d\xa5")
|
||||||
@@ -0,0 +1,2 @@
|
|||||||
|
go test fuzz v1
|
||||||
|
[]byte("ZNAVSRFG\xa8\x06\x01\xb0\x06\x01\xb8\x06V\xc2\x06 \xa3\xf7\x97\xa7\xf3\x87:\x90)\\ӊj\xb9\xf7\xfaTJ\x1b\xe7:\xee\xbe\"V\xe0:\x8d(Z\xd6%\xca\x06\x10\x93\x85\vpu\x85\xe4\x04\xe4\x95\x1a=\xdc\x1f\x05\xa3\xba\fc(\xb5/\xfd\x04\x00\xb1\x02\x00\xa0\x06\x01\xaa\x06=\n\x05a.txt\x10\x01\x1a$\n\"\x12 ʗ\x81\x12\xca\x1b\xbd\xca\xfa\xc21\xb3\x9a#\xdcM\xa7\x86\xef\xf8\x14|Nr\xb9\x80w\x85\xaf\xeeH\xbb\xf2\x12\v\b\x80\x92\xb8Ø\xfe\xff\xff\xff\x01\xb2\x06\x10\x93\x85\vpu\x85\xe4\x04\xe4\x95\x1a=\xdc\x1f\x05\xa3s\xeeG\x80")
|
||||||
@@ -0,0 +1,2 @@
|
|||||||
|
go test fuzz v1
|
||||||
|
[]byte("ZNAVSRFG\xa8\x06\x01\xb0\x06\x01\xb8\x06V\xc2\x06 \xa3\xf7\x97\xa7\xf3\x87:\x90)\\ӊj\xb9\xf7\xfaTJ\x1b\xe7:\xee\xbe\"V\xe0:\x8d(Z\xd6%\xca\x06\x10\x93\x85\vpu\x85\xe4\x04\xe4\x95\x1a=\xdc\x1f\x05\xa3\xba\fc(\xb5/\xfd\x04\x00\xb1\x02\x00\xa0\x06\x01\xaa\x06=\n\x05a.txt\x10\x01\x1a$\n\"\x12 ʗ\x81\x12\xca\x1b\xbd\xca\xfa\xc21\xb3\x9a#\xdcM\xa7\x86\xef\xf8\x14|Nr\xb9\x80w\x85\xaf\xeeH\xbb\xf2\x12\v\b\x80\x92\xb8Ø\xfe\xff\xff\xff\x01\xb2\x06\x10\x93\x85\vpu\x85\xe4\x04\xe4\x95\x1a=\xdc\x1f\x05\xa3s\xeeG\x80\xca\f\xe8\x03-----BEGIN PGP SIGNATURE-----\n\niQEzBAABCgAdFiEET1Yr+4Y/3GtRtO6IhypRF2zvI64FAmrBIXAACgkQhypRF2zv\nI67BQAf/QrpX2MjY15YGMGkjR5oIhnx/YV96aGYZyZThzb+l/R/N75iVFVkhX21d\nZhQqdCsORrodTPAXic2g2UGVXP9PhNMh7n6Wm3LsvQYjrRQGrQnqtCkut+3tUt8K\n7pt4OAnnwRSieaVImA1COmzxIrQQKNOs6UkgmAstGuPV0XZoeDiSG8TUYJ/vieCn\np5hC0FFXtzfw4NtkxSmkewE0xBxIwFCA/RfSHCGH3m5K+tRz41vMEgGbL1iEp6+V\nuBaoEc4hqCgEt+Af2pA8VHfqeu2vKiwggOpYpaILXZKVqH9+tWHL1EBv9t0vTsYE\n9D57euuR9+kOdngYNPieP1yn5dOSHg==\n=kvgL\n-----END PGP SIGNATURE-----\n\xd2\f(4F562BFB863FDC6B51B4EE88872A51176CEF23AE\xda\f\xb5\a-----BEGIN PGP PUBLIC KEY BLOCK-----\n\nmQENBGrBIW8BCADESetN5EdxIe7Fafgxl99Yoo5cOexf7wJyYT0wfUYlRaxt3neR\nhir7LOfH4PZWWoDx7qghxCS4+vs7yGypl6JOm7jnJlhn4HneDa2zeIlgGW2TamyE\nua9KPWBQqkFOYmKPmzp+KnL6ncnBLR5mDkNKFyON812KVvteu6Dp/DNk4Meufe44\nWWr49LSFZa9gEbmRCoQGKby9F0H0yIi4FAc74VdQudy0+fMKcfkKjEvByMzlbBEK\n92Hq3sRFzWd3kvPliNjZTmlh5n5m9aBhMpoy3GkKy8gpDdFc6NLA9iAJe7oNMriR\nkVoa5EjQL1xCXAiAWTYA9NScFfU/574sCTxZABEBAAG0Hk1GRVIgVGVzdCBLZXkg\nPHRlc3RAbWZlci50ZXN0PokBTwQTAQoAORYhBE9WK/uGP9xrUbTuiIcqURds7yOu\nBQJqwSFvAxsvBAULCQgHAgYVCgkICwIEFgIDAQIeAQIXgAAKCRCHKlEXbO8jrjml\nCAC8wUK9wmvxq0+NZUpFyP+P29klLZYzBDaBrLPJFs0GjnG4kvfUAktWx0Ro80F7\ncjTJ4f44XjDj4glvSjbe2VaDnZl9FTfzUfG+xjD4462NgntQ4fHk/uG4F6d1ikWx\nkEoMpIn1PlSMas1jTQSGlxUr+zFwWuUbGq4n6hRxEnwLlwJlwQt/Aw1vPDYuPDE3\nOYDhJIAJyP+6e9W8ToaAG9byg/22KA1u1qxnNQqsx5Tped2VltAzdYub+yeCuNc8\nIUo5ILo/fQq3GM5sUEaHjPolv88WlDm3vcdbSbDoh5m2inDtg5zuUKJwu32UGu8l\nKoTjp4nxgQGy5WmeBzs4Hdm/\n=TCB8\n-----END PGP PUBLIC KEY BLOCK-----\n")
|
||||||
@@ -0,0 +1,2 @@
|
|||||||
|
go test fuzz v1
|
||||||
|
[]byte("ZNAVSRFG\xa8\x06\x01\xb0\x06\x01\xb8\x06!\xc2\x06 \x91\x90*\xa5>\fݐ \x87\xbeaL\xc1\x05?\x0eR\xc18\xa4eՕ\xa95\xb9KʺoZ\xca\x06\x10\x93\x85\vpu\x85\xe4\x04\xe4\x95\x1a=\xdc\x1f\x05\xa3\xba\f-(\xb5/\xfd\x04\x00\x01\x01\x00\xa0\x06\x01\xaa\x06\a\n\x05a.txt\xb2\x06\x10\x93\x85\vpu\x85\xe4\x04\xe4\x95\x1a=\xdc\x1f\x05\xa3a[k'")
|
||||||
@@ -0,0 +1,2 @@
|
|||||||
|
go test fuzz v1
|
||||||
|
[]byte("ZNAVSRFG\xa8\x06\x01\xb0\x06\x01\xb8\x06\x1f\xc2\x06 \x91\x90*\xa5>\fݐ \x87\xbeaL\xc1\x05?\x0eR\xc18\xa4eՕ\xa95\xb9KʺoZ\xca\x06\x10\x93\x85\vpu\x85\xe4\x04\xe4\x95\x1a=\xdc\x1f\x05\xa3\xba\f-(\xb5/\xfd\x04\x00\x01\x01\x00\xa0\x06\x01\xaa\x06\a\n\x05a.txt\xb2\x06\x10\x93\x85\vpu\x85\xe4\x04\xe4\x95\x1a=\xdc\x1f\x05\xa3a[k'")
|
||||||
@@ -0,0 +1,2 @@
|
|||||||
|
go test fuzz v1
|
||||||
|
[]byte("ZNAVSRFG\xa8\x06\x01\xb0\x06\x01\xb8\x06V\xc2\x06 \xa3\xf7\x97\xa7\xf3\x87:\x90)\\ӊj\xb9\xf7\xfaTJ\x1b\xe7:\xee\xbe\"V\xe0:\x8d(Z\xd6%\xca\x06\x10\x93\x85\vpu\x85\xe4\x04\xe4\x95\x1a=\xdc\x1f\x05\xa3\xba\fc(\xb5/\xfd\x04\x00\xb1\x02\x00\xa0\x06\x01\xaa\x06=\n\x05a.txt\x10\x01\x1a$\n\"\x12 ʗ\x81\x12\xca\x1b\xbd\xca\xfa\xc21\xb3\x9a#\xdcM\xa7\x86\xef\xf8\x14|Nr\xb9\x80w\x85\xaf\xeeH\xbb\xf2\x12\v\b\x80\x92\xb8Ø\xfe\xff\xff\xff\x01\xb2\x06\x10\x93\x85\vpu\x85\xe4\x04\xe4\x95\x1a=\xdc\x1f\x05\xa3s\xeeG")
|
||||||
@@ -0,0 +1,2 @@
|
|||||||
|
go test fuzz v1
|
||||||
|
[]byte("ZNAVSRFG\xa8\x06\x01\xb0\x06\x01\xb8\x06V\xc2\x06 ")
|
||||||
@@ -0,0 +1,2 @@
|
|||||||
|
go test fuzz v1
|
||||||
|
[]byte("ZNAV")
|
||||||
@@ -0,0 +1,2 @@
|
|||||||
|
go test fuzz v1
|
||||||
|
[]byte("ZNAVSRFG")
|
||||||
@@ -0,0 +1,2 @@
|
|||||||
|
go test fuzz v1
|
||||||
|
[]byte("ZNAVSRFG\xa8\x06\x01\xb0\x06\x01\xb8\x06V\xc2\x06 \xa3\xf7\x97\xa7\xf3\x87:\x90)\\ӊj\xb9\xf7\xfaTJ\x1b\xe7:\xee\xbe\"V\xe0:\x8d(Z\xd6%\xca\x06\x10\x93\x85\vpu\x85\xe4\x04\xe4\x95\x1a=\xdc\x1f\x05\xa3\xba\fc(\xb5/\xfd\x04\x00\xb1\x02\x00\xa0\x06\x01")
|
||||||
@@ -0,0 +1,2 @@
|
|||||||
|
go test fuzz v1
|
||||||
|
[]byte("ZNAVSRFX\xa8\x06\x01\xb0\x06\x01\xb8\x06V\xc2\x06 \xa3\xf7\x97\xa7\xf3\x87:\x90)\\ӊj\xb9\xf7\xfaTJ\x1b\xe7:\xee\xbe\"V\xe0:\x8d(Z\xd6%\xca\x06\x10\x93\x85\vpu\x85\xe4\x04\xe4\x95\x1a=\xdc\x1f\x05\xa3\xba\fc(\xb5/\xfd\x04\x00\xb1\x02\x00\xa0\x06\x01\xaa\x06=\n\x05a.txt\x10\x01\x1a$\n\"\x12 ʗ\x81\x12\xca\x1b\xbd\xca\xfa\xc21\xb3\x9a#\xdcM\xa7\x86\xef\xf8\x14|Nr\xb9\x80w\x85\xaf\xeeH\xbb\xf2\x12\v\b\x80\x92\xb8Ø\xfe\xff\xff\xff\x01\xb2\x06\x10\x93\x85\vpu\x85\xe4\x04\xe4\x95\x1a=\xdc\x1f\x05\xa3s\xeeG\x80")
|
||||||
File diff suppressed because one or more lines are too long
@@ -0,0 +1,2 @@
|
|||||||
|
go test fuzz v1
|
||||||
|
[]byte("ZNAVSRFG\xa8\x06\x01\xb0\x06\x01\xb8\x06\x01\xc2\x06 \xd6aQ\xa4\x85\xc0+\xbb\xca\x11\x11\a<\x019\x97\xb3\xbb3\xd0 \xd5U\xfa!\xaeAf<N@\x9d\xca\x06\x10\x93\x85\vpu\x85\xe4\x04\xe4\x95\x1a=\xdc\x1f\x05\xa3\xba\f\x12(\xb5/\xfd\xc0\x00\x00\x00\x00\x00\x02\x00\x00\x00\v\x00\x00\x00")
|
||||||
File diff suppressed because one or more lines are too long
Executable
+17
@@ -0,0 +1,17 @@
|
|||||||
|
#!/bin/sh
|
||||||
|
# script/fuzz: fuzz the manifest parser for one minute. Run by hand only:
|
||||||
|
# script/test already runs the committed seed corpus as ordinary tests,
|
||||||
|
# and CI never fuzzes. An input that fails is written to
|
||||||
|
# mfer/testdata/fuzz/FuzzNewManifestFromReader/; once the parser is fixed,
|
||||||
|
# commit it there as a regression seed.
|
||||||
|
set -eu
|
||||||
|
|
||||||
|
ROOT="$(cd "$(dirname "$0")/.." && pwd -P)"
|
||||||
|
|
||||||
|
main() {
|
||||||
|
cd "$ROOT"
|
||||||
|
go test -run '^$' -fuzz '^FuzzNewManifestFromReader$' \
|
||||||
|
-fuzztime 1m -parallel 2 ./mfer
|
||||||
|
}
|
||||||
|
|
||||||
|
main "$@"
|
||||||
Reference in New Issue
Block a user