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.

1 files changed, 41 insertions, 0 deletions

diff --git a/test-suite/unit/upload-guard.test.mjs b/test-suite/unit/upload-guard.test.mjs
new file mode 100644
index 0000000..b2a0daa
--- /dev/null
+++ b/test-suite/unit/upload-guard.test.mjs
@@ -0,0 +1,41 @@
+import assert from "node:assert/strict";
+import fs from "node:fs";
+import os from "node:os";
+import path from "node:path";
+import { test } from "node:test";
+import { assertUploadsStaged } from "../../broker/upload-guard.mjs";
+
+const base = fs.realpathSync(fs.mkdtempSync(path.join(os.tmpdir(), "guard-")));
+const stage = path.join(base, "stage");
+fs.mkdirSync(stage);
+fs.writeFileSync(path.join(stage, "ok.txt"), "x");
+fs.writeFileSync(path.join(base, "secret.txt"), "s");
+fs.symlinkSync(path.join(base, "secret.txt"), path.join(stage, "link.txt"));
+
+const upload = (...paths) => () => assertUploadsStaged("page.upload", { paths }, stage);
+
+test("a file in the staging directory is allowed", () => {
+	assert.doesNotThrow(upload(path.join(stage, "ok.txt")));
+});
+test("a file elsewhere, a ../ escape, a symlink out, a relative path and a missing file are refused", () => {
+	assert.throws(upload(path.join(base, "secret.txt")), /limited to files/);
+	assert.throws(upload(path.join(stage, "..", "secret.txt")), /limited to files/);
+	assert.throws(upload(path.join(stage, "link.txt")), /limited to files/);
+	assert.throws(upload("ok.txt"), /limited to files/);
+	assert.throws(upload(path.join(stage, "nope.txt")), /limited to files/);
+});
+test("one bad path among good ones refuses the lot", () => {
+	assert.throws(upload(path.join(stage, "ok.txt"), "/etc/hosts"), /limited to files/);
+});
+test("a missing staging directory refuses everything", () => {
+	assert.throws(() => assertUploadsStaged("page.upload", { paths: [path.join(stage, "ok.txt")] }, path.join(base, "none")), /limited to files/);
+});
+test("raw DOM.setFileInputFiles through cdp.call is held to the same rule", () => {
+	const call = (files) => () => assertUploadsStaged("cdp.call", { method: "DOM.setFileInputFiles", params: { files } }, stage);
+	assert.doesNotThrow(call([path.join(stage, "ok.txt")]));
+	assert.throws(call(["/etc/hosts"]), /limited to files/);
+});
+test("other commands are untouched", () => {
+	assert.doesNotThrow(() => assertUploadsStaged("page.click", { paths: ["/etc/hosts"] }, stage));
+	assert.doesNotThrow(() => assertUploadsStaged("cdp.call", { method: "Runtime.evaluate", params: {} }, stage));
+});