From cee95b2ae6a19122de95103b2d23bc6f8c9c4428 Mon Sep 17 00:00:00 2001 From: morluto <76467478+morluto@users.noreply.github.com> Date: Mon, 19 Oct 2026 02:28:27 +0800 Subject: [PATCH] fix(plist): preserve source semantics in XML data decoding Adapt the completed-quartet decoder from PR #4981 at 4cf6cac14. Bind raw source spans to validated DOM nodes before entity and line-ending interpretation can change data bytes. Preserve depth preflight, reject invalid data forms and allocate exact decoded output through one shared quartet grammar. Fixes #4978 Co-authored-by: Avichal Pandey --- docs/native-investigation.md | 6 + src/domain/propertyListKeys.ts | 57 +------- src/domain/propertyListXml.test.ts | 14 ++ src/domain/propertyListXml.ts | 15 +- src/domain/propertyListXmlData.test.ts | 78 +++++++++++ src/domain/propertyListXmlData.ts | 65 +++++++++ src/domain/propertyListXmlSource.ts | 128 ++++++++++++++++++ .../keyedArchiveXmlNumbers.test.ts | 73 +++++++++- 8 files changed, 374 insertions(+), 62 deletions(-) create mode 100644 src/domain/propertyListXmlData.test.ts create mode 100644 src/domain/propertyListXmlData.ts create mode 100644 src/domain/propertyListXmlSource.ts diff --git a/docs/native-investigation.md b/docs/native-investigation.md index 938c62dc2..dee54059d 100644 --- a/docs/native-investigation.md +++ b/docs/native-investigation.md @@ -337,6 +337,12 @@ complete observation. Object `kind` describes the decoded archive value: data, dates and numbers remain `scalar` when their JSON value uses a `$plist_type` wrapper. An actual dictionary with the same fields remains `dictionary`. + XML must be well-formed. Data literals use + [CoreFoundation's raw ASCII quartet rules](https://github.com/swiftlang/swift-corelibs-foundation/blob/384ff680be7c7445fee6ce0c55e9ae11a46e07c5/Sources/CoreFoundation/CFPropertyList.c#L1588): + non-alphabet ASCII characters are ignored and an incomplete final quartet + emits no bytes. Character references retain their source spelling for data + decoding. Non-ASCII content, child markup and self-closing `` are format + failures; explicit `` represents empty data. UID 0 cannot distinguish conditional nil from ordinary nil. Limits are 64 MiB of input, 200,000 objects/references and 128 nesting levels. XML dictionary entries decoded as `__proto__` are omitted with an explicit diff --git a/src/domain/propertyListKeys.ts b/src/domain/propertyListKeys.ts index 551391f46..d1c5cbcf3 100644 --- a/src/domain/propertyListKeys.ts +++ b/src/domain/propertyListKeys.ts @@ -1,4 +1,3 @@ -import { normalizeLineEndings, type Document, type Node } from "@xmldom/xmldom"; import { parseOpenStep } from "plist"; import { decodeXmlPropertyList } from "./propertyListXml.js"; @@ -9,7 +8,6 @@ import { } from "./apple/plistNumbers.js"; import { MAX_PLIST_DEPTH } from "./apple/plistValue.js"; -import { parseXmlDocument } from "./xmlDocument.js"; /** The dictionary key REA results cannot carry: validation strips it. */ const PROTOTYPE_KEY = "__proto__"; @@ -108,20 +106,7 @@ export const parseXmlPropertyList = (text: string): ParsedPropertyList => { omittedPrototypeKeys: prototypeKeys, }; } - // Normalize only the literal line endings the pinned decoder normalizes. - // Decode this DOM directly so character references are never reserialized - // into literal line endings and normalized a second time. - const normalized = normalizeLineEndings(text); - // Strict syntax owns well-formedness before value interpretation. Reuse the - // same DOM for depth, keys, values and number facts. - const parsed = parseXmlDocument(normalized, { - doctype: "allow", - lineEndings: "preserve", - }); - if (!parsed.ok) throw parsed.error; - const document = parsed.value; - assertXmlPlistDepthWithinLimit(document); - const decoded = decodeXmlPropertyList(document); + const decoded = decodeXmlPropertyList(text); return { ...omitPrototypeKeys(decoded.value), numberLiterals: decoded.numberLiterals, @@ -288,43 +273,3 @@ const skipOpenStepScalar = (text: string, start: number): number => { } return cursor; }; - -/** Elements whose children the plist value decoder visits recursively. */ -const PLIST_CONTAINER_ELEMENTS = new Set(["plist", "dict", "array"]); - -/** - * Reject XML plists nested deeper than the projection supports before any - * recursive value pass runs. Value decoding and prototype omission recurse once - * per nesting level, so a deeply nested document exhausts the call stack - * before the projection limit can report the resource bound. The failure - * matches `projectPlistValue` so callers classify it as a limit failure. - * - * Depth counts values the way the projection does: the root value is level 0 - * and every container member is one level deeper. A dictionary key names a - * member instead of nesting one, and a self-closing element is the value at - * its own level. The outer plist envelope is unwrapped by the decoder; - * nested plist elements decode as arrays and must count toward the bound. - */ -const assertXmlPlistDepthWithinLimit = (document: Document): void => { - const root = document.documentElement; - if (root === null) return; - const pending: Array<{ node: Node; depth: number }> = [ - { node: root, depth: -1 }, - ]; - while (pending.length > 0) { - const entry = pending.pop(); - if (entry === undefined) break; - const { node, depth } = entry; - // Schedule only the next sibling, keeping traversal storage proportional - // to depth rather than the number of members in a wide container. - if (node.nextSibling !== null) - pending.push({ node: node.nextSibling, depth }); - if (node.nodeType !== node.ELEMENT_NODE) continue; - if (node.nodeName !== "key" && depth > MAX_PLIST_DEPTH) - throw new RangeError( - `Plist exceeds ${String(MAX_PLIST_DEPTH)} nesting levels`, - ); - if (PLIST_CONTAINER_ELEMENTS.has(node.nodeName) && node.firstChild !== null) - pending.push({ node: node.firstChild, depth: depth + 1 }); - } -}; diff --git a/src/domain/propertyListXml.test.ts b/src/domain/propertyListXml.test.ts index 845211dbb..fa08815a2 100644 --- a/src/domain/propertyListXml.test.ts +++ b/src/domain/propertyListXml.test.ts @@ -92,6 +92,20 @@ it.each([ }, ); +it.each([ + ["AQID", [1, 2, 3]], + [" A Q I D\n", [1, 2, 3]], + ["AQI", []], + ["AQ!ID", [1, 2, 3]], + ["AQ==", [1]], + ["AQI=", [1, 2]], + ["AQ= =\n", [1]], + ["", []], +])("decodes data text %j as CFPropertyList does", (text, expected) => { + const parsed = parseXmlPropertyList(`${text}`); + expect(parsed.value).toEqual(Uint8Array.from(expected)); +}); + it("collects source integers and CDATA reals before rounding and duplicate assignment", () => { const parsed = parseXmlPropertyList( "a0x20000000000001a9007199254740992b9007199254", diff --git a/src/domain/propertyListXml.ts b/src/domain/propertyListXml.ts index 62db805a3..6ebf462d9 100644 --- a/src/domain/propertyListXml.ts +++ b/src/domain/propertyListXml.ts @@ -1,4 +1,6 @@ -import type { Document, Node } from "@xmldom/xmldom"; +import type { Node } from "@xmldom/xmldom"; + +import { readXmlPlistSource } from "./propertyListXmlSource.js"; import { UnknownPlistReal, @@ -7,8 +9,10 @@ import { parsePlistNonFiniteRealLiteral, } from "./apple/plistNumbers.js"; -/** Interpret an already validated, depth-bounded DOM, owning values and number facts together. */ -export const decodeXmlPropertyList = (document: Document) => { +/** Own source interpretation, depth, values and numeric facts together. */ +export const decodeXmlPropertyList = (text: string) => { + const source = readXmlPlistSource(text); + const document = source.document; const root = document.documentElement; if (root?.nodeName !== "plist") throw new TypeError("malformed document. First element should be "); @@ -134,8 +138,9 @@ export const decodeXmlPropertyList = (document: Document) => { return value; } case "data": { - const binary = atob(content(node, textNode).text.replace(/\s+/gu, "")); - return Uint8Array.from(binary, (character) => character.charCodeAt(0)); + const value = source.dataValue(node); + if (!value.ok) throw value.error; + return value.value; } case "date": { const value = requireContent(node).nodeValue; diff --git a/src/domain/propertyListXmlData.test.ts b/src/domain/propertyListXmlData.test.ts new file mode 100644 index 000000000..a1e0f95d0 --- /dev/null +++ b/src/domain/propertyListXmlData.test.ts @@ -0,0 +1,78 @@ +import { expect, it } from "vitest"; +import { parseXmlPropertyList } from "./propertyListKeys.js"; + +// Expected bytes were independently read from plutil's binary conversion. +it.each([ + ["AQID", "eb9408"], + ["&AQID", "6a6a40"], + ["AQ ID", "010d74"], + ["éAQID", "c44f40"], + ["====", "00"], + ["=AQI", "000408"], + ["AQI=Z", "0102"], + ["AQ=!==", "0100"], +])( + "retains CFPropertyList data bytes from source spelling %j", + (literal, hex) => { + const parsed = parseXmlPropertyList( + `${literal}`, + ); + expect(parsed.value).toEqual(Uint8Array.from(Buffer.from(hex, "hex"))); + }, +); + +it.each(["é", "😀", "\u00a0", "\u0085", "\u2028", "\u2029"])( + "rejects non-ASCII data before source normalization can hide %j", + (character) => { + expect(() => + parseXmlPropertyList(`AQ${character}ID`), + ).toThrow( + expect.objectContaining({ + _tag: "XmlPlistDataError", + reason: "non-ascii", + characterOffset: 2, + }), + ); + }, +); + +it.each([ + "AQID", + "", + "AQID", + "AQID", +])( + "rejects data markup instead of silently decoding a different source: %j", + (literal) => { + expect(() => + parseXmlPropertyList(`${literal}`), + ).toThrow(expect.objectContaining({ reason: "markup" })); + }, +); + +it.each(["", ' note=">"', " note='>/'"])( + "distinguishes explicit empty data from a self-closing element with attributes %j", + (attributes) => { + expect( + parseXmlPropertyList(``).value, + ).toEqual(new Uint8Array()); + expect(() => + parseXmlPropertyList(``), + ).toThrow(expect.objectContaining({ reason: "empty-element" })); + }, +); + +it.each(["\n", "\r\n", "\r", "\u0085", "\u2028", "\u2029"])( + "binds raw data after astral text, entities and line ending %j to the right node", + (ending) => { + const parsed = parseXmlPropertyList( + `π😀 ${ending}before${ending}` + + "AQIDAQID", + ); + expect(parsed.value).toEqual([ + expect.any(String), + Uint8Array.from([235, 148, 8]), + Uint8Array.from([1, 2, 3]), + ]); + }, +); diff --git a/src/domain/propertyListXmlData.ts b/src/domain/propertyListXmlData.ts new file mode 100644 index 000000000..81d5a0a3f --- /dev/null +++ b/src/domain/propertyListXmlData.ts @@ -0,0 +1,65 @@ +import { err, ok, type Result } from "./result.js"; + +export class XmlPlistDataError extends TypeError { + readonly _tag = "XmlPlistDataError"; + constructor( + readonly reason: + | "non-ascii" + | "markup" + | "empty-element" + | "source-location", + /** UTF-16 offset within the raw data literal, when known. */ + readonly characterOffset: number | null = null, + ) { + super(`Cannot decode XML plist : ${reason}`); + this.name = "XmlPlistDataError"; + } +} + +const BASE64_DIGITS = + "ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789+/"; +const C_WHITESPACE = new Set([" ", "\t", "\n", "\v", "\f", "\r"]); + +/** Interpret each completed quartet once per pass, including CF's padding resets. */ +const visitDataQuartets = ( + text: string, + visit: (value: number, byteCount: 1 | 2 | 3) => void, +): Result => { + let quartet = 0; + let digits = 0; + let padding = 0; + for (let index = 0; index < text.length; index += 1) { + if (text.charCodeAt(index) >= 128) + return err(new XmlPlistDataError("non-ascii", index)); + const character = text[index]!; + if (character === "=") padding += 1; + else if (!C_WHITESPACE.has(character)) padding = 0; + const digit = character === "=" ? 0 : BASE64_DIGITS.indexOf(character); + if (digit < 0) continue; + quartet = ((quartet << 6) | digit) & 0xffffff; + digits += 1; + if (digits % 4 === 0) + visit(quartet, padding >= 2 ? 1 : padding === 1 ? 2 : 3); + } + return ok(undefined); +}; + +/** CFPropertyList reads raw ASCII, skips nonalphabet bytes and emits complete quartets. */ +export const decodeXmlPlistData = ( + text: string, +): Result => { + let length = 0; + const counted = visitDataQuartets(text, (_value, byteCount) => { + length += byteCount; + }); + if (!counted.ok) return counted; + // Count actual output before allocation, including padding in every quartet. + const bytes = new Uint8Array(length); + let offset = 0; + const decoded = visitDataQuartets(text, (value, byteCount) => { + bytes[offset++] = value >> 16; + if (byteCount >= 2) bytes[offset++] = value >> 8; + if (byteCount === 3) bytes[offset++] = value; + }); + return decoded.ok ? ok(bytes) : decoded; +}; diff --git a/src/domain/propertyListXmlSource.ts b/src/domain/propertyListXmlSource.ts new file mode 100644 index 000000000..6f180d27e --- /dev/null +++ b/src/domain/propertyListXmlSource.ts @@ -0,0 +1,128 @@ +import type { Document, Node } from "@xmldom/xmldom"; +import { MAX_PLIST_DEPTH } from "./apple/plistValue.js"; +import { err, ok, type Result } from "./result.js"; +import { parseXmlDocument } from "./xmlDocument.js"; +import { + decodeXmlPlistData, + XmlPlistDataError, +} from "./propertyListXmlData.js"; + +// Preserve the established XML decoder normalization; keep raw data separately. +const LINE_ENDING = /\r[\n\u0085]|[\r\n\u0085\u2028\u2029]/gu; + +interface DataSpan { + readonly start: number; + readonly end: number; +} + +/** One source owner binds lexical data values to their validated DOM nodes. */ +export const readXmlPlistSource = (text: string) => { + const normalized = text.replace(LINE_ENDING, "\n"); + const parsed = parseXmlDocument(normalized, { + doctype: "allow", + lineEndings: "preserve", + }); + if (!parsed.ok) throw parsed.error; + const document = parsed.value; + const lineEndings = new RegExp(LINE_ENDING); + let line = 1; + let lineStart = 0; + // The depth walk visits elements in source order, so location lookup is + // linear without an index entry for every line in a potentially large value. + const readData = ( + node: Node, + ): Result => { + if (node.lineNumber === undefined || node.columnNumber === undefined) + return err("source-location"); + while (line < node.lineNumber) { + const ending = lineEndings.exec(text); + if (ending === null) return err("source-location"); + lineStart = ending.index + ending[0].length; + line += 1; + } + if (line !== node.lineNumber) return err("source-location"); + for (let child = node.firstChild; child !== null; child = child.nextSibling) + if (child.nodeType !== child.TEXT_NODE) return err("markup"); + const start = lineStart + node.columnNumber - 1; + const openingEnd = openingTagEnd(text, start); + if (openingEnd < 0) return err("source-location"); + if (text[openingEnd - 1] === "/") return err("empty-element"); + const end = text.indexOf("<", openingEnd + 1); + if (end < 0) return err("source-location"); + return ok({ start: openingEnd + 1, end }); + }; + const data = collectXmlPlistDataSpans(document, readData); + return { + document, + dataValue(node: Node): Result { + const span = data.get(node); + if (span === undefined) + return err(new XmlPlistDataError("source-location")); + return span.ok + ? decodeXmlPlistData(text.slice(span.value.start, span.value.end)) + : err(new XmlPlistDataError(span.error)); + }, + }; +}; + +/** The validated DOM omits the distinction between and . */ +const openingTagEnd = (source: string, start: number): number => { + let quote: string | undefined; + for (let cursor = start; cursor < source.length; cursor += 1) { + const character = source[cursor]; + if (quote !== undefined) { + if (character === quote) quote = undefined; + } else if (character === '"' || character === "'") quote = character; + else if (character === ">") return cursor; + } + return -1; +}; + +/** Elements whose children the plist value decoder visits recursively. */ +const PLIST_CONTAINER_ELEMENTS = new Set(["plist", "dict", "array"]); + +/** + * Reject XML plists nested deeper than the projection supports before any + * recursive value pass runs. Value decoding and prototype omission recurse once + * per nesting level, so a deeply nested document exhausts the call stack + * before the projection limit can report the resource bound. The failure + * matches `projectPlistValue` so callers classify it as a limit failure. + * + * Depth counts values the way the projection does: the root value is level 0 + * and every container member is one level deeper. A dictionary key names a + * member instead of nesting one, and a self-closing element is the value at + * its own level. The outer plist envelope is unwrapped by the decoder; + * nested plist elements decode as arrays and must count toward the bound. + */ +const collectXmlPlistDataSpans = ( + document: Document, + readData: (node: Node) => Result, +): WeakMap> => { + const data = new WeakMap< + Node, + Result + >(); + const root = document.documentElement; + if (root === null) return data; + const pending: Array<{ node: Node; depth: number }> = [ + { node: root, depth: -1 }, + ]; + while (pending.length > 0) { + const entry = pending.pop(); + if (entry === undefined) break; + const { node, depth } = entry; + // Schedule only the next sibling, keeping traversal storage proportional + // to depth rather than the number of members in a wide container. + if (node.nextSibling !== null) + pending.push({ node: node.nextSibling, depth }); + if (node.nodeType !== node.ELEMENT_NODE) continue; + if (node.nodeName !== "key" && depth > MAX_PLIST_DEPTH) + throw new RangeError( + `Plist exceeds ${String(MAX_PLIST_DEPTH)} nesting levels`, + ); + if (node.nodeName === "data") data.set(node, readData(node)); + if (PLIST_CONTAINER_ELEMENTS.has(node.nodeName) && node.firstChild !== null) + pending.push({ node: node.firstChild, depth: depth + 1 }); + } + return data; +}; diff --git a/tests/acceptance/applications/keyedArchiveXmlNumbers.test.ts b/tests/acceptance/applications/keyedArchiveXmlNumbers.test.ts index 1a1d501f0..a9dfaf705 100644 --- a/tests/acceptance/applications/keyedArchiveXmlNumbers.test.ts +++ b/tests/acceptance/applications/keyedArchiveXmlNumbers.test.ts @@ -39,8 +39,28 @@ cliTest.for([ element: `1${"0".repeat(400)}`, expected: { $plist_type: "integer", decimal: `1${"0".repeat(400)}` }, }, + { + name: "incomplete data quartet", + element: "AQI", + expected: { $plist_type: "data", base64: "" }, + }, + { + name: "data with a non-alphabet character", + element: "AQ!ID", + expected: { $plist_type: "data", base64: "AQID" }, + }, + { + name: "data with a numeric entity spelling", + element: "AQID", + expected: { $plist_type: "data", base64: "65QI" }, + }, + { + name: "data with a predefined entity spelling", + element: "&AQID", + expected: { $plist_type: "data", base64: "ampA" }, + }, ])( - "preserves $name XML numbers through CLI and MCP", + "preserves $name XML scalars through CLI and MCP", async (scenario, { cli }) => { const root = await createTestTempDirectory("rea-keyed-xml-numbers-"); const bytes = Buffer.from( @@ -116,3 +136,54 @@ cliTest.for([ expect(await readFile(path)).toEqual(bytes); }, ); + +cliTest( + "refuses invalid data without publishing observed bytes through CLI and MCP", + async ({ cli }) => { + const root = await createTestTempDirectory("rea-keyed-data-refusal-"); + const { call } = await connectLocalToolsMcp({ + HOME: root, + USERPROFILE: root, + }); + for (const [name, element, reason] of [ + ["non-ascii", "AQ\u0085ID", "non-ascii"], + ["markup", "AQID", "markup"], + ["self-closing", '', "empty-element"], + ] as const) { + const path = join(root, `${name}.plist`); + const bytes = Buffer.from( + build({ + $archiver: "NSKeyedArchiver", + $version: 100000, + $objects: ["$null", "__VALUE__"], + $top: { root: { CF$UID: 1 } }, + }).replace("__VALUE__", element), + ); + await writeFile(path, bytes); + const output = await cli.run({ + arguments: ["inspect-keyed-archive", path, "--json"], + environment: { HOME: root, USERPROFILE: root, REA_LOG_LEVEL: "silent" }, + }); + expect(output.exitCode).toBe(1); + expect(output.json).toMatchObject({ + code: "artifact_operation_failed", + details: { reason: "format", detail: expect.stringContaining(reason) }, + }); + expect((await call("open_binary", { path })).isError).not.toBe(true); + const reply = await call("inspect_keyed_archive", {}); + expect(reply.isError).toBe(true); + const block = reply.content.find((item) => item.type === "text"); + if (block?.type !== "text") throw new Error("Missing MCP error text"); + expect(JSON.parse(block.text)).toMatchObject({ + error: { + code: "artifact_operation_failed", + details: { + reason: "format", + detail: expect.stringContaining(reason), + }, + }, + }); + expect(await readFile(path)).toEqual(bytes); + } + }, +);