Merge pull request #3024 from code-yeongyu/fix/security-tar-containment

fix(security): add archive extraction containment validation

src/shared/archive-entry-validator.test.ts

@@ -0,0 +1,184 @@
+/// <reference types="bun-types" />
+
+import { afterEach, describe, expect, it } from "bun:test"
+import { lstatSync, mkdirSync, mkdtempSync, readFileSync, rmSync, symlinkSync, writeFileSync } from "node:fs"
+import { tmpdir } from "node:os"
+import { join } from "node:path"
+import { spawnSync } from "bun"
+
+import { extractTarGz } from "./binary-downloader"
+import { validateArchiveEntries } from "./archive-entry-validator"
+import { extractZip } from "./zip-extractor"
+
+const testDirs: string[] = []
+
+function createTestDir(): string {
+	const dir = mkdtempSync(join(tmpdir(), "archive-entry-validator-"))
+	testDirs.push(dir)
+	return dir
+}
+
+function runCommand(command: string, cwd?: string): void {
+	const result = spawnSync(["bash", "-lc", command], { cwd, stderr: "pipe", stdout: "pipe" })
+	if (result.exitCode !== 0) {
+		throw new Error(result.stderr.toString())
+	}
+}
+
+function writePythonScript(dir: string, filename: string, content: string): string {
+	const scriptPath = join(dir, filename)
+	writeFileSync(scriptPath, content)
+	return scriptPath
+}
+
+afterEach(() => {
+	for (const dir of testDirs.splice(0)) {
+		rmSync(dir, { recursive: true, force: true })
+	}
+})
+
+describe("validateArchiveEntries", () => {
+	it("rejects absolute paths and traversal entries", () => {
+		//#given
+		const destDir = "/tmp/archive-root"
+
+		//#when
+		const rejectAbsolutePath = () =>
+			validateArchiveEntries([{ path: "/etc/passwd", type: "file" }], destDir)
+		const rejectTraversalPath = () =>
+			validateArchiveEntries([{ path: "nested/../../evil.txt", type: "file" }], destDir)
+
+		//#then
+		expect(rejectAbsolutePath).toThrow(/absolute path/i)
+		expect(rejectTraversalPath).toThrow(/path traversal/i)
+	})
+
+	it("rejects symlink targets that escape the extraction directory", () => {
+		//#given
+		const destDir = "/tmp/archive-root"
+
+		//#when
+		const rejectEscapeSymlink = () =>
+			validateArchiveEntries(
+				[{ path: "bin/tool", type: "symlink", linkPath: "../../outside/tool" }],
+				destDir
+			)
+
+		//#then
+		expect(rejectEscapeSymlink).toThrow(/symlink target/i)
+	})
+
+	it("accepts contained files, directories, and symlinks", () => {
+		//#given
+		const destDir = "/tmp/archive-root"
+		const entries = [
+			{ path: "bin/", type: "directory" as const },
+			{ path: "bin/tool", type: "file" as const },
+			{ path: "bin/tool-link", type: "symlink" as const, linkPath: "tool" },
+		]
+
+		//#when
+		const validateContainedEntries = () => validateArchiveEntries(entries, destDir)
+
+		//#then
+		expect(validateContainedEntries).not.toThrow()
+	})
+})
+
+describe("archive extraction preflight", () => {
+	it("rejects tar archives with traversal entries before extraction", async () => {
+		//#given
+		const rootDir = createTestDir()
+		const archivePath = join(rootDir, "malicious.tar.gz")
+		const destDir = join(rootDir, "dest")
+		mkdirSync(destDir, { recursive: true })
+		const scriptPath = writePythonScript(
+			rootDir,
+			"make-malicious-tar.py",
+			[
+				"import io",
+				"import sys",
+				"import tarfile",
+				"with tarfile.open(sys.argv[1], 'w:gz') as archive:",
+				"    data = b'owned'",
+				"    info = tarfile.TarInfo('../escape.txt')",
+				"    info.size = len(data)",
+				"    archive.addfile(info, io.BytesIO(data))",
+			].join("\n")
+		)
+		runCommand(`python3 "${scriptPath}" "${archivePath}"`)
+
+		//#when
+		let errorMessage = ""
+		try {
+			await extractTarGz(archivePath, destDir)
+		} catch (error) {
+			errorMessage = error instanceof Error ? error.message : String(error)
+		}
+
+		//#then
+		expect(errorMessage).toMatch(/path traversal/i)
+	})
+
+	it("rejects zip archives with symlink escapes before extraction", async () => {
+		//#given
+		const rootDir = createTestDir()
+		const archivePath = join(rootDir, "malicious.zip")
+		const destDir = join(rootDir, "dest")
+		mkdirSync(destDir, { recursive: true })
+		const scriptPath = writePythonScript(
+			rootDir,
+			"make-malicious-zip.py",
+			[
+				"import stat",
+				"import sys",
+				"import zipfile",
+				"archive = zipfile.ZipFile(sys.argv[1], 'w')",
+				"entry = zipfile.ZipInfo('bin/tool-link')",
+				"entry.create_system = 3",
+				"entry.external_attr = (stat.S_IFLNK | 0o777) << 16",
+				"archive.writestr(entry, '../../escape.txt')",
+				"archive.close()",
+			].join("\n")
+		)
+		runCommand(`python3 "${scriptPath}" "${archivePath}"`)
+
+		//#when
+		let errorMessage = ""
+		try {
+			await extractZip(archivePath, destDir)
+		} catch (error) {
+			errorMessage = error instanceof Error ? error.message : String(error)
+		}
+
+		//#then
+		expect(errorMessage).toMatch(/symlink target/i)
+	})
+
+	it("extracts safe tar and zip archives into the destination directory", async () => {
+		//#given
+		const rootDir = createTestDir()
+		const sourceDir = join(rootDir, "source")
+		const tarArchivePath = join(rootDir, "safe.tar.gz")
+		const zipArchivePath = join(rootDir, "safe.zip")
+		const tarDestDir = join(rootDir, "tar-dest")
+		const zipDestDir = join(rootDir, "zip-dest")
+		mkdirSync(join(sourceDir, "bin"), { recursive: true })
+		mkdirSync(tarDestDir, { recursive: true })
+		mkdirSync(zipDestDir, { recursive: true })
+		writeFileSync(join(sourceDir, "bin", "tool.txt"), "safe")
+		symlinkSync("tool.txt", join(sourceDir, "bin", "tool-link"))
+		runCommand(`tar -czf "${tarArchivePath}" -C "${sourceDir}" .`)
+		runCommand(`zip -qry "${zipArchivePath}" .`, sourceDir)
+
+		//#when
+		await extractTarGz(tarArchivePath, tarDestDir)
+		await extractZip(zipArchivePath, zipDestDir)
+
+		//#then
+		expect(readFileSync(join(tarDestDir, "bin", "tool.txt"), "utf8")).toBe("safe")
+		expect(readFileSync(join(zipDestDir, "bin", "tool.txt"), "utf8")).toBe("safe")
+		expect(lstatSync(join(tarDestDir, "bin", "tool-link")).isSymbolicLink()).toBe(true)
+		expect(lstatSync(join(zipDestDir, "bin", "tool-link")).isSymbolicLink()).toBe(true)
+	})
+})

