From 47a8277567a2c72c64434a5e133507a9de93160c Mon Sep 17 00:00:00 2001 From: Garry Tan Date: Thu, 12 Mar 2026 20:49:34 -0700 Subject: [PATCH] security: fix path validation bypass, CORS restriction, cookie-import path check MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - startsWith('/tmp') matched '/tmpevil' — now requires trailing slash - CORS Access-Control-Allow-Origin changed from * to http://127.0.0.1: - cookie-import now validates file paths (was missing validateReadPath) - 3 new tests for prefix collision and cookie-import path traversal Co-Authored-By: Claude Opus 4.6 --- browse/src/cookie-picker-routes.ts | 4 ++-- browse/src/meta-commands.ts | 2 +- browse/src/read-commands.ts | 3 ++- browse/src/write-commands.ts | 11 +++++++++++ browse/test/commands.test.ts | 28 ++++++++++++++++++++++++++++ 5 files changed, 44 insertions(+), 4 deletions(-) diff --git a/browse/src/cookie-picker-routes.ts b/browse/src/cookie-picker-routes.ts index 1eb9d59f..e185af0c 100644 --- a/browse/src/cookie-picker-routes.ts +++ b/browse/src/cookie-picker-routes.ts @@ -31,7 +31,7 @@ function jsonResponse(data: any, status = 200): Response { status, headers: { 'Content-Type': 'application/json', - 'Access-Control-Allow-Origin': '*', + 'Access-Control-Allow-Origin': `http://127.0.0.1:${parseInt(url.port, 10) || 9400}`, }, }); } @@ -54,7 +54,7 @@ export async function handleCookiePickerRoute( return new Response(null, { status: 204, headers: { - 'Access-Control-Allow-Origin': '*', + 'Access-Control-Allow-Origin': `http://127.0.0.1:${parseInt(url.port, 10) || 9400}`, 'Access-Control-Allow-Methods': 'GET, POST, OPTIONS', 'Access-Control-Allow-Headers': 'Content-Type', }, diff --git a/browse/src/meta-commands.ts b/browse/src/meta-commands.ts index 74076c48..b9b39848 100644 --- a/browse/src/meta-commands.ts +++ b/browse/src/meta-commands.ts @@ -14,7 +14,7 @@ const SAFE_DIRECTORIES = ['/tmp', process.cwd()]; function validateOutputPath(filePath: string): void { const resolved = path.resolve(filePath); - const isSafe = SAFE_DIRECTORIES.some(dir => resolved.startsWith(dir)); + const isSafe = SAFE_DIRECTORIES.some(dir => resolved === dir || resolved.startsWith(dir + '/')); if (!isSafe) { throw new Error(`Path must be within: ${SAFE_DIRECTORIES.join(', ')}`); } diff --git a/browse/src/read-commands.ts b/browse/src/read-commands.ts index 5926fe13..31d1018f 100644 --- a/browse/src/read-commands.ts +++ b/browse/src/read-commands.ts @@ -16,7 +16,8 @@ const SAFE_DIRECTORIES = ['/tmp', process.cwd()]; function validateReadPath(filePath: string): void { if (path.isAbsolute(filePath)) { - const isSafe = SAFE_DIRECTORIES.some(dir => path.resolve(filePath).startsWith(dir)); + const resolved = path.resolve(filePath); + const isSafe = SAFE_DIRECTORIES.some(dir => resolved === dir || resolved.startsWith(dir + '/')); if (!isSafe) { throw new Error(`Absolute path must be within: ${SAFE_DIRECTORIES.join(', ')}`); } diff --git a/browse/src/write-commands.ts b/browse/src/write-commands.ts index d8319e14..08c94253 100644 --- a/browse/src/write-commands.ts +++ b/browse/src/write-commands.ts @@ -236,6 +236,17 @@ export async function handleWriteCommand( case 'cookie-import': { const filePath = args[0]; if (!filePath) throw new Error('Usage: browse cookie-import '); + // Path validation — prevent reading arbitrary files + if (path.isAbsolute(filePath)) { + const safeDirs = ['/tmp', process.cwd()]; + const resolved = path.resolve(filePath); + if (!safeDirs.some(dir => resolved === dir || resolved.startsWith(dir + '/'))) { + throw new Error(`Path must be within: ${safeDirs.join(', ')}`); + } + } + if (path.normalize(filePath).includes('..')) { + throw new Error('Path traversal sequences (..) are not allowed'); + } if (!fs.existsSync(filePath)) throw new Error(`File not found: ${filePath}`); const raw = fs.readFileSync(filePath, 'utf-8'); let cookies: any[]; diff --git a/browse/test/commands.test.ts b/browse/test/commands.test.ts index af5c7b25..70478ea4 100644 --- a/browse/test/commands.test.ts +++ b/browse/test/commands.test.ts @@ -1495,4 +1495,32 @@ describe('Path traversal prevention', () => { try { fs.unlinkSync(tmpFile); } catch {} } }); + + test('screenshot rejects /tmpevil prefix collision', async () => { + await handleWriteCommand('goto', [baseUrl + '/basic.html'], bm); + try { + await handleMetaCommand('screenshot', ['/tmpevil/steal.png'], bm, () => {}); + expect(true).toBe(false); + } catch (err: any) { + expect(err.message).toContain('Path must be within'); + } + }); + + test('cookie-import rejects path traversal', async () => { + try { + await handleWriteCommand('cookie-import', ['../../etc/shadow'], bm); + expect(true).toBe(false); + } catch (err: any) { + expect(err.message).toContain('Path traversal'); + } + }); + + test('cookie-import rejects absolute path outside safe dirs', async () => { + try { + await handleWriteCommand('cookie-import', ['/etc/passwd'], bm); + expect(true).toBe(false); + } catch (err: any) { + expect(err.message).toContain('Path must be within'); + } + }); });