security(core): fix path validation bypasses in archive, transcript, and memory
- Replace vulnerable startsWith() checks with path.relative() validation - Add isPathWithinBase() helper for consistent path validation - Add path filter to tar extraction to prevent directory escape - Validate sessionFile parameter in transcript resolution - Add non-root 'sandbox' user to Docker sandbox (UID 1000) Security impact: - Prevents path traversal in archive extraction (zip and tar) - Prevents arbitrary file write via transcript path injection - Prevents workspace escape in memory manager - Reduces privilege in Docker sandbox Fixes #3277 [AI-assisted]
This commit is contained in:
parent
9688454a30
commit
4686606c66
@ -13,4 +13,8 @@ RUN apt-get update \
|
||||
ripgrep \
|
||||
&& rm -rf /var/lib/apt/lists/*
|
||||
|
||||
RUN useradd -m -s /bin/bash -u 1000 sandbox
|
||||
USER sandbox
|
||||
WORKDIR /home/sandbox
|
||||
|
||||
CMD ["sleep", "infinity"]
|
||||
|
||||
@ -56,7 +56,21 @@ function resolveTranscriptPath(params: {
|
||||
sessionFile?: string;
|
||||
}): string | null {
|
||||
const { sessionId, storePath, sessionFile } = params;
|
||||
if (sessionFile) return sessionFile;
|
||||
if (sessionFile) {
|
||||
if (storePath) {
|
||||
const storeDir = path.dirname(storePath);
|
||||
const absSessionFile = path.resolve(storeDir, sessionFile);
|
||||
const rel = path.relative(storeDir, absSessionFile);
|
||||
if (rel.startsWith("..") || path.isAbsolute(rel)) {
|
||||
throw new Error("sessionFile escapes store directory");
|
||||
}
|
||||
return absSessionFile;
|
||||
}
|
||||
if (!path.isAbsolute(sessionFile)) {
|
||||
throw new Error("sessionFile must be absolute when storePath is not set");
|
||||
}
|
||||
return sessionFile;
|
||||
}
|
||||
if (!storePath) return null;
|
||||
return path.join(path.dirname(storePath), `${sessionId}.jsonl`);
|
||||
}
|
||||
|
||||
@ -61,6 +61,11 @@ export async function withTimeout<T>(
|
||||
}
|
||||
}
|
||||
|
||||
function isPathWithinBase(basePath: string, targetPath: string): boolean {
|
||||
const rel = path.relative(basePath, targetPath);
|
||||
return !rel.startsWith("..") && !path.isAbsolute(rel);
|
||||
}
|
||||
|
||||
async function extractZip(params: { archivePath: string; destDir: string }): Promise<void> {
|
||||
const buffer = await fs.readFile(params.archivePath);
|
||||
const zip = await JSZip.loadAsync(buffer);
|
||||
@ -70,7 +75,7 @@ async function extractZip(params: { archivePath: string; destDir: string }): Pro
|
||||
const entryPath = entry.name.replaceAll("\\", "/");
|
||||
if (!entryPath || entryPath.endsWith("/")) {
|
||||
const dirPath = path.resolve(params.destDir, entryPath);
|
||||
if (!dirPath.startsWith(params.destDir)) {
|
||||
if (!isPathWithinBase(params.destDir, dirPath)) {
|
||||
throw new Error(`zip entry escapes destination: ${entry.name}`);
|
||||
}
|
||||
await fs.mkdir(dirPath, { recursive: true });
|
||||
@ -78,7 +83,7 @@ async function extractZip(params: { archivePath: string; destDir: string }): Pro
|
||||
}
|
||||
|
||||
const outPath = path.resolve(params.destDir, entryPath);
|
||||
if (!outPath.startsWith(params.destDir)) {
|
||||
if (!isPathWithinBase(params.destDir, outPath)) {
|
||||
throw new Error(`zip entry escapes destination: ${entry.name}`);
|
||||
}
|
||||
await fs.mkdir(path.dirname(outPath), { recursive: true });
|
||||
@ -101,7 +106,14 @@ export async function extractArchive(params: {
|
||||
const label = kind === "zip" ? "extract zip" : "extract tar";
|
||||
if (kind === "tar") {
|
||||
await withTimeout(
|
||||
tar.x({ file: params.archivePath, cwd: params.destDir }),
|
||||
tar.x({
|
||||
file: params.archivePath,
|
||||
cwd: params.destDir,
|
||||
filter: (entryPath) => {
|
||||
const absPath = path.resolve(params.destDir, entryPath);
|
||||
return isPathWithinBase(params.destDir, absPath);
|
||||
},
|
||||
}),
|
||||
params.timeoutMs,
|
||||
label,
|
||||
);
|
||||
|
||||
@ -401,7 +401,8 @@ export class MemoryIndexManager {
|
||||
throw new Error("path required");
|
||||
}
|
||||
const absPath = path.resolve(this.workspaceDir, relPath);
|
||||
if (!absPath.startsWith(this.workspaceDir)) {
|
||||
const rel = path.relative(this.workspaceDir, absPath);
|
||||
if (rel.startsWith("..") || path.isAbsolute(rel)) {
|
||||
throw new Error("path escapes workspace");
|
||||
}
|
||||
const content = await fs.readFile(absPath, "utf-8");
|
||||
|
||||
Loading…
Reference in New Issue
Block a user