Compare commits

..
1 Commits
Author SHA1 Message Date
sneak e7331e8d11 Reject manifests whose file entries decode far larger than their bytes (closes #123)
check / check (push) Failing after 18s
Parser fix: before decoding the manifest, the parser walks its file
entries and adds up what decoding sets aside for each entry, hash,
timestamp and MIME type, however short its encoding. It refuses the
manifest once that sum passes 8 times the decompressed size; the
densest manifests mfer writes come to about 7 times. Empty entries
decoded to about 50 times their size, so a 1.6 KB manifest allocated
nearly 1 GB. The fuzz target's ceiling falls to 20 times the input and
decompressed data, and a new seed of entries holding only an empty MIME
type and empty times fails it without the fix.

Model: opus-5-5
2026-10-04 05:56:21 +00:00
13 changed files with 58 additions and 87 deletions
+2 -7
View File
@@ -14,9 +14,7 @@ RUN touch mfer/mf.pb.go
# Go half of fmt-check only: this image has no node, so no prettier. The # Go half of fmt-check only: this image has no node, so no prettier. The
# markdown half runs in the mdfmt stage below. # markdown half runs in the mdfmt stage below.
RUN make fmt-check-go RUN make fmt-check-go
# The linter directly, not `make lint`: script/lint builds this stage, and RUN make lint
# there is no docker inside this build.
RUN golangci-lint run --config .golangci.yml ./...
# Markdown/JSON format stage — prettier needs node, which the Go images # Markdown/JSON format stage — prettier needs node, which the Go images
# do not have. node:22.17.0-bookworm-slim (2026-08-09); ships node # do not have. node:22.17.0-bookworm-slim (2026-08-09); ships node
@@ -69,10 +67,7 @@ RUN version="${VERSION:-$(git describe --tags --always)}"; \
exit 1; \ exit 1; \
fi; \ fi; \
cd cmd/mfer && \ cd cmd/mfer && \
CGO_ENABLED=0 go build -tags urfave_cli_no_docs -ldflags "-X main.Gitrev=$version" -o /mfer . go build -tags urfave_cli_no_docs -ldflags "-X main.Gitrev=$version" -o /mfer .
# Fail unless /mfer is statically linked: scratch has no C library to run it.
RUN ldd /mfer 2>&1 | grep -q 'not a dynamic executable'
FROM scratch FROM scratch
COPY --from=builder /mfer /mfer COPY --from=builder /mfer /mfer
+1 -3
View File
@@ -49,9 +49,7 @@ The `innerMessage` field is compressed with
enforce a decompression size limit to prevent decompression bombs. The reference enforce a decompression size limit to prevent decompression bombs. The reference
implementation limits decompressed size to 256 MB. It writes zstd frames with a 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 window of at most 8 MiB, the largest window the zstd format recommends decoders
support, and refuses frames that ask for a larger one. It also refuses an inner support, and refuses frames that ask for a larger one.
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`) ## Inner Message (`MFFile`)
+3
View File
@@ -58,6 +58,9 @@ fmt-check-md:
hooks: hooks:
@script/install-precommit @script/install-precommit
devprereqs:
which golangci-lint || go install -v github.com/golangci/golangci-lint/v2/cmd/golangci-lint@v2.12.2
mfer/mf.pb.go: mfer/mf.proto mfer/mf.pb.go: mfer/mf.proto
cd mfer && go generate . cd mfer && go generate .
+11 -12
View File
@@ -65,9 +65,9 @@ standard: normalized scripts in `script/` are the entrypoints for the
development workflow, and the Makefile targets are thin shims that call them. We development workflow, and the Makefile targets are thin shims that call them. We
provide: provide:
- `script/bootstrap` — install all dependencies (Go, Go module download, and - `script/bootstrap` — install all dependencies (Go, golangci-lint, Go module
node/yarn plus the prettier version pinned in `package.json`/`yarn.lock`), download, and node/yarn plus the prettier version pinned in
idempotently; golangci-lint is not installed, it runs only in Docker `package.json`/`yarn.lock`), idempotently
- `script/setup` — make a fresh clone ready for development: runs - `script/setup` — make a fresh clone ready for development: runs
`script/bootstrap`, then `script/install-precommit` `script/bootstrap`, then `script/install-precommit`
- `script/projectname` — output the project name (`mfer`); used by other scripts - `script/projectname` — output the project name (`mfer`); used by other scripts
@@ -77,11 +77,9 @@ provide:
- `script/fuzz` — fuzz the manifest parser for one minute; run by hand - `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 (`make fuzz`), never by CI, while `script/test` runs its committed seed corpus
as ordinary tests as ordinary tests
- `script/lint` — run `golangci-lint` in Docker: builds only the `lint` stage of - `script/lint` — run `golangci-lint` and verify `gofmt` cleanliness
the `Dockerfile` (the Go format check, then the linter), uncached so it runs - `script/fmt` — format all code and docs (writes): `gofumpt`,
every time, then removes the image `golangci-lint run --fix`, and `script/prettier --write`
- `script/fmt` — format all code and docs (writes): `gofumpt` and
`script/prettier --write`
- `script/prettier` — run prettier over the repository's canonical file set - `script/prettier` — run prettier over the repository's canonical file set
(Markdown and JSON, minus `.prettierignore`) in the given mode, `--write` or (Markdown and JSON, minus `.prettierignore`) in the given mode, `--write` or
`--check`; the single definition of that file set, so `script/fmt` and `--check`; the single definition of that file set, so `script/fmt` and
@@ -107,10 +105,11 @@ yet. Primary development happens on a privately-run Gitea instance at
[tracked there](https://git.eeqj.de/sneak/mfer/issues). [tracked there](https://git.eeqj.de/sneak/mfer/issues).
Changes must always be formatted with a standard `go fmt`, syntactically valid, Changes must always be formatted with a standard `go fmt`, syntactically valid,
and must pass the linting defined in the repository's `.golangci.yml`, which and must pass the linting defined in the repository (presently only the
`make lint` runs in Docker. The `main` branch is protected and all changes must `golangci-lint` defaults), which can be run with a `make lint`. The `main`
be made via [pull requests](https://git.eeqj.de/sneak/mfer/pulls) and pass CI to branch is protected and all changes must be made via
be merged. Any changes submitted to this project must also be [pull requests](https://git.eeqj.de/sneak/mfer/pulls) and pass CI to be merged.
Any changes submitted to this project must also be
[WTFPL-licensed](https://wtfpl.net) to be considered. [WTFPL-licensed](https://wtfpl.net) to be considered.
See [`REPO_POLICIES.md`](REPO_POLICIES.md) for detailed coding standards, See [`REPO_POLICIES.md`](REPO_POLICIES.md) for detailed coding standards,
+1 -4
View File
@@ -35,9 +35,6 @@ const (
decodedTimestampSize = 64 decodedTimestampSize = 64
decodedMIMETypeSize = 16 decodedMIMETypeSize = 16
// Each file entry mfer writes holds a path of at least one byte, a // The densest manifests mfer writes add up to about 7 times their size.
// 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 maxDecodedGrowth = 8
) )
+3 -2
View File
@@ -76,8 +76,9 @@ func FuzzNewManifestFromReader(f *testing.F) {
// fails if the decoder accepts windows of twice zstdWindowSize; the // fails if the decoder accepts windows of twice zstdWindowSize; the
// seed whose two frames together exceed MaxDecompressedSize fails // seed whose two frames together exceed MaxDecompressedSize fails
// if the decoder decodes them in full instead of stopping at the // if the decoder decodes them in full instead of stopping at the
// declared size; the seeds of empty file entries and of a file // declared size; the seeds of empty file entries, of a file entry
// entry of empty hashes fail if the parser decodes them. // of empty hashes, and of file entries of only an empty MIME type
// and empty times fail if the parser decodes them.
limit := 20*(uint64(len(data))+decompressed) + 24*zstdWindowSize limit := 20*(uint64(len(data))+decompressed) + 24*zstdWindowSize
allocated := after.TotalAlloc - before.TotalAlloc allocated := after.TotalAlloc - before.TotalAlloc
+18 -45
View File
@@ -7,7 +7,6 @@ import (
"crypto/sha256" "crypto/sha256"
"fmt" "fmt"
"strconv" "strconv"
"strings"
"testing" "testing"
"time" "time"
@@ -117,55 +116,29 @@ func TestDeserializeRejectsInvalidEntryPaths(t *testing.T) {
} }
} }
// Entries of a path, an empty hash, an empty MIME type and empty modification // Entries of a one-character path and empty modification and change times
// and change times are counted at 416 bytes each (160 + 112 + 16 + 64 + 64) // pass every other check, but would take about 23 times their size to decode.
// 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) { func TestDeserializeRefusesEntriesThatDecodeTooLarge(t *testing.T) {
t.Parallel() t.Parallel()
tests := []struct { entry := protowire.AppendTag(nil, 1, protowire.BytesType) // MFFilePath.path
pathLen int entry = protowire.AppendString(entry, "a")
refused bool entry = protowire.AppendTag(entry, 302, protowire.BytesType) // MFFilePath.mtime
}{ entry = protowire.AppendBytes(entry, nil)
{35, true}, entry = protowire.AppendTag(entry, 303, protowire.BytesType) // MFFilePath.ctime
{37, false}, 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)
} }
for _, tt := range tests { _, err := NewManifestFromReader(bytes.NewReader(wrapInner(t, id, inner)))
t.Run(strconv.Itoa(tt.pathLen), func(t *testing.T) { require.ErrorIs(t, err, errDecodedTooLarge)
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)
}
})
}
} }
// Many empty files with names of at most three characters and modification // Many empty files with names of at most three characters and modification
File diff suppressed because one or more lines are too long
File diff suppressed because one or more lines are too long
File diff suppressed because one or more lines are too long
+6 -1
View File
@@ -140,7 +140,12 @@ main() {
# ---- Go repos ---- # ---- Go repos ----
if missing go; then pkg_install go golang go go; fi if missing go; then pkg_install go golang go go; fi
# No golangci-lint: script/lint runs it in Docker only. # golangci-lint: packaged in nix, brew, and apk. On apt there is no
# package: download a specific release archive from GitHub and
# verify its hash (verify_sha256), never curl | sh.
if missing golangci-lint; then
pkg_install golangci-lint golangci-lint golangci-lint golangci-lint
fi
go mod download go mod download
# ---- Python repos ---- # ---- Python repos ----
+1
View File
@@ -19,6 +19,7 @@ main() {
cd "$ROOT" cd "$ROOT"
ensure_pb ensure_pb
gofumpt -l -w mfer internal cmd gofumpt -l -w mfer internal cmd
golangci-lint run --fix
# Markdown and JSON, over the same file set script/fmt-check verifies. # Markdown and JSON, over the same file set script/fmt-check verifies.
"$SCRIPT_DIR/prettier" --write "$SCRIPT_DIR/prettier" --write
} }
+8 -11
View File
@@ -1,20 +1,17 @@
#!/bin/sh #!/bin/sh
# script/lint: run golangci-lint, in Docker only. Builds the lint stage of # script/lint: run the linter.
# the Dockerfile, whose build runs the linter, so a successful build is a
# clean lint. --no-cache because a cached build runs no linter. The image
# is removed afterwards, whatever the outcome.
set -eu set -eu
SCRIPT_DIR="$(cd "$(dirname "$0")" && pwd -P)" ROOT="$(cd "$(dirname "$0")/.." && pwd -P)"
ROOT="$(cd "$SCRIPT_DIR/.." && pwd -P)"
main() { main() {
cd "$ROOT" cd "$ROOT"
# Tagged per run, so concurrent runs never remove each other's image. golangci-lint run
image="$("$SCRIPT_DIR/projectname")-lint:$$" if [ -n "$(gofmt -l .)" ]; then
# A failed build leaves no image, so there is nothing to remove then. echo "gofmt: files need formatting:" >&2
trap 'docker image rm "$image" >/dev/null 2>&1 || true' EXIT INT TERM gofmt -l . >&2
docker build --no-cache --target lint -t "$image" . exit 1
fi
} }
main "$@" main "$@"