Merge pull request #885 from morluto/fix/plist-prototype-keys

fix(plist): read XML plists containing a __proto__ key

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<string> =>
   // `-n` strips only the newline plutil appends; it requires macOS 12+.
   (

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);
 };

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(
+      "<key>$top</key>",
+      "<key>__proto__</key><string>x</string><key>$top</key>",
+    );
+    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"), {

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)]),
+    ],
   };
 };
 

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) =>
+  `<?xml version="1.0" encoding="UTF-8"?><plist version="1.0">${body}</plist>`;
+
+it.each([
+  "<key>__proto__</key>",
+  "<key><![CDATA[__proto__]]></key>",
+  "<key>&#95;&#x5F;proto__</key>",
+])("omits and counts a dictionary keyed %s", (key) => {
+  const parsed = parseXmlPropertyList(
+    plist(
+      `<dict><key>CFBundleExecutable</key><string>App</string>${key}<dict><key>polluted</key><true/></dict><key>list</key><array><dict>${key}<string>x</string><key>kept</key><integer>1</integer></dict></array></dict>`,
+    ),
+  );
+  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(
+    "<dict><!-- <key>__proto__</key> --><key> __proto__ </key><string><![CDATA[<key>__proto__</key>]]></string><key>__rea_prototype_key__</key><string>__proto__</string></dict>",
+  );
+  expect(parseXmlPropertyList(text)).toEqual({
+    value: {
+      " __proto__ ": "<key>__proto__</key>",
+      __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.",
+  );
+});

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 =
+  /<!--[\s\S]*?-->|<!\[CDATA\[[\s\S]*?\]\]>|<key>([^<]*)<\/key>|<key><!\[CDATA\[([\s\S]*?)\]\]><\/key>/gu;
+
+const XML_ENTITIES: Readonly<Record<string, string>> = {
+  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
+        ? `<key>${placeholder}</key>`
+        : token,
+  );
+  return withoutKey(parse(substituted), placeholder);
+};

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("</plist>");
   if (start < 0 || end < start) return null;
   return jsonValueSchema.parse(
-    parseXmlPlist(output.slice(start, end + "</plist>".length)),
+    parseXmlPropertyList(output.slice(start, end + "</plist>".length)).value,
   );
 };
 

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<typeof omitPrototypeKeys>;
   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<ParsedPlist, AnalysisOutputError> => {
   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,

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", "<key>Blob</key><data>AAEC</data>"],
+    ] 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"),
+          `<plist><dict><key>CFBundleExecutable</key><string>App</string><key>__proto__</key><dict><key>polluted</key><true/></dict>${extra}</dict></plist>`,
+        );
+        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");
+      },
+    );
+  },
+);

tests/boundary/filesystem/binaryTarget.test.ts

@@ -243,6 +243,26 @@ describe("app executable filename fidelity", () => {
     });
   });
 
+  it.each([
+    "<key>__proto__</key><dict><key>CFBundleExecutable</key><string>Forged</string></dict>",
+    "<key>&#95;_proto__</key><string>x</string>",
+  ])("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"),
+      `<plist><dict>${entry}<key>CFBundleExecutable</key><string>App</string></dict></plist>`,
+    );
+    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 () => {