From f3a301eca5d126f072a481dfb053f5e80f29290a Mon Sep 17 00:00:00 2001 From: morluto <76467478+morluto@users.noreply.github.com> Date: Tue, 27 Oct 2026 09:45:03 +0800 Subject: [PATCH] fix(artifacts): validate selected path traversal --- src/artifacts/ArtifactSelectedSource.ts | 15 ++- .../extraction/ArtifactExtraction.ts | 7 +- src/artifacts/inventory/ArtifactInventory.ts | 6 +- .../artifactAdmittedIdentity.test.ts | 95 +++++++++++++++++++ .../artifactInventoryRegularRoot.test.ts | 19 ++++ 5 files changed, 135 insertions(+), 7 deletions(-) diff --git a/src/artifacts/ArtifactSelectedSource.ts b/src/artifacts/ArtifactSelectedSource.ts index fa79e6a5c6..9aaec293c9 100644 --- a/src/artifacts/ArtifactSelectedSource.ts +++ b/src/artifacts/ArtifactSelectedSource.ts @@ -1,6 +1,17 @@ -import { realpath } from "node:fs/promises"; +import { realpath, stat } from "node:fs/promises"; import { ArtifactReaderFailure } from "./ArtifactReader.js"; +/** Canonicalize only a path that native filesystem lookup actually accepts. */ +export const resolveArtifactSourcePath = async ( + path: string, +): Promise => { + // Darwin realpath can collapse file/.. although opening that spelling fails + // with ENOTDIR. Keep both file and directory selections valid here; the + // inventory/reader owns format and regular-file admission. + await stat(path); + return realpath(path); +}; + /** Verify selection spelling independently of canonical descriptor ownership. */ export const assertArtifactSelectedSource = async ( canonicalPath: string, @@ -19,7 +30,7 @@ export const assertArtifactSelectedSource = async ( ); let current: string; try { - current = await realpath(selectedPath); + current = await resolveArtifactSourcePath(selectedPath); } catch (cause: unknown) { if ( cause instanceof Error && diff --git a/src/artifacts/extraction/ArtifactExtraction.ts b/src/artifacts/extraction/ArtifactExtraction.ts index ab36f69d22..b0efcd5a0d 100644 --- a/src/artifacts/extraction/ArtifactExtraction.ts +++ b/src/artifacts/extraction/ArtifactExtraction.ts @@ -1,6 +1,9 @@ import { assertAdmittedRootDigest } from "../inventory/classify.js"; import { assertArtifactFileSelectedSource } from "../ArtifactFileSource.js"; -import { assertArtifactSelectedSource } from "../ArtifactSelectedSource.js"; +import { + assertArtifactSelectedSource, + resolveArtifactSourcePath, +} from "../ArtifactSelectedSource.js"; import { hashStableRootArtifactHandle } from "../inventory/hashStableRootArtifact.js"; import { absoluteCallerPath } from "../../filesystem/callerPath.js"; import type { AsarHeaderAdmission } from "../../config/asarHeaderAdmission.js"; @@ -83,7 +86,7 @@ const extractArtifactInScope = async ( ): Promise => { // Cancellation wins over every later refusal, including unsupported formats. abortIfNeeded(signal); - const sourcePath = await realpath(input.inputPath); + const sourcePath = await resolveArtifactSourcePath(input.inputPath); if (input.expectedRootSha256 !== undefined) await assertArtifactSelectedSource(sourcePath, input.selectedPath); const admission = await requireExtractableFormat(sourcePath, input, signal); diff --git a/src/artifacts/inventory/ArtifactInventory.ts b/src/artifacts/inventory/ArtifactInventory.ts index 421fe71a4d..d3913bfb85 100644 --- a/src/artifacts/inventory/ArtifactInventory.ts +++ b/src/artifacts/inventory/ArtifactInventory.ts @@ -1,5 +1,5 @@ import { absoluteCallerPath } from "../../filesystem/callerPath.js"; -import { realpath } from "node:fs/promises"; +import { resolveArtifactSourcePath } from "../ArtifactSelectedSource.js"; import { artifactInventoryResultSchema, @@ -31,7 +31,7 @@ export const scanArtifactInventoryInScope = async ( options: ArtifactInventoryOptions, ): Promise => { abortIfNeeded(options.signal); - const path = await realpath(inputPath); + const path = await resolveArtifactSourcePath(inputPath); return scanCanonicalArtifactInventoryInScope( path, options, @@ -49,7 +49,7 @@ export const scanArtifactInventoryForExtractionInScope = async ( readonly transferredSource: TransferredArtifactInventorySource | undefined; }> => { abortIfNeeded(options.signal); - const path = await realpath(inputPath); + const path = await resolveArtifactSourcePath(inputPath); return scanCanonicalArtifactInventoryForExtractionInScope( path, options, diff --git a/tests/boundary/filesystem/artifactAdmittedIdentity.test.ts b/tests/boundary/filesystem/artifactAdmittedIdentity.test.ts index 121efd7448..f1160d2447 100644 --- a/tests/boundary/filesystem/artifactAdmittedIdentity.test.ts +++ b/tests/boundary/filesystem/artifactAdmittedIdentity.test.ts @@ -12,6 +12,7 @@ import { writeFile, unlink, symlink, + rmdir, } from "node:fs/promises"; import { join } from "node:path"; import { createPackage } from "@electron/asar"; @@ -24,6 +25,100 @@ import { } from "../../../src/domain/artifactGraph.js"; import { createTestBinarySession } from "../../fixtures/binarySession.js"; import { createTestTempDirectory } from "../../fixtures/temporaryDirectory.js"; +import { createSingleEntryZip } from "../../fixtures/zipNames.js"; + +it.skipIf(process.platform === "win32").each(["zip", "asar"])( + "rejects a %s selection whose traversed directory became a file", + async (format) => { + const root = await createTestTempDirectory("rea-admitted-traversal-"); + const child = join(root, "child"); + await mkdir(child); + const stored = join(root, `stored.${format}`); + if (format === "asar") { + const source = join(root, "source"); + await mkdir(source); + await writeFile(join(source, "payload.txt"), "member bytes"); + await createPackage(source, stored); + } else { + await writeFile(stored, await createSingleEntryZip("payload.txt")); + } + const selected = `${child}/../stored.${format}`; + const session = createTestBinarySession(new ArtifactProvider({})); + try { + const opened = await session.open(selected); + if (!opened.ok) throw opened.error; + await rmdir(child); + await writeFile(child, "now a regular file"); + await expect(readFile(selected)).rejects.toMatchObject({ + code: "ENOTDIR", + }); + for (const operation of [ + "inventory_artifact", + "extract_artifact", + ] as const) { + const output = join(root, operation); + const result = await session.execute( + operation, + operation === "extract_artifact" ? { output_root: output } : {}, + ); + expect(result).toMatchObject({ + ok: false, + error: { + reason: "integrity", + detail: expect.stringContaining( + "no longer resolves to its admitted source", + ), + }, + }); + await expect(access(output)).rejects.toMatchObject({ code: "ENOENT" }); + } + } finally { + await session.close(); + } + }, +); + +it.skipIf(process.platform === "win32")( + "keeps native symlink parent traversal when revalidating archive selections", + async () => { + const root = await createTestTempDirectory( + "rea-admitted-native-traversal-", + ); + const physical = join(root, "physical"); + await mkdir(join(physical, "child"), { recursive: true }); + const link = join(root, "link"); + await symlink(join(physical, "child"), link); + await writeFile( + join(physical, "stored.zip"), + await createSingleEntryZip("payload.txt", "selected bytes"), + ); + await writeFile( + join(root, "stored.zip"), + "lexical sibling is not an archive", + ); + const session = createTestBinarySession(new ArtifactProvider({})); + try { + const opened = await session.open(`${link}/../stored.zip`); + if (!opened.ok) throw opened.error; + const inventory = await session.execute("inventory_artifact", {}); + if (!inventory.ok) throw inventory.error; + expect( + artifactInventoryResultSchema.parse(inventory.value.result).manifest + .root_format, + ).toBe("zip"); + const output = join(root, "output"); + const extraction = await session.execute("extract_artifact", { + output_root: output, + }); + if (!extraction.ok) throw extraction.error; + expect(await readFile(join(output, "payload.txt"), "utf8")).toBe( + "selected bytes", + ); + } finally { + await session.close(); + } + }, +); it.each([ { format: "zip", replacement: "valid" }, diff --git a/tests/boundary/filesystem/artifactInventoryRegularRoot.test.ts b/tests/boundary/filesystem/artifactInventoryRegularRoot.test.ts index 10abc154d5..955832fd42 100644 --- a/tests/boundary/filesystem/artifactInventoryRegularRoot.test.ts +++ b/tests/boundary/filesystem/artifactInventoryRegularRoot.test.ts @@ -6,6 +6,7 @@ import { rename, rm, writeFile, + mkdir, } from "node:fs/promises"; import { join } from "node:path"; import { promisify } from "node:util"; @@ -26,6 +27,24 @@ import { fatImage, } from "../../../src/artifacts/apple/MachoImage.fixture.js"; +it.skipIf(process.platform === "win32").each(["file", "directory"])( + "rejects invalid native traversal before canonicalizing a %s inventory root", + async (kind) => { + const root = await createTestTempDirectory( + "rea-inventory-native-selection-", + ); + await writeFile(join(root, "blocker"), "regular file"); + const path = join(root, "target"); + if (kind === "directory") await mkdir(path); + else await writeFile(path, "target bytes"); + await expect( + inventoryArtifact(`${root}/blocker/../target`), + ).rejects.toMatchObject({ code: "ENOTDIR" }); + const valid = await inventoryArtifact(path); + expect(valid.manifest.root_format).toBe(kind); + }, +); + it.skipIf(process.platform === "win32")( "rejects a FIFO artifact root without waiting for a writer", async () => {