mirror of
https://github.com/garrytan/gstack.git
synced 2026-05-06 21:46:40 +02:00
security: fix path validation bypass, CORS restriction, cookie-import path check
- startsWith('/tmp') matched '/tmpevil' — now requires trailing slash
- CORS Access-Control-Allow-Origin changed from * to http://127.0.0.1:<port>
- 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 <noreply@anthropic.com>
This commit is contained in:
@@ -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',
|
||||
},
|
||||
|
||||
@@ -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(', ')}`);
|
||||
}
|
||||
|
||||
@@ -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(', ')}`);
|
||||
}
|
||||
|
||||
@@ -236,6 +236,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 = ['/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[];
|
||||
|
||||
@@ -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');
|
||||
}
|
||||
});
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user