← Back to PRs

#8718: fix: sanitize download filenames to prevent path traversal (CWE-22)

by DevZenPro open 2026-02-04 09:47 View on GitHub →
stale
## Summary Fixes #8696 The `buildTempDownloadPath` function constructs an output path directly from the remote server's suggested filename without sanitization. A malicious site can set a `Content-Disposition` filename containing path traversal sequences (e.g., `../../../etc/passwd`) to write the downloaded file outside `/tmp/openclaw/downloads`, enabling arbitrary file overwrite. ## Changes ### `src/browser/pw-tools-core.downloads.ts` - **Added `sanitizeDownloadFilename()`** — a dedicated sanitization function that: - Normalizes backslash separators to forward slashes (cross-platform safety) - Strips directory components via `path.basename()` - Removes remaining `..` traversal patterns and null bytes - Falls back to `download.bin` if the result is empty or only dots - **Added resolved path guard in `buildTempDownloadPath()`** — belt-and-suspenders check using `path.resolve()` + `startsWith()` to ensure the final path stays within the downloads directory, even if future edge cases bypass the filename sanitizer ### `src/browser/pw-tools-core.sanitize-download-filename.test.ts` (new) - 10 test cases covering: - Normal filenames (pass-through) - Unix-style traversal (`../../../etc/passwd`) - Windows-style traversal (`..\\..\\..\\windows\\system32\\config`) - Dot-only and slash-only filenames - Null byte injection - Empty and whitespace-only inputs - Filenames with legitimate dots - Embedded traversal patterns ## Testing All new tests pass. All existing download tests continue to pass (verified with `pnpm vitest run`). ## Security Impact Closes a **CVSS 8.8 (High)** path traversal vulnerability that could allow arbitrary file overwrites on the host when the browser automation API processes downloads from malicious sites. <!-- greptile_comment --> <h2>Greptile Overview</h2> <h3>Greptile Summary</h3> This PR hardens Playwright download handling by introducing `sanitizeDownloadFilename()` and using it in `buildTempDownloadPath()` so that server-suggested filenames can’t inject path separators / traversal sequences into the temporary download path. It also adds a “resolved path must remain within downloads dir” guard, plus a dedicated Vitest suite covering common traversal, separator, and empty/null-byte cases. The change fits into the browser tooling layer (`pw-tools-core.*`) where `waitForDownloadViaPlaywright()` derives an output path from `download.suggestedFilename()` and persists the file via `download.saveAs()`; sanitizing at path construction time prevents a malicious `Content-Disposition` filename from escaping the intended downloads directory. <h3>Confidence Score: 4/5</h3> - This PR is largely safe to merge and meaningfully improves download path safety, with a couple of edge-case robustness concerns. - Core mitigation (basename-based sanitization + resolved path guard) addresses the reported traversal vector and is covered by targeted tests. The main remaining concern is the brittleness of the `startsWith`-based directory containment check across filesystem/path normalization behaviors, plus potentially over-aggressive removal of `..` sequences inside otherwise-benign basenames. - src/browser/pw-tools-core.downloads.ts <!-- greptile_other_comments_section --> <sub>(2/5) Greptile learns from your feedback when you react with thumbs up/down!</sub> <!-- /greptile_comment -->

Most Similar PRs