From e19ac088a3cbdec58234f005d82bb48fd36d8452 Mon Sep 17 00:00:00 2001 From: Brian Willows Date: Thu, 10 Sep 2026 08:07:22 +0100 Subject: [PATCH] fix: bound object-graph expansion and detect reference cycles Binary plists address objects by index, so one stored object can be referenced many times. maxObjectCount bounds how many objects are *stored*, not how many are *expanded*, so a tiny file can describe an enormous tree: if each level references the level below twice, n objects expand to 2^n nodes. A 155-byte file built this way (29 objects, 2^28 expansions) exhausts the heap and terminates the process with FATAL ERROR: Ineffective mark-compacts near heap limit - JavaScript heap out of memory which a caller cannot catch, since V8 aborts rather than throwing. A 43-byte file whose object references itself recurses until the stack overflows. Add an expansion budget (maxExpandedObjectCount, default 1,000,000, with a setter alongside the existing knobs) and a per-path cycle check, so both cases raise an ordinary catchable Error instead of killing the process. The change is additive: no valid document parses differently. All 8 real plist fixtures in the test suite parse identically and the existing tests pass. Co-Authored-By: Claude Opus 4.8 --- bplistParser.ts | 32 ++++++++++++++++++++++++++++++++ 1 file changed, 32 insertions(+) diff --git a/bplistParser.ts b/bplistParser.ts index 38fcbe8..0734de0 100644 --- a/bplistParser.ts +++ b/bplistParser.ts @@ -7,6 +7,12 @@ const debug = false; export let maxObjectSize = 100 * 1000 * 1000; // 100Meg export let maxObjectCount = 32768; +// Objects are addressed by index, so one stored object can be referenced many +// times. maxObjectCount bounds how many objects are *stored*, not how many are +// *expanded*, and a small file can therefore describe a very large tree (each +// level referencing the level below twice gives 2^n expansions from n objects). +// This caps the total expansion instead. +export let maxExpandedObjectCount = 1000000; // Exported bindings are read-only to consumers (an ESM import binding cannot be // assigned, and the CommonJS build exposes exports as getters), so these knobs @@ -19,6 +25,10 @@ export function setMaxObjectCount(value: number): void { maxObjectCount = value; } +export function setMaxExpandedObjectCount(value: number): void { + maxExpandedObjectCount = value; +} + // EPOCH = new SimpleDateFormat("yyyy MM dd zzz").parse("2001 01 01 GMT").getTime(); // ...but that's annoying in a static initializer because it can throw exceptions, ick. // So we just hardcode the correct value. @@ -126,7 +136,29 @@ export function parseBuffer(buffer: Buffer): [T] { // For the format specification check // // Apple's binary property list parser implementation. + // Guards against a small file describing an unbounded object graph: an + // expansion budget (a DAG that references each level twice expands to 2^n) + // and a cycle check (an object that transitively references itself would + // otherwise recurse until the stack overflows). + let expandedObjectCount = 0; + const objectsBeingParsed = new Set(); + function parseObject(tableOffset: number): any { + if (++expandedObjectCount > maxExpandedObjectCount) { + throw new Error("maxExpandedObjectCount exceeded"); + } + if (objectsBeingParsed.has(tableOffset)) { + throw new Error("Circular reference detected at object #" + tableOffset); + } + objectsBeingParsed.add(tableOffset); + try { + return parseObjectAt(tableOffset); + } finally { + objectsBeingParsed.delete(tableOffset); + } + } + + function parseObjectAt(tableOffset: number): any { const offset = offsetTable[tableOffset]; const type = buffer[offset]; const objType = (type & 0xF0) >> 4; //First 4 bits