Skip to content

Repository files navigation

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.

TargetExpectedResult
miekg/dnssvcb.go (3 known sites)33 — two high, and SVCBMandatory correctly low (its loop cannot return early)
miekg/dns whole repoonly those2 high, no noise — the wire parsers, bounded by a uint16 rdata length, are correctly ignored
quic-go whole repo00 — every length-driven make there validates first
hashicorp/hcl00
pelletier/go-toml00
buger/jsonparser0 expected1 — 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, "["))
forpos:=0; pos<len(jsonPath); {
switchjsonPath[pos] {
case'.':
returnnil, 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 unrecoverablerecover() 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/cosignpkg/cosign/tlog.go:225getUUID — 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/caddycaddy.go:311indexConfigObjects — an ordinary tree walk over an already-parsedany, 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/exprast/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/giteamodules/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/giteamodules/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.

About

Find Go parsers that reserve memory from untrusted input before validating it

Topics

Resources

Stars

0 stars

Watchers

0 watching

Forks

Releases

Packages

Contributors

Languages