diff --git a/packages/compiler/src/node-islands/libnode-pack.test.ts b/packages/compiler/src/node-islands/libnode-pack.test.ts index 108e211f2b..06ddcfabdb 100644 --- a/packages/compiler/src/node-islands/libnode-pack.test.ts +++ b/packages/compiler/src/node-islands/libnode-pack.test.ts @@ -1,5 +1,13 @@ import { createHash } from "node:crypto"; -import { mkdirSync, mkdtempSync, readFileSync, readdirSync, rmSync, writeFileSync } from "node:fs"; +import { + existsSync, + mkdirSync, + mkdtempSync, + readFileSync, + readdirSync, + rmSync, + writeFileSync, +} from "node:fs"; import { tmpdir } from "node:os"; import { join } from "node:path"; import { fileURLToPath } from "node:url"; @@ -287,3 +295,44 @@ test("the license notices carry Node's LICENSE", () => { ].join("\n"), ); }); + +test("beside refuses an installed pack whose manifest names a file outside it", () => { + const root = temp(); + const name = libnodePackPackage(hostLibnodeTarget()); + if (name === null) return; + const pack = fakePack( + join(root, "node_modules", name), + (m) => { + m["files"] = { ...(m["files"] as object), "../escaped": entry("payload") }; + }, + name, + ); + // The pack's "../escaped" and the file it would replace next to the output. + writeFileSync(join(pack, "..", "escaped"), "payload"); + writeFileSync(join(root, "escaped"), "victim"); + expect(libnodeRecord("beside", join(root, "app"), {}, root)).toBe( + `the libnode pack at ${pack} names a file outside the pack ("../escaped")`, + ); + expect(readFileSync(join(root, "escaped"), "utf8")).toBe("victim"); + expect(existsSync(join(root, "app.libnode"))).toBe(false); +}); + +test("readLibnodePack refuses any listed file or license outside the pack", () => { + for (const edit of [ + (m: Record) => { + m["files"] = { ...(m["files"] as object), "../escaped": entry("payload") }; + }, + (m: Record) => { + m["files"] = { ...(m["files"] as object), "/etc/passwd": entry("payload") }; + }, + (m: Record) => { + m["license"] = "../../secret"; + }, + (m: Record) => { + m["license"] = 7; + }, + ]) { + const pack = fakePack(join(temp(), "pack"), edit); + expect(thrown(() => readLibnodePack(pack)).kind).toBe("invalid"); + } +}); diff --git a/packages/compiler/src/node-islands/libnode-pack.ts b/packages/compiler/src/node-islands/libnode-pack.ts index 4c4599b7bf..4de4a2af35 100644 --- a/packages/compiler/src/node-islands/libnode-pack.ts +++ b/packages/compiler/src/node-islands/libnode-pack.ts @@ -99,6 +99,11 @@ export function parseLibnodeSetting(value: unknown): LibnodePlacement | string { return `must be "embedded", "beside" or an absolute directory`; } +/** Whether `name` names a file directly in the pack directory. */ +function packFileName(name: string): boolean { + return name !== "" && name !== "." && name !== ".." && !/[/\\\0]/.test(name) && !isAbsolute(name); +} + function isRecord(value: unknown): value is Record { return value !== null && typeof value === "object" && !Array.isArray(value); } @@ -134,8 +139,18 @@ export function parseLibnodePackManifest(json: unknown, where: string): LibnodeP } const files = json["files"]; if (!isRecord(files)) return invalid("lists no files in manifest.json"); + // Every name the pack lists is a file directly in the pack directory. + const license = json["license"]; + if (license !== undefined && typeof license !== "string") { + return invalid("has a malformed license in manifest.json"); + } + for (const name of [...Object.keys(files), json["license"] ?? "LICENSE"] as string[]) { + if (!packFileName(name)) { + return invalid(`names a file outside the pack (${JSON.stringify(name)})`); + } + } for (const name of [json["libnode"], json["bridge"]] as string[]) { - if (name === "." || name === ".." || /[/\\\0]/.test(name) || isAbsolute(name)) { + if (!packFileName(name)) { return invalid(`names a file outside the pack (${JSON.stringify(name)})`); } if (!isFileEntry(files[name])) return invalid(`lists no size and SHA-256 for ${name}`);