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") + } +}