src/shared/archive-entry-validator.ts

@@ -0,0 +1,74 @@
+import { dirname, isAbsolute, relative, resolve, sep } from "node:path"
+
+export type ArchiveEntry = {
+	path: string
+	type: "file" | "directory" | "symlink"
+	linkPath?: string
+}
+
+function normalizeArchivePath(filePath: string): string {
+	return filePath.replaceAll("\\", "/")
+}
+
+function containsTraversalSegment(filePath: string): boolean {
+	return normalizeArchivePath(filePath)
+		.split("/")
+		.some(segment => segment === "..")
+}
+
+function isArchiveAbsolutePath(filePath: string): boolean {
+	const normalizedPath = normalizeArchivePath(filePath)
+	return isAbsolute(normalizedPath) || /^[A-Za-z]:\//.test(normalizedPath) || normalizedPath.startsWith("//")
+}
+
+function escapesDirectory(rootDir: string, candidatePath: string): boolean {
+	const relativePath = relative(rootDir, candidatePath)
+	return relativePath === ".." || relativePath.startsWith(`..${sep}`) || isAbsolute(relativePath)
+}
+
+function resolveContainedPath(rootDir: string, filePath: string, errorLabel: string): string {
+	const normalizedPath = normalizeArchivePath(filePath)
+	if (isArchiveAbsolutePath(normalizedPath)) {
+		throw new Error(`Unsafe archive entry: ${errorLabel} uses an absolute path (${filePath})`)
+	}
+
+	if (containsTraversalSegment(normalizedPath)) {
+		throw new Error(`Unsafe archive entry: ${errorLabel} contains path traversal (${filePath})`)
+	}
+
+	const resolvedPath = resolve(rootDir, normalizedPath)
+	if (escapesDirectory(rootDir, resolvedPath)) {
+		throw new Error(`Unsafe archive entry: ${errorLabel} contains path traversal (${filePath})`)
+	}
+
+	return resolvedPath
+}
+
+export function validateArchiveEntries(entries: ArchiveEntry[], destDir: string): void {
+	const resolvedDestDir = resolve(destDir)
+
+	for (const entry of entries) {
+		const resolvedEntryPath = resolveContainedPath(resolvedDestDir, entry.path, "path")
+		if (entry.type !== "symlink") {
+			continue
+		}
+
+		if (!entry.linkPath) {
+			throw new Error(`Unsafe archive entry: symlink target missing for ${entry.path}`)
+		}
+
+		const normalizedLinkPath = normalizeArchivePath(entry.linkPath)
+		if (isArchiveAbsolutePath(normalizedLinkPath)) {
+			throw new Error(`Unsafe archive entry: symlink target uses an absolute path (${entry.linkPath})`)
+		}
+
+		if (containsTraversalSegment(normalizedLinkPath)) {
+			throw new Error(`Unsafe archive entry: symlink target contains path traversal (${entry.linkPath})`)
+		}
+
+		const resolvedLinkPath = resolve(dirname(resolvedEntryPath), normalizedLinkPath)
+		if (escapesDirectory(resolvedDestDir, resolvedLinkPath)) {
+			throw new Error(`Unsafe archive entry: symlink target escapes extraction directory (${entry.linkPath})`)
+		}
+	}
+}

