Security: path traversal in download facade via server-controlled suggestedFilename (arbitrary local file read)

[shani-singh1]

Summary

page.waitForEvent("download") builds the on-disk download path by joining the remote-controlled suggestedFilename onto the download directory with no sanitization. Because suggestedFilename comes from the downloaded response (the server's Content-Disposition header), a malicious or compromised page can make download.path() and download.saveAs() operate on an arbitrary absolute path outside the download directory. In a harness whose whole purpose is driving untrusted web pages, this is an arbitrary local file read / exfiltration primitive that crosses the web-content to host boundary.

Affected code

package/ego-browser/src/driver/downloads.ts (in waitForDownload):

const suggestedFilename =
  willBegin.params?.suggestedFilename || guid || "download";
...
const downloadedPath = join(downloadDir, suggestedFilename);   // no basename / no ".." rejection
return {
  suggestedFilename: () => suggestedFilename,
  url: () => willBegin.params?.url || "",
  path: async () => downloadedPath,
  saveAs: async (targetPath) => {
    await copyFile(downloadedPath, targetPath);   // copies FROM the attacker-chosen path
    return targetPath;
  },
};

suggestedFilename originates from the CDP Page.downloadWillBegin event, which Chromium populates from the response Content-Disposition filename. It is untrusted input, and it is joined into a filesystem path directly. There is no basename, no .. rejection, and no containment check anywhere in the download path (grep across src/driver/ confirms none).

Proof (harness level, no browser required)

The harness ships a mock CDP bridge used by downloads.test.mjs. Firing a Page.downloadWillBegin whose suggestedFilename contains traversal shows the harness produces an out-of-directory path:

fireEvent("Page.downloadWillBegin", {
  guid: "dl-1",
  url: "https://attacker.example/report",
  suggestedFilename: "../../../../../../../../etc/passwd",
});
fireEvent("Page.downloadProgress", { guid: "dl-1", state: "completed" });
const download = await promise;
console.log(await download.path());

Observed output (run against the current source on this repo's own test harness):

download.path() => /etc/passwd      # on the macOS target; on a Windows dev box it resolves to C:\etc\passwd
escapes download dir => true

So download.path() points completely outside the per-download temp directory, and download.saveAs(dest) would copyFile from that attacker-chosen path. An agent doing an ordinary "download this file and read it" task would instead read/exfiltrate whatever local file the page names (for example ~/.ssh/id_rsa, ~/.aws/credentials, /etc/passwd).

Reproduction against a live browser

1. Serve a download whose header carries a traversal filename:

   Content-Disposition: attachment; filename="../../../../../../../../etc/passwd"
   

2. From an agent task, trigger and consume the download:

   const dl = await page.waitForEvent("download")
   await page.locator("#malicious-link").click()
   const d = await dl
   console.log(await d.path())            // attacker-controlled absolute path
   // or: await d.saveAs("/tmp/out")      // copies the arbitrary source file

Impact

Arbitrary local file read driven entirely by web content, crossing the untrusted-page to host boundary. Severity is high: the harness is designed to drive untrusted pages, and downloads are a normal agent task.

Honest scope note: the harness-level defect (trusting and joining the value unsanitized) is proven above and is fixable regardless. The end-to-end live exploit additionally depends on Chromium passing path separators through the CDP suggestedFilename field rather than pre-sanitizing them. Playwright hardens against exactly this by sanitizing the suggested filename before using it as a path, which is why the same defensive fix is appropriate here whether or not a given Chromium build filters the value.

Suggested fix

Reduce suggestedFilename to a safe basename (strip directory components, reject ./../empty) before joining, and fall back to the download guid or a static name. This keeps the file inside the per-download directory while leaving the reported suggestedFilename() value unchanged.

I will open a PR against dev with the fix plus a regression test.