mirror of
https://github.com/garrytan/gstack.git
synced 2026-05-02 03:35:09 +02:00
fix(security): cookie-import path validation bypass + hardcoded /tmp
Two fixes: 1. cookie-import relative path bypass (#707): path.isAbsolute() gated the entire validation, so relative paths like "sensitive-file.json" bypassed the safe-directory check entirely. Now always resolves to absolute path with realpathSync for symlink resolution, matching validateOutputPath(). 2. Hardcoded /tmp in cookie-import-browser (#708): openDbFromCopy used /tmp directly instead of os.tmpdir(), breaking Windows support. Also adds explicit imports for SAFE_DIRECTORIES and isPathWithin in write-commands.ts (previously resolved implicitly through bundler). Closes #852 Co-Authored-By: Toby Morning <urbantech@users.noreply.github.com>
This commit is contained in:
@@ -386,7 +386,8 @@ function openDb(dbPath: string, browserName: string): Database {
|
||||
}
|
||||
|
||||
function openDbFromCopy(dbPath: string, browserName: string): Database {
|
||||
const tmpPath = `/tmp/browse-cookies-${browserName.toLowerCase()}-${crypto.randomUUID()}.db`;
|
||||
// Use os.tmpdir() instead of hardcoded /tmp for cross-platform support (#708)
|
||||
const tmpPath = path.join(os.tmpdir(), `browse-cookies-${browserName.toLowerCase()}-${crypto.randomUUID()}.db`);
|
||||
try {
|
||||
fs.copyFileSync(dbPath, tmpPath);
|
||||
// Also copy WAL and SHM if they exist (for consistent reads)
|
||||
|
||||
@@ -13,7 +13,8 @@ import { validateNavigationUrl } from './url-validation';
|
||||
import { validateOutputPath } from './path-security';
|
||||
import * as fs from 'fs';
|
||||
import * as path from 'path';
|
||||
import { TEMP_DIR } from './platform';
|
||||
import { TEMP_DIR, isPathWithin } from './platform';
|
||||
import { SAFE_DIRECTORIES } from './path-security';
|
||||
import { modifyStyle, undoModification, resetModifications, getModificationHistory } from './cdp-inspector';
|
||||
|
||||
/**
|
||||
@@ -441,16 +442,17 @@ export async function handleWriteCommand(
|
||||
case 'cookie-import': {
|
||||
const filePath = args[0];
|
||||
if (!filePath) throw new Error('Usage: browse cookie-import <json-file>');
|
||||
// Path validation — prevent reading arbitrary files
|
||||
if (path.isAbsolute(filePath)) {
|
||||
const safeDirs = [TEMP_DIR, process.cwd()];
|
||||
const resolved = path.resolve(filePath);
|
||||
if (!safeDirs.some(dir => isPathWithin(resolved, dir))) {
|
||||
throw new Error(`Path must be within: ${safeDirs.join(', ')}`);
|
||||
}
|
||||
// Path validation — resolve to absolute and check against safe dirs.
|
||||
// Fixes #707: relative paths previously bypassed the safe directory check.
|
||||
// Mirrors validateOutputPath() — resolves symlinks (e.g., macOS /tmp → /private/tmp).
|
||||
const resolved = path.resolve(filePath);
|
||||
let resolvedReal = resolved;
|
||||
try { resolvedReal = fs.realpathSync(resolved); } catch {
|
||||
// File may not exist yet — resolve parent dir instead
|
||||
try { resolvedReal = path.join(fs.realpathSync(path.dirname(resolved)), path.basename(resolved)); } catch {}
|
||||
}
|
||||
if (path.normalize(filePath).includes('..')) {
|
||||
throw new Error('Path traversal sequences (..) are not allowed');
|
||||
if (!SAFE_DIRECTORIES.some(dir => isPathWithin(resolvedReal, dir))) {
|
||||
throw new Error(`Path must be within: ${SAFE_DIRECTORIES.join(', ')}`);
|
||||
}
|
||||
if (!fs.existsSync(filePath)) throw new Error(`File not found: ${filePath}`);
|
||||
const raw = fs.readFileSync(filePath, 'utf-8');
|
||||
|
||||
@@ -1811,7 +1811,8 @@ describe('Path traversal prevention', () => {
|
||||
await handleWriteCommand('cookie-import', ['../../etc/shadow'], bm);
|
||||
expect(true).toBe(false);
|
||||
} catch (err: any) {
|
||||
expect(err.message).toContain('Path traversal');
|
||||
// Traversal blocked by safe-directory check (#707) or explicit .. check
|
||||
expect(err.message).toMatch(/Path must be within|Path traversal/);
|
||||
}
|
||||
});
|
||||
|
||||
|
||||
Reference in New Issue
Block a user