diff --git a/src/infra/archive.test.ts b/src/infra/archive.test.ts index ccf998894..239e429d5 100644 --- a/src/infra/archive.test.ts +++ b/src/infra/archive.test.ts @@ -49,6 +49,23 @@ describe("archive utils", () => { expect(content).toBe("hi"); }); + it("blocks zip entries that escape the destination directory", async () => { + const workDir = await makeTempDir(); + const archivePath = path.join(workDir, "evil.zip"); + const extractDir = path.join(workDir, "extract"); + const siblingDir = path.join(workDir, "extractX"); + + const zip = new JSZip(); + zip.file("../extractX/pwned.txt", "pwned"); + await fs.writeFile(archivePath, await zip.generateAsync({ type: "nodebuffer" })); + + await fs.mkdir(extractDir, { recursive: true }); + await expect( + extractArchive({ archivePath, destDir: extractDir, timeoutMs: 5_000 }), + ).rejects.toThrow(/escapes destination/i); + await expect(fs.stat(path.join(siblingDir, "pwned.txt"))).rejects.toThrow(); + }); + it("extracts tar archives", async () => { const workDir = await makeTempDir(); const archivePath = path.join(workDir, "bundle.tar"); diff --git a/src/infra/archive.ts b/src/infra/archive.ts index 35ad4fa04..5bf638ad5 100644 --- a/src/infra/archive.ts +++ b/src/infra/archive.ts @@ -61,7 +61,28 @@ export async function withTimeout( } } +function ensureTrailingSep(filePath: string): string { + return filePath.endsWith(path.sep) ? filePath : `${filePath}${path.sep}`; +} + +async function normalizeDestRoot( + destDir: string, +): Promise<{ destRoot: string; destRootLower?: string }> { + await fs.mkdir(destDir, { recursive: true }); + const destReal = await fs.realpath(destDir); + const destRoot = ensureTrailingSep(destReal); + return process.platform === "win32" + ? { destRoot, destRootLower: destRoot.toLowerCase() } + : { destRoot }; +} + async function extractZip(params: { archivePath: string; destDir: string }): Promise { + const { destRoot, destRootLower } = await normalizeDestRoot(params.destDir); + const startsWithDest = (targetPath: string): boolean => + destRootLower + ? targetPath.toLowerCase().startsWith(destRootLower) + : targetPath.startsWith(destRoot); + const buffer = await fs.readFile(params.archivePath); const zip = await JSZip.loadAsync(buffer); const entries = Object.values(zip.files); @@ -69,16 +90,16 @@ async function extractZip(params: { archivePath: string; destDir: string }): Pro for (const entry of entries) { const entryPath = entry.name.replaceAll("\\", "/"); if (!entryPath || entryPath.endsWith("/")) { - const dirPath = path.resolve(params.destDir, entryPath); - if (!dirPath.startsWith(params.destDir)) { + const dirPath = path.resolve(destRoot, entryPath); + if (!startsWithDest(dirPath)) { throw new Error(`zip entry escapes destination: ${entry.name}`); } await fs.mkdir(dirPath, { recursive: true }); continue; } - const outPath = path.resolve(params.destDir, entryPath); - if (!outPath.startsWith(params.destDir)) { + const outPath = path.resolve(destRoot, entryPath); + if (!startsWithDest(outPath)) { throw new Error(`zip entry escapes destination: ${entry.name}`); } await fs.mkdir(path.dirname(outPath), { recursive: true });