From 45c45c43aae1ea66e18779e8e9133a50db1f2e7a Mon Sep 17 00:00:00 2001 From: Matt Van Horn <455140+mvanhorn@users.noreply.github.com> Date: Sun, 1 Nov 2026 17:06:48 -0800 Subject: [PATCH] fix(audit): keep readable plist text when one object is unreadable Throwing on one unreadable stored object discarded every string already decoded, so a UTF-16BE credential beside a truncated unused object was lost: the raw bytes that replaced it cannot match NUL-separated text. Return the decoded text with complete false instead, and keep the raw bytes in the scan beside it so ASCII in the unread object is still seen. The member stays unassessed. Text past the budget ends the same way. --- src/artifacts/ReleaseExposureReader.ts | 16 +++++++---- .../apple/BinaryPlistMaterializer.test.ts | 19 +++++++++---- .../apple/BinaryPlistMaterializer.ts | 28 +++++++++++++------ 3 files changed, 42 insertions(+), 21 deletions(-) diff --git a/src/artifacts/ReleaseExposureReader.ts b/src/artifacts/ReleaseExposureReader.ts index 0e9e1cd341..4863e83f78 100644 --- a/src/artifacts/ReleaseExposureReader.ts +++ b/src/artifacts/ReleaseExposureReader.ts @@ -1204,15 +1204,19 @@ interface MemberText { } /** - * A binary plist that cannot be decoded keeps its raw bytes in the scan, for the - * ASCII strings they show, but its other strings were not examined. + * A binary plist that cannot be fully decoded keeps its raw bytes in the scan, + * for the ASCII strings they show, beside whatever text it did decode, but its + * other strings were not examined. */ const decodeBinaryPlistMember = (bytes: Buffer): MemberText => { try { - return { - text: decodeBinaryPlistText(bytes, RELEASE_EXPOSURE_PLIST_DECODE_BYTES), - complete: true, - }; + const decoded = decodeBinaryPlistText( + bytes, + RELEASE_EXPOSURE_PLIST_DECODE_BYTES, + ); + return decoded.complete + ? decoded + : { text: `${decoded.text}\n${bytes.toString("utf8")}`, complete: false }; } catch { return { text: bytes.toString("utf8"), complete: false }; } diff --git a/src/artifacts/apple/BinaryPlistMaterializer.test.ts b/src/artifacts/apple/BinaryPlistMaterializer.test.ts index f52f12c441..5b924525e9 100644 --- a/src/artifacts/apple/BinaryPlistMaterializer.test.ts +++ b/src/artifacts/apple/BinaryPlistMaterializer.test.ts @@ -194,7 +194,7 @@ it("reads a string no object references", () => { text("kept"), text("orphan"), ]); - expect(decodeBinaryPlistText(bytes, budget)).toContain("orphan"); + expect(decodeBinaryPlistText(bytes, budget).text).toContain("orphan"); }); it("reads the value a later duplicate key replaced", () => { @@ -205,7 +205,7 @@ it("reads the value a later duplicate key replaced", () => { text("final"), ]); expect(decode(bytes)).toEqual({ key: "final" }); - expect(decodeBinaryPlistText(bytes, budget)).toContain("replaced"); + expect(decodeBinaryPlistText(bytes, budget).text).toContain("replaced"); }); it("reads an unreferenced UTF-16BE string and data object", () => { @@ -221,19 +221,23 @@ it("reads an unreferenced UTF-16BE string and data object", () => { wide, data, ]); - const decoded = decodeBinaryPlistText(bytes, budget); + const { text: decoded, complete } = decodeBinaryPlistText(bytes, budget); expect(decoded).toContain("é!"); expect(decoded).toContain("blob"); + expect(complete).toBe(true); }); -it("refuses an unreferenced string it cannot read", () => { +it("keeps readable text beside an unreferenced string it cannot read", () => { const bytes = source([ dictionary(1, 2), text("key"), text("kept"), Buffer.from([0x5f, 0x10, 0xff]), ]); - expect(() => decodeBinaryPlistText(bytes, budget)).toThrow(RangeError); + expect(decodeBinaryPlistText(bytes, budget)).toEqual({ + text: expect.stringContaining("kept"), + complete: false, + }); }); it("bounds the text that overlapping table entries repeat", () => { @@ -261,5 +265,8 @@ it("bounds the text that overlapping table entries repeat", () => { bytes[trailer + 7] = 1; bytes.writeBigUInt64BE(BigInt(offsets.length), trailer + 8); bytes.writeBigUInt64BE(BigInt(table), trailer + 24); - expect(() => decodeBinaryPlistText(bytes, 2 * budget)).toThrow(RangeError); + const decoded = decodeBinaryPlistText(bytes, 2 * budget); + expect(decoded.complete).toBe(false); + expect(decoded.text.length).toBeLessThanOrEqual(2 * budget + offsets.length); + expect(decoded.text).toContain("k"); }); diff --git a/src/artifacts/apple/BinaryPlistMaterializer.ts b/src/artifacts/apple/BinaryPlistMaterializer.ts index 8e28f7d433..2e5d9a53c5 100644 --- a/src/artifacts/apple/BinaryPlistMaterializer.ts +++ b/src/artifacts/apple/BinaryPlistMaterializer.ts @@ -139,13 +139,13 @@ const storedPlistText = ( offset: number, objectEnd: number, referenceWidth: number, -): { readonly text: string; readonly bytes: number } | null => { +): { readonly text: string; readonly bytes: number } | "unreadable" | null => { const marker = bytes[offset]; - if (marker === undefined) throw new RangeError("plist object is truncated"); + if (marker === undefined) return "unreadable"; const type = marker >> 4; if (type !== 4 && type !== 5 && type !== 6 && type !== 7) return null; const end = binaryPlistObjectEnd(bytes, offset, objectEnd, referenceWidth); - if (end === undefined) throw new RangeError("plist object is truncated"); + if (end === undefined) return "unreadable"; const payload = (marker & 0x0f) === 0x0f ? offset + 2 + (1 << (bytes[offset + 1]! & 0x0f)) @@ -160,24 +160,32 @@ const storedPlistText = ( return { text, bytes: stored.length }; }; +/** The text a binary plist stores; `complete` is false when part of it went unread. */ +export interface BinaryPlistText { + readonly text: string; + readonly complete: boolean; +} + /** * Decode every string, dictionary key and data payload a binary plist stores. * Its strings are stored per object as ASCII, UTF-16BE or UTF-8, so the raw * bytes do not show them all. This reads the object table rather than the * decoded value, because a value drops objects its root no longer references * and entries a later duplicate key replaced, and those bytes still ship. - * Throws on a plist that cannot be decoded within `maxBytes`, on a stored object - * it cannot read, and when overlapping table entries exceed `maxBytes` of text. + * A stored object it cannot read, or text past `maxBytes` from table entries + * that overlap, leaves the result incomplete and keeps the text already read. + * Throws on a plist that cannot be decoded within `maxBytes`. */ export const decodeBinaryPlistText = ( bytes: Buffer, maxBytes: number, -): string => { +): BinaryPlistText => { decodeBinaryPlistValue(bytes, preflightPropertyListDecode(bytes, maxBytes)); const layout = readBinaryPlistLayout(bytes); if (!layout.ok) throw new RangeError("binary plist trailer is truncated"); const texts: string[] = []; let decoded = 0; + let complete = true; for (const offset of new Set(layout.value.objectOffsets())) { const stored = storedPlistText( bytes, @@ -186,10 +194,12 @@ export const decodeBinaryPlistText = ( layout.value.referenceWidth, ); if (stored === null) continue; + if (stored === "unreadable" || decoded + stored.bytes > maxBytes) { + complete = false; + continue; + } decoded += stored.bytes; - if (decoded > maxBytes) - throw new RangeError("binary plist text exceeds the decode budget"); texts.push(stored.text); } - return texts.join("\n"); + return { text: texts.join("\n"), complete }; };