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.
This commit is contained in:
Matt Van Horn committed 2026-11-01 17:06:48 -08:00
1 parent 9e06282e3a
commit 45c45c43aa
3 files changed
+42 -21

No files matched your search

+10 -6
View File
@@ -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 };
}
@@ -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");
});
+19 -9
View File
@@ -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 };
};