The Go parser bug that reserves memory for input it is about to reject

There is a small, repeatable mistake in Go parsers that costs real memory and is almost invisible in review. A well-known instance landed in rs/cors, the most widely used CORS middleware for Go (issue #170, fixed in #171, merged 2024-04-24). Here is the shape, why it is easy to miss, and — the part most writeups skip — why the obvious fix is often a regression.

The shape

func parseIPList(b string) ([]net.IP, error) {
	out := make([]net.IP, 0, strings.Count(b, ",")+1)
	for len(b) > 0 {
		var e string
		e, b, _ = strings.Cut(b, ",")
		ip := net.ParseIP(e)
		if ip == nil {
			return nil, fmt.Errorf("bad ip %q", e)   // rejected on element #1
		}
		out = append(out, ip)
	}
	return out, nil
}

Read the order of operations. The capacity hint is computed from strings.Count(b, ",") — a number the caller controls — before a single element has been validated. Feed it one mebibyte of nothing but commas and it reserves space for 1,048,577 addresses, then throws all of it away when net.ParseIP fails on the first element.

Measured, go1.22.5 on an Apple M3 Pro, 1 MiB of commas:

unclamped:  25,176,120 B allocated

25 MB of allocation to reject a 1 MB string. The attacker sends bytes; you allocate 25× that and discard it.

Why review misses it

Every part of this function is individually reasonable. Pre-sizing a slice is good practice — it is what you are supposed to do. strings.Count is cheap and exact. The validation is present and correct. The loop returns the error properly.

The defect is only in the ordering, and ordering is exactly what eyes slide over. Worse, make([]T, 0, n) is overwhelmingly benign: in most code n comes from an already-parsed structure and the reservation is used. So you cannot grep for it. Any tool that flags make with a computed capacity drowns you.

Three conditions have to hold together before it is a real defect:

1. the capacity derives from a count over untrusted input;
2. the fill loop can return early, so the reservation goes unused;
3. legitimate inputs have far fewer elements than the separator count implies.

The fix that is actually a regression

The tempting fix is a constant ceiling:

n := strings.Count(b, ",") + 1
if n > 64 { n = 64 }        // looks safe. it is not.

Hostile input collapses beautifully — 25,176,120 B down to 3,896 B, a 6,463× improvement. Ship it?

No. Measure a legitimate input of 1,000 addresses:

unclamped (64-cap):  40,576 B
clamped   (64-cap):  95,616 B      <- 2.36x WORSE

The constant cap under-reserves every valid list longer than 64 entries, so append regrows repeatedly and you have made the common case worse to fix the rare one. This is the trap: the headline number gets better while your users get slower.

The bound has to be proportional to the input, not a constant. The shortest legal IPv4 element is 7 bytes (1.1.1.1), so len(b)/7+1 can never under-reserve a valid list, while still refusing to believe a megabyte of separators:

n := strings.Count(b, ",") + 1
if max := len(b)/7 + 1; n > max { n = max }

Same two measurements:

hostile 1 MiB of commas:  25,176,120 B -> 3,598,400 B   (7x better)
legitimate 1000 IPs:           40,576 B ->    40,576 B   (1.00x — identical)

7× instead of 6,463×, and zero regression on real traffic. That is the trade worth taking. A defensive fix that penalises legitimate users is not a fix.

Note also that clamping cannot change behaviour: capacity is only a hint to append, which grows as needed. Which inputs are accepted, and what a successful parse returns, are both untouched. That property is what makes this safe to land.

It is in real code

buger/jsonparser, path_compiler.go:40, sizes its slice from two strings.Count calls over the path, and the very next loop has case '.': return nil, errMalformedPath — so a path of nothing but dots is rejected at character zero.

Benchmarked against current main, a 100,000-dot path:

upstream:               74,792 ns/op   1,606,185 B/op   1 alloc/op
proportional clamp:     58,093 ns/op     803,367 B/op   1 alloc/op

1.6 MB in a single allocation, to reject input at character zero. (Note for anyone reproducing this: TestOracleSetPr286Regression already fails on unpatched upstream main, so do not read that failure as the clamp's fault — I checked the control before believing my own diff.)

miekg/dns has the same shape at svcb.go:620 and :742. The rs/cors instance was structurally identical: untrusted count → unbounded reservation → small defensive fix, proven with before/after benchmarks. (That fix was jub0bs' work, not mine — I cite it because it is the clearest public example of the pattern.)

Finding them

I wrote a checker for this, because after fixing a few by hand I wanted to know whether it was a pattern or a coincidence. It is a pattern.

It reports a site only when all three conditions above hold, which keeps it quiet. Scanned just now, default mode, ten large Go repositories:

hugo 0    go-git 0    prometheus 0    caddy 0    fasthttp 0    quic-go 0
tailscale 1    etcd 1    jsonparser 1    miekg/dns 2

Six report nothing at all. That silence is the feature — a checker for a rare defect is only useful if it stays quiet on the 99% of make([]T, 0, n) that is fine.

It is honest about what it cannot do. It is syntactic, so it matches the strings.Count / bytes.Count idiom and misses capacity derived another way. And it cannot see who produces the counted value: scanning go-gitea/gitea it reported a site at high severity that is genuinely safe, because those bytes come from git ls-tree, and git escapes newlines in filenames, so the malformed input never arrives. It tells you a capacity is untrusted; you still have to establish the producer before filing anything upstream.

It ships as source (MIT), stdlib only, exits 1 on findings so it drops into CI unchanged — github.com/tzh476/allocguard:

go install github.com/tzh476/allocguard@latest

A packaged archive with install notes and the benchmark fixtures is available at payhip.com/b/27A9r if you would rather buy it than build it.

But the pattern above is the actual point, and you do not need my tool to use it. Search your own parsers for a capacity computed from input, then check whether the loop below it can return before that space is used. If it can, make the bound proportional — and measure the legitimate case before you believe you have fixed anything.

---

I do this kind of work — reproduce a defect, deliver the smallest verified fix, add the regression test that would have caught it — as bounded fixed-price milestones. Scope and terms in writing first: github.com/tzh476.

---

Reproducing the numbers here

cd demo && go test -v -run TestReport .

Prints both measurements — the hostile input and the legitimate one. Every figure in this README came out of that test or out of a benchmark against the upstream repo named beside it; none of them are estimates.

On macOS you may need CGO_ENABLED=0 if your local cgo linker is unhappy with net (that bit me while writing this: dyld: missing LC_UUID load command).

Using it as a library

go get github.com/tzh476/go-prealloc-before-validate/prealloc

n := prealloc.Bound(strings.Count(b, ",")+1, len(b), 7)
out := make([]net.IP, 0, n)

Bound returns the smaller of the parser's own (untrusted) estimate and the largest number of elements the input could actually contain. The ceiling is derived from the input length, so a valid input is never under-reserved.

Measured by the package's own tests (go test ./prealloc/):

input	unbounded	Bound	constant cap (64)
hostile: 1 MiB of separators	25,176,248 B	3,598,424 B (7x less)	—
legitimate: 1000 IPs	40,576 B	40,576 B (no change)	95,616 B (2.36x worse)

The third column is why the constant cap is a regression, and the test suite asserts it: TestMeasured fails if the constant clamp ever stops regressing, and TestBoundNeverUnderReservesValidInput fails if Bound is replaced by a constant. Both were verified by mutation — swapping the proportional ceiling for max := 64 turns all three tests red.

Note for macOS: net links via cgo and can abort the test binary with missing LC_UUID load command on some toolchains. Run CGO_ENABLED=0 go test ./prealloc/.