src/shared/binary-downloader.ts

@@ -1,6 +1,7 @@
 import { chmodSync, existsSync, mkdirSync, unlinkSync } from "node:fs";
 import * as path from "node:path";
 import { spawn } from "bun";
+import { validateArchiveEntries, type ArchiveEntry } from "./archive-entry-validator";
 import { extractZip } from "./zip-extractor";
 
 export function getCachedBinaryPath(cacheDir: string, binaryName: string): string | null {
@@ -29,6 +30,9 @@ export async function extractTarGz(
   destDir: string,
   options?: { args?: string[]; cwd?: string }
 ): Promise<void> {
+  const entries = await listTarEntries(archivePath, options?.cwd)
+  validateArchiveEntries(entries, destDir)
+
   const args = options?.args ?? ["tar", "-xzf", archivePath, "-C", destDir];
   const proc = spawn(args, {
     cwd: options?.cwd,
@@ -58,3 +62,54 @@ export function ensureExecutable(binaryPath: string): void {
     chmodSync(binaryPath, 0o755);
   }
 }
+
+function parseTarEntry(line: string): ArchiveEntry | null {
+  const match = line.match(/^([^\s])\S*\s+\d+\s+\S+\s+\S+\s+\d+\s+\w+\s+\d+\s+(?:\d{2}:\d{2}|\d{4})\s+(.*)$/)
+  if (!match) {
+    return null
+  }
+
+  const [, rawType, rawEntryPath] = match
+  if (rawType === "l") {
+    const arrowIndex = rawEntryPath.lastIndexOf(" -> ")
+    if (arrowIndex === -1) {
+      return { path: rawEntryPath, type: "symlink" }
+    }
+
+    return {
+      path: rawEntryPath.slice(0, arrowIndex),
+      type: "symlink",
+      linkPath: rawEntryPath.slice(arrowIndex + 4),
+    }
+  }
+
+  return {
+    path: rawEntryPath,
+    type: rawType === "d" ? "directory" : "file",
+  }
+}
+
+async function listTarEntries(archivePath: string, cwd?: string): Promise<ArchiveEntry[]> {
+  const proc = spawn(["tar", "-tvzf", archivePath], {
+    cwd,
+    stdout: "pipe",
+    stderr: "pipe",
+  })
+
+  const [exitCode, stdout, stderr] = await Promise.all([
+    proc.exited,
+    new Response(proc.stdout).text(),
+    new Response(proc.stderr).text(),
+  ])
+
+  if (exitCode !== 0) {
+    throw new Error(`tar entry listing failed (exit ${exitCode}): ${stderr}`)
+  }
+
+  return stdout
+    .split(/\r?\n/)
+    .map(line => line.trim())
+    .filter(Boolean)
+    .map(line => parseTarEntry(line))
+    .filter((entry): entry is ArchiveEntry => entry !== null)
+}

src/shared/zip-entry-listing.ts

@@ -0,0 +1,122 @@
+import { spawn } from "bun"
+
+import type { ArchiveEntry } from "./archive-entry-validator"
+
+function parseTarListedZipEntry(line: string): ArchiveEntry | null {
+	const match = line.match(/^([^\s])\S*\s+\d+\s+\S+\s+\S+\s+\d+\s+\w+\s+\d+\s+(?:\d{2}:\d{2}|\d{4})\s+(.*)$/)
+	if (!match) {
+		return null
+	}
+
+	const [, rawType, rawEntryPath] = match
+	if (rawType === "l") {
+		const arrowIndex = rawEntryPath.lastIndexOf(" -> ")
+		return {
+			path: arrowIndex === -1 ? rawEntryPath : rawEntryPath.slice(0, arrowIndex),
+			type: "symlink",
+			linkPath: arrowIndex === -1 ? undefined : rawEntryPath.slice(arrowIndex + 4),
+		}
+	}
+
+	return {
+		path: rawEntryPath,
+		type: rawType === "d" ? "directory" : "file",
+	}
+}
+
+export async function listZipEntriesWithTar(archivePath: string): Promise<ArchiveEntry[]> {
+	const proc = spawn(["tar", "-tvf", archivePath], {
+		stdout: "pipe",
+		stderr: "pipe",
+	})
+
+	const [exitCode, stdout, stderr] = await Promise.all([
+		proc.exited,
+		new Response(proc.stdout).text(),
+		new Response(proc.stderr).text(),
+	])
+
+	if (exitCode !== 0) {
+		throw new Error(`zip entry listing failed (exit ${exitCode}): ${stderr}`)
+	}
+
+	return stdout
+		.split(/\r?\n/)
+		.map(line => line.trim())
+		.filter(Boolean)
+		.map(line => parseTarListedZipEntry(line))
+		.filter((entry): entry is ArchiveEntry => entry !== null)
+}
+
+export async function listZipEntriesWithPowerShell(
+	archivePath: string,
+	escapePowerShellPath: (path: string) => string,
+	extractor: "pwsh" | "powershell"
+): Promise<ArchiveEntry[]> {
+	const proc = spawn(
+		[
+			extractor,
+			"-Command",
+			[
+				"Add-Type -AssemblyName System.IO.Compression.FileSystem",
+				`$archive = [System.IO.Compression.ZipFile]::OpenRead('${escapePowerShellPath(archivePath)}')`,
+				"try {",
+				"  foreach ($entry in $archive.Entries) {",
+				"    $mode = ($entry.ExternalAttributes -shr 16) -band 0xFFFF",
+				"    $type = if (($mode -band 0xF000) -eq 0xA000) { 'symlink' } elseif ($entry.FullName.EndsWith('/')) { 'directory' } else { 'file' }",
+				"    $target = ''",
+				"    if ($type -eq 'symlink') {",
+				"      $stream = $entry.Open()",
+				"      try {",
+				"        $reader = New-Object System.IO.StreamReader($stream)",
+				"        try { $target = $reader.ReadToEnd() } finally { $reader.Dispose() }",
+				"      } finally { $stream.Dispose() }",
+				"    }",
+				"    Write-Output ($type + \"`t\" + $entry.FullName + \"`t\" + $target)",
+				"  }",
+				"} finally {",
+				"  $archive.Dispose()",
+				"}",
+			].join("; "),
+		],
+		{
+			stdout: "pipe",
+			stderr: "pipe",
+		}
+	)
+
+	const [exitCode, stdout, stderr] = await Promise.all([
+		proc.exited,
+		new Response(proc.stdout).text(),
+		new Response(proc.stderr).text(),
+	])
+
+	if (exitCode !== 0) {
+		throw new Error(`zip entry listing failed (exit ${exitCode}): ${stderr}`)
+	}
+
+	return stdout
+		.split(/\r?\n/)
+		.map(line => line.trim())
+		.filter(Boolean)
+		.map((line): ArchiveEntry | null => {
+			const [type, entryPath, linkPath = ""] = line.split("\t")
+			if (type !== "file" && type !== "directory" && type !== "symlink") {
+				return null
+			}
+
+			if (type === "symlink") {
+				return {
+					path: entryPath,
+					type,
+					linkPath,
+				}
+			}
+
+			return {
+				path: entryPath,
+				type,
+			}
+		})
+		.filter((entry): entry is ArchiveEntry => entry !== null)
+}

src/shared/zip-extractor.ts

@@ -1,6 +1,9 @@
 import { spawn, spawnSync } from "bun"
 import { release } from "os"
 
+import { validateArchiveEntries } from "./archive-entry-validator"
+import { listZipEntriesWithPowerShell, listZipEntriesWithTar } from "./zip-entry-listing"
+
 const WINDOWS_BUILD_WITH_TAR = 17134
 
 function getWindowsBuildNumber(): number | null {
@@ -41,6 +44,9 @@ function getWindowsZipExtractor(): WindowsZipExtractor {
 }
 
 export async function extractZip(archivePath: string, destDir: string): Promise<void> {
+  const entries = await listZipEntries(archivePath)
+  validateArchiveEntries(entries, destDir)
+
   let proc
   
   if (process.platform === "win32") {
@@ -81,3 +87,16 @@ export async function extractZip(archivePath: string, destDir: string): Promise<
     throw new Error(`zip extraction failed (exit ${exitCode}): ${stderr}`)
   }
 }
+
+async function listZipEntries(archivePath: string) {
+  if (process.platform === "win32") {
+    const extractor = getWindowsZipExtractor()
+    if (extractor === "tar") {
+      return listZipEntriesWithTar(archivePath)
+    }
+
+    return listZipEntriesWithPowerShell(archivePath, escapePowerShellPath, extractor)
+  }
+
+  return listZipEntriesWithTar(archivePath)
+}