mirror of https://github.com/garrytan/gstack.git
fix: path traversal security vulnerability in screenshot/pdf/eval commands
- Add validateOutputPath() to ensure screenshot, pdf, and responsive commands only write to /tmp or current working directory - Add validateReadPath() to prevent eval command from reading files outside allowed directories or using path traversal (..) - Validates both absolute paths and relative paths with .. sequences Security impact: Prevents arbitrary file write (screenshot/pdf) and arbitrary file read (eval) attacks via path traversal. Fixes: #13
This commit is contained in:
parent
1b317aae9a
commit
39702a90ae
|
|
@ -6,6 +6,43 @@ import type { BrowserManager } from './browser-manager';
|
||||||
import { handleSnapshot } from './snapshot';
|
import { handleSnapshot } from './snapshot';
|
||||||
import * as Diff from 'diff';
|
import * as Diff from 'diff';
|
||||||
import * as fs from 'fs';
|
import * as fs from 'fs';
|
||||||
|
import * as path from 'path';
|
||||||
|
|
||||||
|
// Security: Path validation to prevent path traversal attacks
|
||||||
|
const SAFE_DIRECTORIES = ['/tmp', process.cwd()];
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Validates that a file path is within allowed directories
|
||||||
|
* @param filePath The path to validate
|
||||||
|
* @throws Error if path is outside safe directories
|
||||||
|
*/
|
||||||
|
function validateOutputPath(filePath: string): void {
|
||||||
|
const resolved = path.resolve(filePath);
|
||||||
|
const isSafe = SAFE_DIRECTORIES.some(dir => resolved.startsWith(dir));
|
||||||
|
if (!isSafe) {
|
||||||
|
throw new Error(`Path must be within: ${SAFE_DIRECTORIES.join(', ')}`);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Validates that a file path for reading doesn't contain path traversal
|
||||||
|
* @param filePath The path to validate
|
||||||
|
* @throws Error if path contains traversal sequences
|
||||||
|
*/
|
||||||
|
function validateReadPath(filePath: string): void {
|
||||||
|
// Reject absolute paths outside allowed directories
|
||||||
|
if (path.isAbsolute(filePath)) {
|
||||||
|
const isSafe = SAFE_DIRECTORIES.some(dir => path.resolve(filePath).startsWith(dir));
|
||||||
|
if (!isSafe) {
|
||||||
|
throw new Error(`Absolute path must be within: ${SAFE_DIRECTORIES.join(', ')}`);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
// Check for path traversal sequences
|
||||||
|
const normalized = path.normalize(filePath);
|
||||||
|
if (normalized.includes('..')) {
|
||||||
|
throw new Error('Path traversal sequences (..) are not allowed');
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
export async function handleMetaCommand(
|
export async function handleMetaCommand(
|
||||||
command: string,
|
command: string,
|
||||||
|
|
@ -73,6 +110,7 @@ export async function handleMetaCommand(
|
||||||
case 'screenshot': {
|
case 'screenshot': {
|
||||||
const page = bm.getPage();
|
const page = bm.getPage();
|
||||||
const screenshotPath = args[0] || '/tmp/browse-screenshot.png';
|
const screenshotPath = args[0] || '/tmp/browse-screenshot.png';
|
||||||
|
validateOutputPath(screenshotPath);
|
||||||
await page.screenshot({ path: screenshotPath, fullPage: true });
|
await page.screenshot({ path: screenshotPath, fullPage: true });
|
||||||
return `Screenshot saved: ${screenshotPath}`;
|
return `Screenshot saved: ${screenshotPath}`;
|
||||||
}
|
}
|
||||||
|
|
@ -80,6 +118,7 @@ export async function handleMetaCommand(
|
||||||
case 'pdf': {
|
case 'pdf': {
|
||||||
const page = bm.getPage();
|
const page = bm.getPage();
|
||||||
const pdfPath = args[0] || '/tmp/browse-page.pdf';
|
const pdfPath = args[0] || '/tmp/browse-page.pdf';
|
||||||
|
validateOutputPath(pdfPath);
|
||||||
await page.pdf({ path: pdfPath, format: 'A4' });
|
await page.pdf({ path: pdfPath, format: 'A4' });
|
||||||
return `PDF saved: ${pdfPath}`;
|
return `PDF saved: ${pdfPath}`;
|
||||||
}
|
}
|
||||||
|
|
@ -87,6 +126,7 @@ export async function handleMetaCommand(
|
||||||
case 'responsive': {
|
case 'responsive': {
|
||||||
const page = bm.getPage();
|
const page = bm.getPage();
|
||||||
const prefix = args[0] || '/tmp/browse-responsive';
|
const prefix = args[0] || '/tmp/browse-responsive';
|
||||||
|
validateOutputPath(prefix);
|
||||||
const viewports = [
|
const viewports = [
|
||||||
{ name: 'mobile', width: 375, height: 812 },
|
{ name: 'mobile', width: 375, height: 812 },
|
||||||
{ name: 'tablet', width: 768, height: 1024 },
|
{ name: 'tablet', width: 768, height: 1024 },
|
||||||
|
|
|
||||||
|
|
@ -8,6 +8,30 @@
|
||||||
import type { BrowserManager } from './browser-manager';
|
import type { BrowserManager } from './browser-manager';
|
||||||
import { consoleBuffer, networkBuffer } from './buffers';
|
import { consoleBuffer, networkBuffer } from './buffers';
|
||||||
import * as fs from 'fs';
|
import * as fs from 'fs';
|
||||||
|
import * as path from 'path';
|
||||||
|
|
||||||
|
// Security: Path validation to prevent path traversal attacks
|
||||||
|
const SAFE_DIRECTORIES = ['/tmp', process.cwd()];
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Validates that a file path for reading doesn't contain path traversal
|
||||||
|
* @param filePath The path to validate
|
||||||
|
* @throws Error if path contains traversal sequences or is outside safe directories
|
||||||
|
*/
|
||||||
|
function validateReadPath(filePath: string): void {
|
||||||
|
// Reject absolute paths outside allowed directories
|
||||||
|
if (path.isAbsolute(filePath)) {
|
||||||
|
const isSafe = SAFE_DIRECTORIES.some(dir => path.resolve(filePath).startsWith(dir));
|
||||||
|
if (!isSafe) {
|
||||||
|
throw new Error(`Absolute path must be within: ${SAFE_DIRECTORIES.join(', ')}`);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
// Check for path traversal sequences
|
||||||
|
const normalized = path.normalize(filePath);
|
||||||
|
if (normalized.includes('..')) {
|
||||||
|
throw new Error('Path traversal sequences (..) are not allowed');
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
export async function handleReadCommand(
|
export async function handleReadCommand(
|
||||||
command: string,
|
command: string,
|
||||||
|
|
@ -98,6 +122,7 @@ export async function handleReadCommand(
|
||||||
case 'eval': {
|
case 'eval': {
|
||||||
const filePath = args[0];
|
const filePath = args[0];
|
||||||
if (!filePath) throw new Error('Usage: browse eval <js-file>');
|
if (!filePath) throw new Error('Usage: browse eval <js-file>');
|
||||||
|
validateReadPath(filePath);
|
||||||
if (!fs.existsSync(filePath)) throw new Error(`File not found: ${filePath}`);
|
if (!fs.existsSync(filePath)) throw new Error(`File not found: ${filePath}`);
|
||||||
const code = fs.readFileSync(filePath, 'utf-8');
|
const code = fs.readFileSync(filePath, 'utf-8');
|
||||||
const result = await page.evaluate(code);
|
const result = await page.evaluate(code);
|
||||||
|
|
|
||||||
Loading…
Reference in New Issue