fix(adb): validate screen capture PNG headers (#7962)

* fix(adb): validate screen capture PNG headers * fix(adb): state screen header validation limits * style(adb): format screen capture assertion Match the current formatter after integrating main. Co-authored-by: morluto <morluto@users.noreply.github.com> --------- Co-authored-by: Cursor Agent <cursoragent@cursor.com> Co-authored-by: morluto <morluto@users.noreply.github.com>
2026-11-03 04:29:07 +00:00 · 2026-10-29 16:04:49 +08:00
1 parent 2b85a4a2ba
commit f7cd4e5256
@@ -103,6 +103,10 @@ existing analysis families directly, for example
 | `start_adb_activity`    | **mutates device** | Starts an intent (`am start -a` with optional URI, component, and up to 16 typed extras); preserves the activity manager's verdict                                                                                                    |
 | `stop_adb_app`          | **mutates device** | Force-stops every process of a package, ending a dynamic-analysis run                                                                                                                                                                 |

+Screen capture validates the PNG signature, IHDR framing, dimensions, and IHDR
+checksum before publishing. It does not validate encoding fields or decode and
+validate image contents; that limitation is retained in Evidence.
+
 When `push_adb_file` receives an existing directory, adb places the local file
 inside it under the local basename; Evidence reports that effective file path
 while retaining the caller's directory path in the input parameters. The
@@ -38,6 +38,11 @@ import type { AdbRequest } from "../domain/adb/adbDeviceAnalysis.js";

 const it = test.skipIf(process.platform === "win32");

+const SCREEN_PNG = Buffer.from(
+  "iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAIAAACQd1PeAAAADElEQVR4nGP4z8AAAAMBAQDJ/pLvAAAAAElFTkSuQmCC",
+  "base64",
+);
+
 const VERSION_BLOCK =
  "printf 'Android Debug Bridge version 1.0.41\\nVersion 34.0.5-stub\\nInstalled as %s\\n' \"$0\"";

@@ -1453,19 +1458,20 @@ exit 1`,

 it("captures the screen as a digested PNG with its dimensions", async () => {
  const { binary, root } = await writeAdbStub(
-    () => `if [ "$1" = "version" ]; then ${VERSION_BLOCK}; exit 0; fi
+    (root) => `if [ "$1" = "version" ]; then ${VERSION_BLOCK}; exit 0; fi
 if [ "$1" = "-s" ] && [ "$3" = "exec-out" ]; then
-  printf '\\211PNG\\r\\n\\032\\n\\0\\0\\0\\rIHDR\\0\\0\\0\\004\\0\\0\\0\\003'
+  cat "${join(root, "screen-fixture.png")}"
  exit 0
 fi
 exit 1`,
  );
+  await writeFile(join(root, "screen-fixture.png"), SCREEN_PNG);
  const output = join(root, "captures");
  const result = await evidenceResult(providerOver(binary), {
    operation: "capture_adb_screen",
    input: { serial: "emulator-5554", output_directory: output },
  });
