Merge pull request #3088 from code-yeongyu/fix/prepublish-security-hardening

fix(shared): harden archive preflight for zip and tar links

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

@@ -68,6 +68,21 @@ describe("validateArchiveEntries", () => {
 		expect(rejectEscapeSymlink).toThrow(/symlink target/i)
 	})
 
+	it("rejects hard-link targets that escape the extraction directory", () => {
+		//#given
+		const destDir = "/tmp/archive-root"
+
+		//#when
+		const rejectEscapeHardLink = () =>
+			validateArchiveEntries(
+				[{ path: "bin/tool", type: "hardlink", linkPath: "../../etc/passwd" }],
+				destDir
+			)
+
+		//#then
+		expect(rejectEscapeHardLink).toThrow(/hard link target/i)
+	})
+
 	it("accepts contained files, directories, and symlinks", () => {
 		//#given
 		const destDir = "/tmp/archive-root"
@@ -120,6 +135,39 @@ describe("archive extraction preflight", () => {
 		expect(errorMessage).toMatch(/path traversal/i)
 	})
 
+	it("rejects tar archives with hard-link traversal before extraction", async () => {
+		//#given
+		const rootDir = createTestDir()
+		const archivePath = join(rootDir, "malicious-hard-link.tar.gz")
+		const destDir = join(rootDir, "dest")
+		mkdirSync(destDir, { recursive: true })
+		const scriptPath = writePythonScript(
+			rootDir,
+			"make-malicious-hard-link-tar.py",
+			[
+				"import sys",
+				"import tarfile",
+				"with tarfile.open(sys.argv[1], 'w:gz') as archive:",
+				"    info = tarfile.TarInfo('bin/tool')",
+				"    info.type = tarfile.LNKTYPE",
+				"    info.linkname = '../../etc/passwd'",
+				"    archive.addfile(info)",
+			].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(/hard link target|path traversal/i)
+	})
+
 	it("rejects zip archives with symlink escapes before extraction", async () => {
 		//#given
 		const rootDir = createTestDir()

src/shared/archive-entry-validator.ts

@@ -2,7 +2,7 @@ import { dirname, isAbsolute, relative, resolve, sep } from "node:path"
 
 export type ArchiveEntry = {
 	path: string
-	type: "file" | "directory" | "symlink"
+	type: "file" | "directory" | "symlink" | "hardlink"
 	linkPath?: string
 }
 
@@ -49,26 +49,35 @@ export function validateArchiveEntries(entries: ArchiveEntry[], destDir: string)
 
 	for (const entry of entries) {
 		const resolvedEntryPath = resolveContainedPath(resolvedDestDir, entry.path, "path")
-		if (entry.type !== "symlink") {
+		if (entry.type !== "symlink" && entry.type !== "hardlink") {
 			continue
 		}
 
 		if (!entry.linkPath) {
-			throw new Error(`Unsafe archive entry: symlink target missing for ${entry.path}`)
+			throw new Error(
+				`Unsafe archive entry: ${entry.type === "symlink" ? "symlink" : "hard link"} target missing for ${entry.path}`
+			)
 		}
 
 		const normalizedLinkPath = normalizeArchivePath(entry.linkPath)
+		const linkTypeLabel = entry.type === "symlink" ? "symlink target" : "hard link target"
 		if (isArchiveAbsolutePath(normalizedLinkPath)) {
-			throw new Error(`Unsafe archive entry: symlink target uses an absolute path (${entry.linkPath})`)
+			throw new Error(
+				`Unsafe archive entry: ${linkTypeLabel} uses an absolute path (${entry.linkPath})`
+			)
 		}
 
 		if (containsTraversalSegment(normalizedLinkPath)) {
-			throw new Error(`Unsafe archive entry: symlink target contains path traversal (${entry.linkPath})`)
+			throw new Error(
+				`Unsafe archive entry: ${linkTypeLabel} 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})`)
+			throw new Error(
+				`Unsafe archive entry: ${linkTypeLabel} escapes extraction directory (${entry.linkPath})`
+			)
 		}
 	}
 }

src/shared/binary-downloader.ts

@@ -78,15 +78,15 @@ function parseTarEntry(line: string): ArchiveEntry | null {
   }
 
   const [, rawType, rawEntryPath] = match
-  if (rawType === "l") {
+  if (rawType === "l" || rawType === "h") {
     const arrowIndex = rawEntryPath.lastIndexOf(" -> ")
     if (arrowIndex === -1) {
-      return { path: rawEntryPath, type: "symlink" }
+      return { path: rawEntryPath, type: rawType === "l" ? "symlink" : "hardlink" }
     }
 
     return {
       path: rawEntryPath.slice(0, arrowIndex),
-      type: "symlink",
+      type: rawType === "l" ? "symlink" : "hardlink",
       linkPath: rawEntryPath.slice(arrowIndex + 4),
     }
   }

src/shared/zip-entry-listing/powershell-zip-entry-listing.test.ts

@@ -0,0 +1,29 @@
+import { describe, expect, it } from "bun:test"
+
+import { validateArchiveEntries } from "../archive-entry-validator"
+import { parsePowerShellZipEntryLine } from "./powershell-zip-entry-listing"
+
+describe("parsePowerShellZipEntryLine", () => {
+	describe("#given a json entry line with tab characters in the file name", () => {
+		it("#when parsing and validating the entry #then preserves the full path for traversal checks", () => {
+			// given
+			const entryLine = JSON.stringify({
+				type: "file",
+				name: `safe.txt\t../../escape.txt`,
+				target: "",
+			})
+
+			// when
+			const parsedEntry = parsePowerShellZipEntryLine(entryLine)
+			const validateParsedEntry = () =>
+				validateArchiveEntries(parsedEntry ? [parsedEntry] : [], "/tmp/archive-root")
+
+			// then
+			expect(parsedEntry).toEqual({
+				path: `safe.txt\t../../escape.txt`,
+				type: "file",
+			})
+			expect(validateParsedEntry).toThrow(/path traversal/i)
+		})
+	})
+})

src/shared/zip-entry-listing/powershell-zip-entry-listing.ts

@@ -4,35 +4,74 @@ import type { ArchiveEntry } from "../archive-entry-validator"
 
 export type PowerShellZipExtractor = "pwsh" | "powershell"
 
+type PowerShellZipEntryRecord = {
+	type: "file" | "directory" | "symlink"
+	name: string
+	target: string
+}
+
+function isPowerShellZipEntryRecord(value: unknown): value is PowerShellZipEntryRecord {
+	if (!value || typeof value !== "object") {
+		return false
+	}
+
+	const candidate = value as Record<string, unknown>
+	return (
+		(candidate.type === "file" || candidate.type === "directory" || candidate.type === "symlink") &&
+		typeof candidate.name === "string" &&
+		typeof candidate.target === "string"
+	)
+}
+
+export function parsePowerShellZipEntryLine(line: string): ArchiveEntry | null {
+	const parsedValue: unknown = JSON.parse(line)
+	if (!isPowerShellZipEntryRecord(parsedValue)) {
+		return null
+	}
+
+	if (parsedValue.type === "symlink") {
+		return {
+			path: parsedValue.name,
+			type: parsedValue.type,
+			linkPath: parsedValue.target,
+		}
+	}
+
+	return {
+		path: parsedValue.name,
+		type: parsedValue.type,
+	}
+}
+
 export async function listZipEntriesWithPowerShell(
 	archivePath: string,
 	escapePowerShellPath: (path: string) => string,
 	extractor: PowerShellZipExtractor
 ): Promise<ArchiveEntry[]> {
 	const proc = spawn(
-		[
-			extractor,
-			"-Command",
 			[
+				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 = ''",
+					"    $target = ''",
-				"    if ($type -eq 'symlink') {",
+					"    if ($type -eq 'symlink') {",
-				"      $stream = $entry.Open()",
+					"      $stream = $entry.Open()",
-				"      try {",
+					"      try {",
-				"        $reader = New-Object System.IO.StreamReader($stream)",
+					"        $reader = New-Object System.IO.StreamReader($stream)",
-				"        try { $target = $reader.ReadToEnd() } finally { $reader.Dispose() }",
+					"        try { $target = $reader.ReadToEnd() } finally { $reader.Dispose() }",
-				"      } finally { $stream.Dispose() }",
+					"      } finally { $stream.Dispose() }",
-				"    }",
+					"    }",
-				"    Write-Output ($type + \"`t\" + $entry.FullName + \"`t\" + $target)",
+					"    Write-Output (ConvertTo-Json @{type=$type; name=$entry.FullName; target=$target} -Compress)",
-				"  }",
+					"  }",
-				"} finally {",
+					"} finally {",
-				"  $archive.Dispose()",
+					"  $archive.Dispose()",
-				"}",
+					"}",
 			].join("; "),
 		],
 		{
@@ -55,24 +94,6 @@ export async function listZipEntriesWithPowerShell(
 		.split(/\r?\n/)
 		.map(line => line.trim())
 		.filter(Boolean)
-		.map((line): ArchiveEntry | null => {
+		.map(line => parsePowerShellZipEntryLine(line))
-			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-entry-listing/tar-zip-entry-listing.ts

@@ -11,11 +11,11 @@ function parseTarListedZipEntry(line: string): ArchiveEntry | null {
 	}
 
 	const [, rawType, rawEntryPath] = match
-	if (rawType === "l") {
+	if (rawType === "l" || rawType === "h") {
 		const arrowIndex = rawEntryPath.lastIndexOf(" -> ")
 		return {
 			path: arrowIndex === -1 ? rawEntryPath : rawEntryPath.slice(0, arrowIndex),
-			type: "symlink",
+			type: rawType === "l" ? "symlink" : "hardlink",
 			linkPath: arrowIndex === -1 ? undefined : rawEntryPath.slice(arrowIndex + 4),
 		}
 	}