From a08be51e9f0d23bd46b66bbdb88dbda44bc54c88 Mon Sep 17 00:00:00 2001 From: Iwakurakaoru0822 <208281322+kaoru0822-kitauji@users.noreply.github.com> Date: Tue, 6 Oct 2026 23:58:59 -0700 Subject: [PATCH] fix(plist): read XML plists containing a __proto__ key The `plist` XML decoder throws on the legal dictionary key `__proto__` (its CVE-2022-22912 guard). An app whose XML Info.plist held such an entry could not be opened at all ("app has no readable CFBundleExecutable"), and entitlements, XML keyed archives, and Interface Builder plists failed the same way. On the plutil JSON route, validation silently dropped the key from `inspect_plist` results. Decode XML plists through a shared reader that substitutes such keys before the decoder runs, then omits and counts those entries. `inspect_plist` and `inspect_keyed_archive` report the omission as a limitation instead of failing or dropping it silently. Co-Authored-By: Claude Opus 5.5 --- src/application/BinaryTargetResolver.ts | 23 ++--- src/application/InterfaceBuilderAnalysis.ts | 5 +- src/artifacts/KeyedArchiveReader.test.ts | 14 +++ src/artifacts/KeyedArchiveReader.ts | 21 +++-- src/domain/propertyListKeys.test.ts | 53 +++++++++++ src/domain/propertyListKeys.ts | 88 +++++++++++++++++++ src/native/NativeMacOSProvider.ts | 4 +- src/native/parsers/plist.ts | 25 ++++-- .../cli/nativePlistPrototypeKeyCli.test.ts | 53 +++++++++++ .../boundary/filesystem/binaryTarget.test.ts | 20 +++++ 10 files changed, 278 insertions(+), 28 deletions(-) create mode 100644 src/domain/propertyListKeys.test.ts create mode 100644 src/domain/propertyListKeys.ts create mode 100644 tests/boundary/cli/nativePlistPrototypeKeyCli.test.ts diff --git a/src/application/BinaryTargetResolver.ts b/src/application/BinaryTargetResolver.ts index 3107729b4..1b2fb8fdc 100644 --- a/src/application/BinaryTargetResolver.ts +++ b/src/application/BinaryTargetResolver.ts @@ -12,9 +12,10 @@ import { execFile } from "node:child_process"; import { extname, isAbsolute, join, resolve } from "node:path"; import { promisify } from "node:util"; -import { parse as parsePlist } from "plist"; +import { z } from "zod"; import { isPathWithinRoot } from "../domain/localPath.js"; +import { parseXmlPropertyList } from "../domain/propertyListKeys.js"; import { BinaryTargetError } from "../domain/configurationErrors.js"; import type { BinaryTarget } from "../domain/binaryTargetTypes.js"; import { err, ok, type Result } from "../domain/result.js"; @@ -268,21 +269,15 @@ const resolveAppBundle = async ( /** Decode the top-level executable name with an XML parser, not a pattern. */ const parseXmlPlistExecutable = (plist: string): string => { - const value = parsePlist(plist); - const executable = - typeof value === "object" && - value !== null && - !Array.isArray(value) && - !(value instanceof Date) && - !(value instanceof Uint8Array) && - Object.hasOwn(value, "CFBundleExecutable") - ? value.CFBundleExecutable - : undefined; - if (typeof executable !== "string") - throw new Error("CFBundleExecutable is missing"); - return executable; + // An unrelated `__proto__` entry must not make the bundle unreadable. + const { value } = parseXmlPropertyList(plist); + const executable = executableEntrySchema.safeParse(value); + if (!executable.success) throw new Error("CFBundleExecutable is missing"); + return executable.data.CFBundleExecutable; }; +const executableEntrySchema = z.looseObject({ CFBundleExecutable: z.string() }); + const readBinaryPlistExecutable = async (plistPath: string): Promise => // `-n` strips only the newline plutil appends; it requires macOS 12+. ( diff --git a/src/application/InterfaceBuilderAnalysis.ts b/src/application/InterfaceBuilderAnalysis.ts index 5b64623f4..697a1fd8f 100644 --- a/src/application/InterfaceBuilderAnalysis.ts +++ b/src/application/InterfaceBuilderAnalysis.ts @@ -1,7 +1,8 @@ import { createHash } from "node:crypto"; -import { parse, parseBinary } from "plist"; +import { parseBinary } from "plist"; import { DirectoryArtifactReader } from "../artifacts/DirectoryArtifactReader.js"; +import { parseXmlPropertyList } from "../domain/propertyListKeys.js"; import { decodeNibArchive, type NibArchiveDocument, @@ -373,6 +374,6 @@ const decodePlist = (bytes: Buffer): JsonValue => { const parsed = bytes.subarray(0, 8).toString("ascii") === "bplist00" ? parseBinary(bytes) - : parse(bytes.toString("utf8")); + : parseXmlPropertyList(bytes.toString("utf8")).value; return jsonValueSchema.parse(parsed); }; diff --git a/src/artifacts/KeyedArchiveReader.test.ts b/src/artifacts/KeyedArchiveReader.test.ts index 52fa72ef0..870dc55e7 100644 --- a/src/artifacts/KeyedArchiveReader.test.ts +++ b/src/artifacts/KeyedArchiveReader.test.ts @@ -86,6 +86,20 @@ describe("inert keyed archive decoding", () => { ]); }, ); + it("reports an XML archive dictionary keyed __proto__ as omitted", () => { + const xml = build({ ...archive, $objects: ["$null", "value"] }).replace( + "$top", + "__proto__x$top", + ); + const graph = decodeKeyedArchiveBytes(Buffer.from(xml), { + offset: 0, + limit: 2, + }); + expect(graph.objects.map(({ value }) => value)).toEqual(["$null", "value"]); + expect(graph.limitations).toContain( + "1 dictionary entry keyed __proto__ was omitted because REA results cannot represent that key.", + ); + }); it("rejects missing roots, malformed plist, and non-keyed archives", () => { expect(() => decodeKeyedArchiveBytes(Buffer.from("bplist00bad"), { diff --git a/src/artifacts/KeyedArchiveReader.ts b/src/artifacts/KeyedArchiveReader.ts index e85f71960..b96253d5d 100644 --- a/src/artifacts/KeyedArchiveReader.ts +++ b/src/artifacts/KeyedArchiveReader.ts @@ -1,9 +1,13 @@ import { createHash } from "node:crypto"; import { TextDecoder } from "node:util"; -import { parse, parseBinary } from "plist"; +import { parseBinary } from "plist"; import { z } from "zod"; import type { JsonValue } from "../domain/jsonValue.js"; import { projectPlistValue } from "../domain/plistValue.js"; +import { + omittedPrototypeKeysLimitation, + parseXmlPropertyList, +} from "../domain/propertyListKeys.js"; import { keyedArchiveInputSchema, keyedArchiveResultSchema, @@ -22,12 +26,19 @@ export const decodeKeyedArchiveBytes = ( if (bytes.length > MAX_BYTES) throw new RangeError("Keyed archive exceeds 64 MiB"); const binary = bytes.subarray(0, 8).toString("ascii") === "bplist00"; - const parsed: unknown = binary - ? parseBinary(bytes) - : parse(decodeXmlPlistText(bytes)); + const parsed = binary + ? { value: parseBinary(bytes), omittedPrototypeKeys: 0 } + : parseXmlPropertyList(decodeXmlPlistText(bytes)); + const graph = projectKeyedArchive(normalizePlist(parsed.value), selection); return { archive_format: binary ? ("binary-plist" as const) : ("xml-plist" as const), - ...projectKeyedArchive(normalizePlist(parsed), selection), + ...graph, + limitations: [ + ...graph.limitations, + ...(parsed.omittedPrototypeKeys === 0 + ? [] + : [omittedPrototypeKeysLimitation(parsed.omittedPrototypeKeys)]), + ], }; }; diff --git a/src/domain/propertyListKeys.test.ts b/src/domain/propertyListKeys.test.ts new file mode 100644 index 000000000..e54e57472 --- /dev/null +++ b/src/domain/propertyListKeys.test.ts @@ -0,0 +1,53 @@ +import { expect, it } from "vitest"; + +import { + omitPrototypeKeys, + omittedPrototypeKeysLimitation, + parseXmlPropertyList, +} from "./propertyListKeys.js"; + +const plist = (body: string) => + `${body}`; + +it.each([ + "__proto__", + "", + "__proto__", +])("omits and counts a dictionary keyed %s", (key) => { + const parsed = parseXmlPropertyList( + plist( + `CFBundleExecutableApp${key}pollutedlist${key}xkept1`, + ), + ); + expect(parsed).toEqual({ + value: { CFBundleExecutable: "App", list: [{ kept: 1 }] }, + omittedPrototypeKeys: 2, + }); + expect(Object.getPrototypeOf(parsed.value)).toBe(Object.prototype); +}); + +it("keeps __proto__ text in strings, comments, and similar keys", () => { + const text = plist( + " __proto__ __proto__]]>__rea_prototype_key____proto__", + ); + expect(parseXmlPropertyList(text)).toEqual({ + value: { + " __proto__ ": "__proto__", + __rea_prototype_key__: "__proto__", + }, + omittedPrototypeKeys: 0, + }); +}); + +it("omits own __proto__ entries that JSON decoding creates", () => { + const decoded: unknown = JSON.parse( + '{"__proto__":{"x":1},"a":[{"__proto__":2,"b":3}]}', + ); + expect(omitPrototypeKeys(decoded)).toEqual({ + value: { a: [{ b: 3 }] }, + omittedPrototypeKeys: 2, + }); + expect(omittedPrototypeKeysLimitation(1)).toBe( + "1 dictionary entry keyed __proto__ was omitted because REA results cannot represent that key.", + ); +}); diff --git a/src/domain/propertyListKeys.ts b/src/domain/propertyListKeys.ts new file mode 100644 index 000000000..bd09fd44b --- /dev/null +++ b/src/domain/propertyListKeys.ts @@ -0,0 +1,88 @@ +import { parse } from "plist"; + +/** The dictionary key REA results cannot carry: validation strips it. */ +const PROTOTYPE_KEY = "__proto__"; + +/** A decoded plist without `__proto__` entries, and how many were omitted. */ +export interface PropertyListWithoutPrototypeKeys { + readonly value: unknown; + readonly omittedPrototypeKeys: number; +} + +/** Limitation for results whose source plist held `__proto__` entries. */ +export const omittedPrototypeKeysLimitation = (count: number): string => + `${String(count)} dictionary ${count === 1 ? "entry" : "entries"} keyed __proto__ ${count === 1 ? "was" : "were"} omitted because REA results cannot represent that key.`; + +const withoutKey = ( + value: unknown, + key: string, +): PropertyListWithoutPrototypeKeys => { + let omitted = 0; + const strip = (item: unknown): unknown => { + if (Array.isArray(item)) return item.map(strip); + if ( + item === null || + typeof item !== "object" || + item instanceof Date || + item instanceof Uint8Array + ) + return item; + return Object.fromEntries( + Object.entries(item).flatMap(([name, entry]) => { + if (name !== key) return [[name, strip(entry)]]; + omitted += 1; + return []; + }), + ); + }; + const stripped = strip(value); + return { value: stripped, omittedPrototypeKeys: omitted }; +}; + +/** Remove own `__proto__` entries, such as JSON.parse creates, and count them. */ +export const omitPrototypeKeys = ( + value: unknown, +): PropertyListWithoutPrototypeKeys => withoutKey(value, PROTOTYPE_KEY); + +// Comments and CDATA text are consumed whole so only element keys match. +const KEY_TOKEN = + /||([^<]*)<\/key>|<\/key>/gu; + +const XML_ENTITIES: Readonly> = { + amp: "&", + apos: "'", + gt: ">", + lt: "<", + quot: '"', +}; + +const decodeXmlText = (text: string): string => + text.replace( + /&(?:#x([\da-f]+)|#(\d+)|([a-z]+));/giu, + (entity, hex?: string, decimal?: string, name?: string) => { + if (hex !== undefined) return String.fromCodePoint(parseInt(hex, 16)); + if (decimal !== undefined) + return String.fromCodePoint(parseInt(decimal, 10)); + return (name === undefined ? undefined : XML_ENTITIES[name]) ?? entity; + }, + ); + +/** + * Decode an XML plist whose dictionaries may use the legal key `__proto__`, + * which the `plist` decoder rejects outright. Those entries are omitted and + * counted so callers can report them instead of failing the whole document. + */ +export const parseXmlPropertyList = ( + text: string, +): PropertyListWithoutPrototypeKeys => { + let placeholder = "__rea_prototype_key__"; + while (text.includes(placeholder)) placeholder = `_${placeholder}`; + const substituted = text.replace( + KEY_TOKEN, + (token, plain?: string, cdata?: string) => + (plain === undefined ? cdata : decodeXmlText(plain)) === PROTOTYPE_KEY + ? `${placeholder}` + : token, + ); + return withoutKey(parse(substituted), placeholder); +}; diff --git a/src/native/NativeMacOSProvider.ts b/src/native/NativeMacOSProvider.ts index ec22fa1dd..78d0b2bb7 100644 --- a/src/native/NativeMacOSProvider.ts +++ b/src/native/NativeMacOSProvider.ts @@ -8,7 +8,6 @@ import { observeNativeUi } from "./NativeUiObservation.js"; import { dirname, isAbsolute, resolve } from "node:path"; import { open, realpath, stat } from "node:fs/promises"; -import { parse as parseXmlPlist } from "plist"; import { z } from "zod"; import { @@ -35,6 +34,7 @@ import { BinaryTargetError } from "../domain/configurationErrors.js"; import { ProviderAdapterError } from "../domain/providerAdapterError.js"; import { AnalysisError } from "../domain/analysisErrorBase.js"; import { jsonValueSchema, type JsonValue } from "../domain/jsonValue.js"; +import { parseXmlPropertyList } from "../domain/propertyListKeys.js"; import { demangleSwiftSchema, inspectPlistSchema, @@ -645,7 +645,7 @@ const parseEntitlements = (output: string): JsonValue | null => { const end = output.lastIndexOf(""); if (start < 0 || end < start) return null; return jsonValueSchema.parse( - parseXmlPlist(output.slice(start, end + "".length)), + parseXmlPropertyList(output.slice(start, end + "".length)).value, ); }; diff --git a/src/native/parsers/plist.ts b/src/native/parsers/plist.ts index f14181454..977e7909a 100644 --- a/src/native/parsers/plist.ts +++ b/src/native/parsers/plist.ts @@ -1,4 +1,3 @@ -import { parse as parseXmlPlist } from "plist"; import { z } from "zod"; import { AnalysisOutputError } from "../../domain/analysisErrorCore.js"; @@ -7,6 +6,11 @@ import { projectPlistValue, type ProjectedPlistValue, } from "../../domain/plistValue.js"; +import { + omitPrototypeKeys, + omittedPrototypeKeysLimitation, + parseXmlPropertyList, +} from "../../domain/propertyListKeys.js"; import { err, ok, type Result } from "../../domain/result.js"; const plistObject = z.record(z.string(), z.unknown()); @@ -99,9 +103,9 @@ export const parsePlistJson = ( ambiguousNumberCount += 1; return item; }; - let value: unknown; + let decoded: ReturnType; try { - value = decodePlistJson(output, classify); + decoded = omitPrototypeKeys(decodePlistJson(output, classify)); } catch (cause: unknown) { return err( new AnalysisOutputError( @@ -111,10 +115,14 @@ export const parsePlistJson = ( ), ); } + const { value } = decoded; return ok({ value, bundle: projectPlistBundle(value), - limitations: numberLimitations(exactIntegerCount, ambiguousNumberCount), + limitations: [ + ...numberLimitations(exactIntegerCount, ambiguousNumberCount), + ...prototypeKeyLimitations(decoded.omittedPrototypeKeys), + ], }); }; @@ -126,8 +134,11 @@ export const parsePlistXml = ( output: string, ): Result => { let projected: ProjectedPlistValue; + let omittedPrototypeKeys: number; try { - projected = projectPlistValue(parseXmlPlist(output)); + const decoded = parseXmlPropertyList(output); + omittedPrototypeKeys = decoded.omittedPrototypeKeys; + projected = projectPlistValue(decoded.value); } catch (cause: unknown) { return err( new AnalysisOutputError( @@ -168,6 +179,7 @@ export const parsePlistXml = ( `${String(projected.unknownRealCount)} non-finite real value(s) are reported as { "$plist_type": "real", "value": null } because the XML decoder does not distinguish NaN from infinity.`, ]), ...numberLimitations(exactIntegerCount, ambiguousNumberCount), + ...prototypeKeyLimitations(omittedPrototypeKeys), ], }); }; @@ -201,6 +213,9 @@ const xmlNumberLiterals = (xml: string): XmlNumberLiterals => { return { integers, reals }; }; +const prototypeKeyLimitations = (count: number): string[] => + count === 0 ? [] : [omittedPrototypeKeysLimitation(count)]; + const numberLimitations = ( exactIntegerCount: number, ambiguousNumberCount: number, diff --git a/tests/boundary/cli/nativePlistPrototypeKeyCli.test.ts b/tests/boundary/cli/nativePlistPrototypeKeyCli.test.ts new file mode 100644 index 000000000..0a75f0e38 --- /dev/null +++ b/tests/boundary/cli/nativePlistPrototypeKeyCli.test.ts @@ -0,0 +1,53 @@ +import { mkdir, writeFile } from "node:fs/promises"; +import { join } from "node:path"; + +import { describe, expect } from "vitest"; + +import { thinMach } from "../../../src/domain/binaryTarget.fixture.js"; +import { createTestTempDirectory } from "../../fixtures/temporaryDirectory.js"; +import { cliTest } from "../../support/cli/cliFixture.js"; + +const OMITTED = + "1 dictionary entry keyed __proto__ was omitted because REA results cannot represent that key."; + +describe.skipIf(process.platform !== "darwin")( + "native plist CLI with a __proto__ key", + () => { + cliTest.for([ + ["JSON-expressible", ""], + ["data-bearing", "BlobAAEC"], + ] as const)( + "inspects a $0 plist and reports the omitted key", + async ([, extra], { cli }) => { + const directory = await createTestTempDirectory("rea-plist-proto-"); + const app = join(directory, "Proto.app"); + const contents = join(app, "Contents"); + await mkdir(join(contents, "MacOS"), { recursive: true }); + await writeFile( + join(contents, "MacOS/App"), + thinMach(0xfeedfacf, 0x100000c), + ); + await writeFile( + join(contents, "Info.plist"), + `CFBundleExecutableApp__proto__polluted${extra}`, + ); + const result = await cli.run({ + arguments: ["inspect-plist", app, "--json"], + environment: { + REA_LOG_LEVEL: "silent", + REA_ANALYSIS_PROVIDER: "auto", + }, + }); + expect(result.exitCode).toBe(0); + expect(result.json).toMatchObject({ + normalized_result: { + value: { CFBundleExecutable: "App" }, + bundle: { executable: "App" }, + limitations: expect.arrayContaining([OMITTED]), + }, + }); + expect(JSON.stringify(result.json)).not.toContain("polluted"); + }, + ); + }, +); diff --git a/tests/boundary/filesystem/binaryTarget.test.ts b/tests/boundary/filesystem/binaryTarget.test.ts index 8597420a5..81f9126df 100644 --- a/tests/boundary/filesystem/binaryTarget.test.ts +++ b/tests/boundary/filesystem/binaryTarget.test.ts @@ -243,6 +243,26 @@ describe("app executable filename fidelity", () => { }); }); + it.each([ + "__proto__CFBundleExecutableForged", + "__proto__x", + ])("opens an app whose XML plist also holds %s", async (entry) => { + const directory = await createTestTempDirectory("rea-app-xml-proto-"); + const app = join(directory, "Proto.app"); + const contents = join(app, "Contents"); + const executable = join(contents, "MacOS", "App"); + await mkdir(join(contents, "MacOS"), { recursive: true }); + await writeFile( + join(contents, "Info.plist"), + `${entry}CFBundleExecutableApp`, + ); + await writeFile(executable, thinMach(0xfeedfacf, 0x0100000c)); + const result = await parseBinaryTarget(app, directory, "arm64"); + expect(result.ok && result.value).toMatchObject({ + path: await realpath(executable), + }); + }); + it.skipIf(process.platform !== "darwin")( "preserves executable filename whitespace from native binary plists", async () => {