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 () => {