diff --git a/src/shared/archive-entry-validator.test.ts b/src/shared/archive-entry-validator.test.ts index 96c1bbbba..c8efc7f67 100644 --- a/src/shared/archive-entry-validator.test.ts +++ b/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() diff --git a/src/shared/archive-entry-validator.ts b/src/shared/archive-entry-validator.ts index 39d117759..46319779a 100644 --- a/src/shared/archive-entry-validator.ts +++ b/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})` + ) } } } diff --git a/src/shared/binary-downloader.ts b/src/shared/binary-downloader.ts index f36829c77..bb6918c30 100644 --- a/src/shared/binary-downloader.ts +++ b/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), } } diff --git a/src/shared/zip-entry-listing/powershell-zip-entry-listing.test.ts b/src/shared/zip-entry-listing/powershell-zip-entry-listing.test.ts new file mode 100644 index 000000000..85caa10ae --- /dev/null +++ b/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) + }) + }) +}) diff --git a/src/shared/zip-entry-listing/powershell-zip-entry-listing.ts b/src/shared/zip-entry-listing/powershell-zip-entry-listing.ts index d1c9558e9..9169f510b 100644 --- a/src/shared/zip-entry-listing/powershell-zip-entry-listing.ts +++ b/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 + 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 { 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 = ''", - " 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()", - "}", + " $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 (ConvertTo-Json @{type=$type; name=$entry.FullName; target=$target} -Compress)", + " }", + "} finally {", + " $archive.Dispose()", + "}", ].join("; "), ], { @@ -55,24 +94,6 @@ export async function listZipEntriesWithPowerShell( .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, - } - }) + .map(line => parsePowerShellZipEntryLine(line)) .filter((entry): entry is ArchiveEntry => entry !== null) } diff --git a/src/shared/zip-entry-listing/tar-zip-entry-listing.ts b/src/shared/zip-entry-listing/tar-zip-entry-listing.ts index 8aec02c3b..05c33ac1b 100644 --- a/src/shared/zip-entry-listing/tar-zip-entry-listing.ts +++ b/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), } }