From 8d6b3cbc7d51cf572bf36c0c6584e2f25ce6bd64 Mon Sep 17 00:00:00 2001 From: Michael Fornaro <20387402+xUnholy@users.noreply.github.com> Date: Mon, 22 Jun 2026 01:19:54 +1000 Subject: [PATCH] fix(bplist): reject an implausible element count to stop an unbounded-allocation DoS MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit bplist_decode parses an untrusted binary plist (iOS / macOS forensics loot, dumped preference files). A string / array / set / dict object encodes its element count in a size marker that, when the low nibble is 0xF, is an extended integer up to int64-max. That count flowed into the per-object span computations unbounded: - refs(): need = n * objectRefSize - object() UTF-16 string: bytesAt(p, n*2) With a large n (e.g. 2^63-1) and objectRefSize=2, n*objectRefSize overflows uint64 and wraps to a small value, so the `p + need > len(d.b)` range check passes — and then make([]uint64, n) (or make([]uint16, n) / make(map, n)) allocates the ORIGINAL huge n. Decode has no recover, so a crafted plist as small as 51 bytes triggers an uncaught `makeslice: len out of range` panic (or, for other size/refSize combinations, an OOM), crashing the agent turn. The existing numObjects <= len(data) trailer check does not help — it bounds the object *count*, not a single object's declared element count. The decoder's fuzz harness never crafted a deep-enough valid skeleton (header + offset table + trailer + an extended-count object) to reach it. Fixed at the source: sizedCount now rejects a count greater than the file size. No object can hold more elements than there are bytes on disk (a data byte costs 1, a UTF-16 char 2, an object ref objectRefSize>=1), so the bound never rejects a legitimate plist; it does keep n <= len(d.b), so the n*2 / n*objectRefSize spans can no longer overflow and every downstream make is bounded by the input size. Regression test crafts the minimal repro (header + one array object with a 2^63-1 extended count + a one-entry offset table + a trailer with objectRefSize=2) and asserts Decode returns an error instead of panicking; verified red->green (the pre-fix code panics makeslice, the fixed code errors). Sibling sweep of the count*size-overflow-before-make class: internal/wsframe bounds the payload against len(input) before allocating, internal/quic's CRYPTO length is a <=2^62 varint that cannot overflow the uint64 check, and internal/subghz.BytesToBits has no non-test callers — bplist was the only live instance. gofmt / go vet / golangci-lint (0 issues) / go test -race + 15s fuzz all green; legitimate plists still decode. --- internal/bplist/bplist.go | 10 +++++++ internal/bplist/bplist_alloc_test.go | 45 ++++++++++++++++++++++++++++ 2 files changed, 55 insertions(+) create mode 100644 internal/bplist/bplist_alloc_test.go diff --git a/internal/bplist/bplist.go b/internal/bplist/bplist.go index 029a242c..53d8b334 100644 --- a/internal/bplist/bplist.go +++ b/internal/bplist/bplist.go @@ -228,6 +228,16 @@ func (d *decoder) sizedCount(off uint64, lo byte) (uint64, uint64, error) { if !ok || cnt < 0 { return 0, 0, fmt.Errorf("bplist: bad extended count") } + // No object can hold more elements/bytes than the whole file contains (a + // data byte costs 1 byte on disk, a UTF-16 char 2, an object ref + // objectRefSize>=1). Bounding the count here keeps the n*2 / n*objectRefSize + // span computations in object() / refs() from overflowing uint64 and + // wrapping their range checks small — the path that otherwise reaches + // make([]T, hugeN) and panics (makeslice) / OOMs on a crafted plist, since + // Decode has no recover. + if uint64(cnt) > uint64(len(d.b)) { + return 0, 0, fmt.Errorf("bplist: implausible element count %d (exceeds the %d-byte file)", cnt, len(d.b)) + } return uint64(cnt), p + 1 + uint64(n), nil } diff --git a/internal/bplist/bplist_alloc_test.go b/internal/bplist/bplist_alloc_test.go new file mode 100644 index 00000000..cd10e334 --- /dev/null +++ b/internal/bplist/bplist_alloc_test.go @@ -0,0 +1,45 @@ +package bplist + +import ( + "encoding/binary" + "testing" +) + +// TestDecode_HugeElementCountIsRejected guards an unbounded-allocation DoS: a +// crafted bplist whose array/string object declares an extended element count +// near int64-max makes the n*objectRefSize span computation in refs (or n*2 in +// the UTF-16 path) overflow uint64, wrapping the range check small so it passes, +// then make([]uint64, n) / make([]uint16, n) allocates the original huge n — an +// uncaught makeslice panic / OOM on an untrusted .plist (Decode has no recover). +// +// The fix bounds the element count to the file size in sizedCount (no object can +// have more elements than there are bytes on disk). This test crafts the +// minimal valid skeleton — header + one array object with a 2^63-1 extended +// count + a one-entry offset table + a trailer with objectRefSize=2 — and +// asserts Decode returns an error instead of panicking. +func TestDecode_HugeElementCountIsRejected(t *testing.T) { + var b []byte + b = append(b, "bplist00"...) // 0..7 + // object 0 at offset 8: array (0xA) with extended count (lo 0xF). + b = append(b, 0xAF) // 8: array marker + b = append(b, 0x13) // 9: int marker, 1<<3 = 8-byte count + b = append(b, 0x7F, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF) // 10..17: count = 2^63-1 + b = append(b, 0x08) // 18: offset table — object 0 at offset 8 + + tr := make([]byte, 32) + tr[6] = 0x01 // offsetIntSize + tr[7] = 0x02 // objectRefSize (makes n*refSize overflow) + binary.BigEndian.PutUint64(tr[8:16], 1) // numObjects + binary.BigEndian.PutUint64(tr[16:24], 0) // topObject + binary.BigEndian.PutUint64(tr[24:32], 18) // offsetTableOffset + b = append(b, tr...) + + defer func() { + if r := recover(); r != nil { + t.Fatalf("Decode panicked on a crafted huge element count (unbounded-allocation DoS): %v", r) + } + }() + if _, err := Decode(b); err == nil { + t.Fatal("expected an error for an implausible element count exceeding the file size, got nil") + } +}