-  expect(result).toMatchObject({ width: 4, height: 3 });
+  expect(result).toMatchObject({ width: 1, height: 1 });
  expect(
    await readFile(join(output, "screen.png")).then(
      (bytes) => bytes.subarray(1, 4).toString("latin1") === "PNG",
@@ -1476,13 +1482,14 @@ exit 1`,

 it("stops a screen capture aborted after adb returns but before publication", async () => {
  const { binary, root } = await writeAdbStub(
-    () => `if [ "$1" = "version" ]; then ${VERSION_BLOCK}; exit 0; fi
+    (root) => `if [ "$1" = "version" ]; then ${VERSION_BLOCK}; exit 0; fi
 if [ "$1" = "-s" ] && [ "$3" = "exec-out" ]; then
-  printf '\\211PNG\\r\\n\\032\\n\\0\\0\\0\\rIHDR\\0\\0\\0\\004\\0\\0\\0\\003'
+  cat "${join(root, "screen-fixture.png")}"
  exit 0
 fi
 exit 1`,
  );
+  await writeFile(join(root, "screen-fixture.png"), SCREEN_PNG);
  const output = join(root, "cancelled-capture");
  const controller = new AbortController();
  adbTestHooks.afterScreencap = () => controller.abort();
@@ -66,6 +66,7 @@ import {
 } from "../domain/analysisErrorCore.js";
 import { AnalysisError } from "../domain/analysisErrorBase.js";
 import { analysisErrorWithCleanupFailure } from "../domain/analysisErrorCleanup.js";
+import { readPngHeaderDimensions } from "../domain/pngHeader.js";
 import { err, ok, type Result } from "../domain/result.js";
 import type { JsonValue } from "../domain/jsonValue.js";
 import { jsonObjectSchema } from "../domain/jsonValue.js";
@@ -132,7 +133,6 @@ import {
  parseFeatureListOutput,
  parsePackageChangeOutput,
  parseProcessListOutput,
-  parsePngDimensions,
  parseResolveActivityOutput,
  parseServiceListOutput,
  parseSettingsGetOutput,
@@ -3114,11 +3114,11 @@ export class AdbProvider {
      return err(
        new AnalysisCancelledError(operation, { cause: signal.reason }),
      );
-    const dimensions = parsePngDimensions(captured.value.stdout);
-    if (dimensions.width === null)
+    const dimensions = readPngHeaderDimensions(captured.value.stdout);
+    if (dimensions === undefined)
      return err(
        new AnalysisProtocolError(
-          "screencap did not return a PNG payload; the device may not expose a display",
+          "screencap returned an invalid PNG header; the device may not expose a display",
        ),
      );
    return this.#withRuntimeRoot(
@@ -3182,6 +3182,7 @@ export class AdbProvider {
          {
            limitations: [
              "The capture is one frame at read time; no recording or interaction occurred",
+              "PNG signature, IHDR framing, checksum, and dimensions are validated; encoding and image contents are not validated",
              ...(usedExclusiveFallback
                ? [
                    "The exclusive-create fallback makes the output path visible while its bytes are being written",
@@ -609,26 +609,3 @@ export const parseAmStartOutput = (
      (text === "" ? "am start returned no activity" : text.slice(0, 512)),
  };
 };
-
-const PNG_SIGNATURE = Buffer.from([
-  0x89, 0x50, 0x4e, 0x47, 0x0d, 0x0a, 0x1a, 0x0a,
-]);
-
-/** Read image dimensions from a PNG header, or null for other payloads. */
-export const parsePngDimensions = (
-  bytes: Uint8Array,
-): {
-  width: number | null;
-  height: number | null;
-} => {
-  if (
-    bytes.length < 24 ||
-    !Buffer.from(bytes.subarray(0, 8)).equals(PNG_SIGNATURE)
-  )
-    return { width: null, height: null };
-  const view = new DataView(bytes.buffer, bytes.byteOffset, bytes.byteLength);
-  return {
-    width: view.getUint32(16),
-    height: view.getUint32(20),
-  };
-};
@@ -1,10 +1,12 @@
-import { chmod, lstat, readFile, unlink } from "node:fs/promises";
+import { chmod, lstat, readFile, unlink, writeFile } from "node:fs/promises";
 import { createHash } from "node:crypto";
+import { crc32 } from "node:zlib";
 import { basename, dirname, join } from "node:path";
 import { expect, it, onTestFinished } from "vitest";
 import { z } from "zod";

 import { toolContract } from "../../../src/contracts/toolContracts.js";
+import { adbResultSchemas } from "../../../src/domain/adb/adbDeviceAnalysis.js";
 import { createTestTempDirectory } from "../../fixtures/temporaryDirectory.js";
 import {
  execFileOutput,
@@ -19,6 +21,93 @@ import {
  removeTestWorkspace,
 } from "../../support/workspace/workspaceFixture.js";

+it
+  .skipIf(process.platform === "win32")
+  .each([
+    "zero width",
+    "zero height",
+    "invalid chunk type",
+    "invalid checksum",
+    "truncated header",
+    "valid",
+  ] as const)(
+  "validates the screencap PNG header before publishing: %s",
+  async (kind) => {
+    const workspace = await createTestWorkspace("rea-adb-screen-cli-");
+    onTestFinished(() => removeTestWorkspace(workspace.root));
+    const png = Buffer.from(
+      "iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAIAAACQd1PeAAAADElEQVR4nGP4z8AAAAMBAQDJ/pLvAAAAAElFTkSuQmCC",
+      "base64",
+    );
+    if (kind === "zero width") png.writeUInt32BE(0, 16);
+    if (kind === "zero height") png.writeUInt32BE(0, 20);
+    if (kind === "invalid chunk type") png[12] = 0xc9;
+    if (kind !== "invalid checksum")
+      png.writeUInt32BE(crc32(png.subarray(12, 29)), 29);
+    else png[29] = png[29]! ^ 0xff;
+    const frame = kind === "truncated header" ? png.subarray(0, 24) : png;
+    await writeFile(workspace.path("frame.png"), frame);
+    const adb = await workspace.write(
+      "adb",
+      `#!/bin/sh
+if [ "$1" = version ]; then printf 'Android Debug Bridge version 1.0.41\\nVersion 34.0.5-cli\\n'; exit 0; fi
+if [ "$1" = -s ] && [ "$3" = exec-out ]; then /bin/cat "$(dirname "$0")/frame.png"; exit 0; fi
+exit 1
+`,
+    );
+    await chmod(adb, 0o755);
+    const outputPath = workspace.path("captures");
+    const result = await execFileOutput(
+      process.execPath,
+      [
+        "scripts/rea.mjs",
+        "capture-adb-screen",
+        "device",
+        "--output-directory",
+        outputPath,
+        "--json",
+      ],
+      {
+        env: { ...process.env, REA_ADB_PATH: adb },
+        timeout: 20_000,
+        stopSignal: "SIGTERM",
+        maxBuffer: 1024 * 1024,
+      },
+    ).then(
+      (output) => ({ exitCode: 0, stdout: output.stdout }),
+      (cause: unknown) => {
+        const failure = execFileOutputFailure(cause);
+        if (failure === undefined) throw cause;
+        return { exitCode: failure.code, stdout: failure.stdout };
+      },
+    );
+    if (kind === "valid") {
+      expect(result.exitCode).toBe(0);
+      const output = JSON.parse(result.stdout) as {
+        normalized_result: unknown;
+        limitations: string[];
+      };
+      expect(
+        adbResultSchemas.capture_adb_screen.parse(output.normalized_result),
+      ).toMatchObject({ width: 1, height: 1 });
+      expect(output.limitations).toContain(
+        "PNG signature, IHDR framing, checksum, and dimensions are validated; encoding and image contents are not validated",
+      );
+      expect(
+        (await readFile(join(outputPath, "screen.png"))).equals(frame),
+      ).toBe(true);
+    } else {
+      expect(result.exitCode).toBe(1);
+      expect(JSON.parse(result.stdout)).toMatchObject({
+        code: "unreadable_output",
+      });
+      await expect(
+        readFile(join(outputPath, "screen.png")),
+      ).rejects.toMatchObject({ code: "ENOENT" });
+    }
+  },
+);
+
 it.skipIf(process.platform === "win32").each([
  { exit: 1, stdout: "", stderr: "adb: error: closed", overwrite: false },
  { exit: 1, stdout: "", stderr: "adb: error: closed", overwrite: true },
@@ -50,7 +50,7 @@ const fixture = async () => {
    root,
    provider,
    png: Buffer.from(
-      "iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAQAAAC1HAwCAAAAC0lEQVR42mP8/x8AAwMCAO+jv4sAAAAASUVORK5CYII=",
+      "iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAIAAACQd1PeAAAADElEQVR4nGP4z8AAAAMBAQDJ/pLvAAAAAElFTkSuQmCC",
      "base64",
    ),
  };
@@ -20,8 +20,8 @@ const sourceBytes = "selected native bytes";
 const pulledBytes = "pulled fixture";
 const bugreportBytes = "bugreport fixture";
 const png = Buffer.from(
-  "89504e470d0a1a0a0000000d494844520000000400000003",
-  "hex",
+  "iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAIAAACQd1PeAAAADElEQVR4nGP4z8AAAAMBAQDJ/pLvAAAAAElFTkSuQmCC",
+  "base64",
 );
 const digest = (bytes: string | Buffer) =>
  createHash("sha256").update(bytes).digest("hex");
@@ -97,6 +97,7 @@ const fixture = async () => {
  await mkdir(join(physical, "nested"), { recursive: true });
  await symlink(join(physical, "nested"), join(root, "alias"));
  const binary = join(root, "adb");
+  await writeFile(join(root, "screen-fixture.png"), png);
  const quote = (value: string) => `'${value.replaceAll("'", "'\\''")}'`;
  await writeFile(
    binary,
@@ -110,7 +111,7 @@ if [ "$3" = push ] || [ "$3" = install ]; then
  printf 'Success\\n'; exit 0
 fi
 if [ "$3" = pull ]; then printf '${pulledBytes}' > "$5"; exit 0; fi
-if [ "$3" = exec-out ]; then printf '\\211PNG\\r\\n\\032\\n\\0\\0\\0\\rIHDR\\0\\0\\0\\004\\0\\0\\0\\003'; exit 0; fi
+if [ "$3" = exec-out ]; then /bin/cat ${quote(join(root, "screen-fixture.png"))}; exit 0; fi
 if [ "$3" = bugreport ]; then
  printf '${bugreportBytes}' > "$4/bugreport.zip"
  printf 'Bug report copied to %s/bugreport.zip\\n' "$4"; exit 0