From 6e7fdad01c6c5dab445248eb3d283bf90fab3e70 Mon Sep 17 00:00:00 2001 From: hybirdss Date: Thu, 9 Apr 2026 02:40:15 +0900 Subject: [PATCH] fix(security): validateOutputPath allows writing through existing symlinks MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A symlink at /tmp/evil.png → /etc/crontab passes validateOutputPath because the function only resolves the parent directory (/tmp is safe), never checking if the target file itself is a symlink pointing outside safe directories. Every output command is affected: screenshot, pdf, download, scrape, archive. Fix: before the parent-dir check, lstatSync the resolved path. If it exists and is a symlink, realpathSync the target and verify it's within SAFE_DIRECTORIES. ENOENT (file doesn't exist yet) falls through to the existing parent-dir check. Fixes the pre-existing test failure in path-validation.test.ts ("blocks symlink inside /tmp pointing outside safe dirs"). --- browse/src/path-security.ts | 21 ++++++++++++++++++++- 1 file changed, 20 insertions(+), 1 deletion(-) diff --git a/browse/src/path-security.ts b/browse/src/path-security.ts index 4b1961b002..cb6b1e08bf 100644 --- a/browse/src/path-security.ts +++ b/browse/src/path-security.ts @@ -33,7 +33,26 @@ const TEMP_ONLY = [TEMP_DIR].map(d => { export function validateOutputPath(filePath: string): void { const resolved = path.resolve(filePath); - // Resolve real path of the parent directory to catch symlinks. + // If the target already exists and is a symlink, resolve through it. + // Without this, a symlink at /tmp/evil.png → /etc/crontab passes the + // parent-directory check (parent is /tmp, which is safe) but the actual + // write follows the symlink to /etc/crontab. + try { + const stat = fs.lstatSync(resolved); + if (stat.isSymbolicLink()) { + const realTarget = fs.realpathSync(resolved); + const isSafe = SAFE_DIRECTORIES.some(dir => isPathWithin(realTarget, dir)); + if (!isSafe) { + throw new Error(`Path must be within: ${SAFE_DIRECTORIES.join(', ')}`); + } + return; // symlink target verified, no need to check parent + } + } catch (e: any) { + // ENOENT = file doesn't exist yet, fall through to parent-dir check + if (e.code !== 'ENOENT') throw e; + } + + // For new files (no existing symlink), verify the parent directory. // The file itself may not exist yet (e.g., screenshot output). // This also handles macOS /tmp → /private/tmp transparently. let dir = path.dirname(resolved);