broker: refuse uploads from outside the staging directory

page.upload and a raw DOM.setFileInputFiles through cdp.call now have to name
files whose real path is under /tmp/pi-chrome-uploads (PI_CHROME_UPLOAD_DIR),
on the broker's host. Anyone holding the token could otherwise attach any file
the user can read to a page.

2 files changed, 52 insertions, 0 deletions

diff --git a/broker/broker.mjs b/broker/broker.mjs
index 4af8e9d..36fa302 100644
--- a/broker/broker.mjs
+++ b/broker/broker.mjs
@@ -27,6 +27,7 @@ import { execSync } from "node:child_process";
 import { createHash, timingSafeEqual } from "node:crypto";
 import { dirname, resolve } from "node:path";
 import { fileURLToPath } from "node:url";
+import { assertUploadsStaged } from "./upload-guard.mjs";
 
 const PKG_PATH = resolve(dirname(fileURLToPath(import.meta.url)), "..", "package.json");
 
@@ -236,6 +237,13 @@ async function handleCommand(request, response) {
 		sendJson(response, 400, { ok: false, error: "Missing command action" });
 		return;
 	}
+	try {
+		assertUploadsStaged(body.action, body.params, process.env.PI_CHROME_UPLOAD_DIR?.trim() || undefined);
+	} catch (error) {
+		log("warn", "command", { action: body.action, peer: request.socket.remoteAddress, ok: false, refused: "upload outside the staging directory" });
+		sendJson(response, 403, { ok: false, error: error.message });
+		return;
+	}
 	const timeoutMs = Math.min(Math.max(Number(body.timeoutMs) || DEFAULT_TIMEOUT_MS, 1_000), MAX_TIMEOUT_MS);
 	const started = Date.now();
 	const peer = request.socket.remoteAddress;
diff --git a/broker/upload-guard.mjs b/broker/upload-guard.mjs
new file mode 100644
index 0000000..dde8cdf
--- /dev/null
+++ b/broker/upload-guard.mjs
@@ -0,0 +1,44 @@
+// Which local files a command may hand to the browser's file inputs.
+//
+// The broker runs on the browser's host, so a path in an upload command names a file on that
+// machine, and nothing else checks it: the companion extension passes it to
+// DOM.setFileInputFiles as given. Anyone holding the broker's token could attach any file the
+// user can read to a page. Clients are told to stage uploads in one directory; this enforces it.
+//
+// Covers chrome_upload_file (page.upload) and the same CDP method through chrome_cdp (cdp.call).
+// Paths are resolved with realpath first, so a symlink or ".." cannot lead out of the directory.
+
+import { realpathSync } from "node:fs";
+import { sep } from "node:path";
+
+export const DEFAULT_UPLOAD_DIR = "/tmp/pi-chrome-uploads";
+
+function resolveReal(path) {
+	try {
+		return realpathSync(path);
+	} catch {
+		return undefined; // missing: nothing to upload, and not ours to guess at
+	}
+}
+
+function pathsOf(action, params) {
+	if (action === "page.upload") return Array.isArray(params?.paths) ? params.paths : [];
+	if (action === "cdp.call" && params?.method === "DOM.setFileInputFiles") {
+		const files = params.params?.files;
+		return Array.isArray(files) ? files : [];
+	}
+	return [];
+}
+
+// Throws if a command would upload a file from outside uploadDir.
+export function assertUploadsStaged(action, params, uploadDir = DEFAULT_UPLOAD_DIR) {
+	const paths = pathsOf(action, params);
+	if (paths.length === 0) return;
+	const root = resolveReal(uploadDir);
+	for (const path of paths) {
+		const real = typeof path === "string" && path.startsWith("/") ? resolveReal(path) : undefined;
+		if (root === undefined || real === undefined || !real.startsWith(root + sep)) {
+			throw new Error(`Uploads are limited to files in ${uploadDir} on the browser's host; "${path}" is not one. Copy the file there first.`);
+		}
+	}
+}