allocguard

README

allocguard

Finds make() capacity hints that are computed from untrusted input before that input is validated — a Go pattern where a short, wholly invalid value can make a parser reserve megabytes it immediately throws away.

out := make([]T, 0, strings.Count(s, sep)+1)   // capacity from attacker input
for len(s) > 0 {
    elem, s, _ = strings.Cut(s, sep)
    if !ok(elem) { return err }                // rejected on element #1
}

strings.Count is counted before any element is checked, so ",,,,,…" allocates sizeof(T) per separator and then fails on the first one. Clamping the hint fixes it with no behaviour change, because capacity is only a hint to append, which still grows as needed.

Install

go install github.com/tzh476/allocguard@latest

Or clone and go build -o allocguard . — stdlib only, no dependencies.

Usage

go build -o allocguard .
./allocguard ./...            # high-confidence findings only
./allocguard -strict ./...    # also sites whose fill loop cannot return early
./allocguard -json ./...      # machine-readable
./allocguard -recursion ./... # also unbounded self-recursion (low precision, see below)

Exit status is 1 when findings exist, so it drops into CI unchanged.

Why it is quiet enough to be useful

make([]T, 0, n) is overwhelmingly legitimate, so a naive grep is unusable. This checker reports a site only when all of these hold:

1. the capacity expression contains strings.Count / bytes.Count,
2. applied to a value the enclosing function did not create (values assigned from a call inside the function are treated as the function's own product, and therefore bounded by real input),
3. and the loop that fills the slice can return early — which is what makes the reserved space wasted rather than used.

Rule 3 is the difference between a finding and noise, so it is also reported as the severity: high when the loop can bail out, low (only under -strict) when it cannot.

Validation against hand-verified ground truth

Every number below was produced by running the tool; the ground truth was established by reading the code and measuring with go test -bench beforehand.

Target	Expected	Result
miekg/dns svcb.go (3 known sites)	3	3 — two high, and SVCBMandatory correctly low (its loop cannot return early)
miekg/dns whole repo	only those	2 high, no noise — the wire parsers, bounded by a uint16 rdata length, are correctly ignored
quic-go whole repo	0	0 — every length-driven make there validates first
hashicorp/hcl	0	0
pelletier/go-toml	0	0
buger/jsonparser	0 expected	1 — a real defect I had missed by hand

That last row is the point. I had previously inspected jsonparser's reader_parser.go and concluded the package was hardened (it uses an explicit heap stack instead of recursion). The tool pointed at a different file, path_compiler.go:40:

parts := make([]string, 0, 1+strings.Count(jsonPath, ".")+strings.Count(jsonPath, "["))
for pos := 0; pos < len(jsonPath); {
    switch jsonPath[pos] {
    case '.':
        return nil, errMalformedPath      // rejected on the first character

Measured (Apple M3 Pro, go1.22.5, -benchtime 200x, input strings.Repeat(".", 200000), which ParsePath rejects immediately):

BenchmarkParsePathRejectDots-12    162253 ns/op    3203077 B/op    1 allocs/op

3.2 MB reserved and discarded for input rejected at byte one. The benchmark used is in testdata/jsonparser-bench.txt.

Two checks

1. Pre-allocation from unvalidated input (the original check, described above).

2. Unbounded recursion over untrusted input — OFF BY DEFAULT, opt in with -recursion. A parser that recurses on caller-supplied bytes with no depth parameter can in principle be driven to a stack overflow, which in Go is fatal and unrecoverable — recover() cannot catch it, so a single crafted input takes the process down rather than returning an error. Reported only when the function takes a string/[]byte/reader, calls itself directly, and carries nothing that could bound depth (no numeric parameter, no name containing depth/level/nest/budget/remaining/limit).

Do not trust this check's output without reading the function. It is off by default because its measured precision on real code is very poor. A sweep of 11 OSS-Fuzz Go projects produced 55 recursion findings and not one true positive — every hit was recursion that was bounded by construction. Two verified examples:

• sigstore/cosign pkg/cosign/tlog.go:225 getUUID — the recursive call passes a string whose length equals uuidHexStringLen, so it necessarily matches the first case and returns. Depth is at most one.
• caddyserver/caddy caddy.go:311 indexConfigObjects — an ordinary tree walk over an already-parsed any, whose depth was bounded by the JSON decoder that produced it. This is precisely the situation the tool's own design note says to skip.

The reason is structural, not a tuning problem: proving a recursion unbounded requires showing the depth is controlled by untrusted input with no limit, and "the function calls itself with a value derived from a parameter" does not establish that. Syntax alone cannot see the bound.

Findings per repository when enabled, after the receiver fix described below: gonids 0, go-coap 0, go-toml 0, fasthttp 0, cel-go 1, minify 2, hcl 2, jsonparser 2, protocompile 5 — a reviewable rate, but treat each as a lead to investigate, not a defect.

The bug this check had, and how it was caught

The first version matched a self-call by name alone. That is wrong: a method like (*Assertions).Conditionf that delegates to a package-level Conditionf is not recursive at all. It produced 172 findings in one repository (expr-lang/expr), nearly all that exact shape. Requiring a method to call itself through its own receiver, and a plain function to be called as a bare identifier, took the same repository from 172 to 2 while the ground-truth fixture stayed at exactly 2. That was a real defect, not a threshold tweak — and it is why the per-repo counts above are quoted rather than described as "quiet".

Honest limits

• The recursion check (-recursion) has near-zero precision — 0 true positives in 55 findings across 11 projects. This is why it is off by default; see "Two checks" above for the verified false positives. It also reports the untrusted parameter rather than the value that drives the depth: in expr-lang/expr ast/dump.go it flags dump and names the ident parameter, but ident is only an indent prefix and the recursion is driven by the reflect.Value being walked. Always read the function before acting on anything this check says.
• It cannot see who produces the counted value. This is the limitation that matters most in practice. The checker proves the capacity is counted before validation; it cannot tell whether the bytes came off a socket or out of a trusted local subprocess. A worked example, from a real scan: go-gitea/gitea modules/git/parse_treeentry.go:18 counts bytes.Count(data, '\n') and its line parser has three early-return paths, so it reports at high — and it is not a defect, because data is the stdout of git ls-tree and git quotes a newline inside a filename as the two characters \n rather than emitting a literal one (verified with git ls-tree -l HEAD | od -c on a repo containing such a file). The producer is trusted, malformed input never arrives, and every reserved slot is used. Always establish who produces the value before filing anything upstream.
• Syntactic analysis only, no type information: it matches the strings.Count / bytes.Count idiom and will miss capacity derived some other way (a parsed length field, len(x)*k).
• "Locally created" is approximated by "assigned from a call in this function", which is a heuristic, not dataflow.
• It reports where capacity is untrusted; whether that is exploitable depends on callers and on any upstream size cap. Confirm before filing anything upstream.
• The min() in the suggested fix needs Go 1.21+.

Specificity at scale

Scanned whole repositories to check the checker stays quiet on code that does not have this defect. All clean (no findings), most in -strict mode:

gohugoio/hugo, go-git/go-git, spf13/viper, prometheus/prometheus, valyala/fastjson, tidwall/gjson, mitchellh/mapstructure, hashicorp/go-getter, golang/snappy, pierrec/lz4, ulikunitz/xz, dsnet/compress, vmihailenco/msgpack, go-yaml/yaml, golang/text, golang/net, go-redis/redis, grpc/grpc-go, sirupsen/logrus, prometheus/client_golang, plus quic-go, hashicorp/hcl and pelletier/go-toml from the table above — 21 repositories, zero false alarms.

A later sweep added 16 more, selected because they parse untrusted input and merge outside contributions: insomniacslk/dhcp, go-ldap/ldap, valyala/fasthttp, google/pprof, OpenPrinting/ipp-usb, shirou/gopsutil, klauspost/compress, distribution/distribution, tailscale/tailscale, caddyserver/caddy, etcd-io/etcd, zalando/skipper, cert-manager/cert-manager, google/go-containerregistry, metallb/metallb, go-gitea/gitea. Default mode was silent on 15 of them.

That is the honest shape of this tool: the defect is rare, so the value is that a scan is cheap and a hit is meaningful — 37 large codebases, and default mode stayed quiet on all but the ones with something to say.

One reported site was not a real defect

Stated plainly because it is the tool's main limitation in action. go-gitea/gitea modules/git/parse_treeentry.go:18 matched every rule and reported at high. It is not exploitable: the counted bytes are git ls-tree output, and git escapes a newline inside a filename rather than emitting a literal one, so the count always equals the entry count. The checker had no way to know that — see Honest limits. Treat a high as "read this and decide", not as a verdict.

The severity split earns its keep

coredns is the instructive case. plugin/pkg/replacer/replacer.go:191 does make(replacer, 0, strings.Count(s, "{")*2) — a textbook match for the pattern. The tool reported it only under -strict, at low, because the fill loop cannot return early. That was the right call: s is a log format string from the Corefile, i.e. operator configuration rather than request data, and the result is memoised. Default mode stays silent, so you are not asked to triage it.

Tests

go test ./... — 10 tests, all passing, 62.6% statement coverage. They pin both directions: the canonical defect is reported at high; a fill loop that cannot return early is low and -strict-only; capacity derived from a value the function itself created is silent; len()-derived capacity is silent; two-argument make is silent; bytes.Count behaves like strings.Count; a parameter shadowed by a call stays untrusted; summed counts (the jsonparser shape) are reported; _test.go files are skipped; and an unparseable file does not abort a directory scan.

Documentation

Overview

lenfield.go — the length-field shape: make([]T, n) where n was decoded from untrusted input and never compared against what the input can actually supply.

This is a different and more severe bug than the capacity-hint check in main.go. A capacity hint is only a hint to append, so an over-large one wastes a reservation that append may never touch. A *length* allocates n elements immediately and for real:

n := binary.BigEndian.Uint32(hdr)   // 4 attacker-controlled bytes
out := make([]uint64, n)            // up to 4 GiB × 8 = 32 GiB, right now

Four of the seven upstream patches this project has filed were this shape, and they are the ones with the large numbers:

Consensys/gnark-crypto #878     16,777,426 → 210 B/op   (−99.999%)
multiformats/go-multihash #198  268,444,004 → 64 B/op
segmentio/encoding #172         67,109,251 → 250 B/op   (−99.9996%)
cubefs/cubefs #4093             uint16 overflow truncation

So the detector was blind to its own best evidence: measured on a sample holding two of those exact shapes, `allocguard` and `allocguard -strict` both reported "no findings" — 0 of 2, not a clean bill of health.

What makes a site reportable

The rule that emerged from writing those four patches, and from the false-positive review that showed the recursion checker was 55/55 wrong:

1. n is decoded from an untrusted reader or byte slice. Recognising the decode is what makes the value untrusted; a plain integer parameter is not evidence of anything.
2. Between that decode and the make() there is **no comparison involving n**. A bare `if n > 0` does not count — the guard has to be able to reject a large n, so it must compare n against something else.
3. The allocation is a length, not a capacity: make(T, n) or make(T, n, …). make(T, 0, n) is the other check's business.

Condition 2 is deliberately generous to the code under test: any comparison mentioning n silences the finding. That direction is chosen on purpose. A missed bug costs nothing but a missed bug; a false positive spends a maintainer's attention, and this project has already had a patch closed in 29 minutes with the comment "AI Garbage". Quiet and correct beats thorough.

Command allocguard reports capacity hints that are computed from untrusted input before that input has been validated.

The pattern it looks for is

out := make([]T, 0, strings.Count(s, sep)+1)
for ... {
        elem, s, _ = strings.Cut(s, sep)
        if !ok(elem) {
                return err          // rejected on the first element
        }
}

The separator count is attacker-controlled and is counted before any element is validated, so a short, wholly invalid value can reserve a large buffer that is then discarded. Clamping the hint fixes it without changing behaviour, because capacity is only a hint to append.

The check is deliberately narrow: it reports a call only when the capacity expression derives from a counting function applied to a value that the enclosing function did not create, and when the loop that fills the slice can return early. That keeps it quiet on the many legitimate uses of make([]T, 0, n) whose n comes from an already-parsed structure.