diff --git a/infra/flue-review/.flue/lib/untar.ts b/infra/flue-review/.flue/lib/untar.ts index 1d3ff67d16..0e60448f7d 100644 --- a/infra/flue-review/.flue/lib/untar.ts +++ b/infra/flue-review/.flue/lib/untar.ts @@ -10,6 +10,11 @@ export interface UntarTarget { writeFileBytes(path: string, content: Uint8Array): Promise; } +/** Ceiling on a single entry's declared size; content is buffered in DO memory. */ +const MAX_ENTRY_SIZE = 64 * 1024 * 1024; + +const OCTAL_FIELD = /^[0-7]*$/; + function stripRoot(name: string): string | undefined { const slash = name.indexOf("/"); if (slash === -1) return undefined; @@ -38,9 +43,9 @@ function normalizeSegments(path: string): string[] | undefined { /** * Untar a ustar stream into `destDir`, stripping the archive's single * top-level directory. Handles regular files, directories, symlinks, GNU - * longname ('L') and pax ('x') path overrides. One entry's content is - * buffered at a time. Rejects any entry path or symlink target that would - * resolve outside `destDir`. + * longname/longlink ('L'/'K') and pax ('x') path/linkpath overrides. One + * entry's content is buffered at a time, capped at MAX_ENTRY_SIZE. Rejects + * any entry path or symlink target that would resolve outside `destDir`. */ export async function untarInto( target: UntarTarget, @@ -52,7 +57,9 @@ export async function untarInto( let files = 0; let bytes = 0; let pendingLongName: string | undefined; + let pendingLongLink: string | undefined; let pendingPaxPath: string | undefined; + let pendingPaxLink: string | undefined; const dirsMade = new Set(); const append = (chunk: Uint8Array) => { @@ -90,12 +97,18 @@ export async function untarInto( const rawName = readCString(header.subarray(0, 100)); const prefix = readCString(header.subarray(345, 500)); - const size = parseInt(readCString(header.subarray(124, 136)).trim() || "0", 8); + const sizeField = readCString(header.subarray(124, 136)).trim(); + if (!OCTAL_FIELD.test(sizeField)) { + throw new Error(`tar entry size is not octal ("${sizeField}"): ${rawName}`); + } + const size = sizeField ? parseInt(sizeField, 8) : 0; // Mode bytes (100-108) are ignored: the workspace has no chmod and the // reviewer never executes files. const type = String.fromCharCode(header[156] ?? 48); - const linkTarget = readCString(header.subarray(157, 257)); + if (size > MAX_ENTRY_SIZE) { + throw new Error(`tar entry size out of range (${size} bytes): ${rawName}`); + } const padded = Math.ceil(size / 512) * 512; if (!(await need(padded)) && size > 0) { throw new Error(`tar truncated: needed ${padded} bytes for entry ${rawName}`); @@ -107,22 +120,30 @@ export async function untarInto( pendingLongName = readCString(content); continue; } + if (type === "K") { + pendingLongLink = readCString(content); + continue; + } if (type === "x" || type === "g") { // pax records: " key=value\n" const text = decoder.decode(content); for (const line of text.split("\n")) { const eq = line.indexOf("="); - if (eq > 0 && line.slice(line.indexOf(" ") + 1, eq) === "path") { - pendingPaxPath = line.slice(eq + 1); - } + if (eq <= 0) continue; + const key = line.slice(line.indexOf(" ") + 1, eq); + if (key === "path") pendingPaxPath = line.slice(eq + 1); + else if (key === "linkpath") pendingPaxLink = line.slice(eq + 1); } continue; } const fullName = pendingPaxPath ?? pendingLongName ?? (prefix ? `${prefix}/${rawName}` : rawName); + const linkTarget = pendingPaxLink ?? pendingLongLink ?? readCString(header.subarray(157, 257)); pendingLongName = undefined; + pendingLongLink = undefined; pendingPaxPath = undefined; + pendingPaxLink = undefined; if (fullName.startsWith("/")) { throw new Error(`tar entry path is absolute: ${fullName}`); diff --git a/infra/flue-review/.flue/sandboxes/cloudflare-shell.ts b/infra/flue-review/.flue/sandboxes/cloudflare-shell.ts index 475d6761ac..0b97fc51a6 100644 --- a/infra/flue-review/.flue/sandboxes/cloudflare-shell.ts +++ b/infra/flue-review/.flue/sandboxes/cloudflare-shell.ts @@ -237,7 +237,7 @@ function instrumentR2(bucket: R2Bucket): R2Bucket { console.log(JSON.stringify({ message: "r2 op start", id, method, key })); try { // oxlint-disable-next-line typescript/no-unsafe-type-assertion - const result = await (bucket as unknown as Record)[method](...args); + const result = await (bucket as unknown as Record)[method]!(...args); console.log(JSON.stringify({ message: "r2 op end", id, method, key, ms: Date.now() - t0 })); return result; } catch (error) { diff --git a/infra/flue-review/test/untar.test.ts b/infra/flue-review/test/untar.test.ts index 47e3459d44..abf27fbe8d 100644 --- a/infra/flue-review/test/untar.test.ts +++ b/infra/flue-review/test/untar.test.ts @@ -126,6 +126,72 @@ describe("untarInto", () => { expect(r.files.has("/repo/ignored-name.txt")).toBe(false); }); + it("applies pax linkpath and GNU longlink overrides to symlink targets", async () => { + const longTarget = `deep/${"d/".repeat(40)}target.txt`; + const paxBody = `linkpath=${longTarget}\n`; + const paxLen = String(paxBody.length + 3).length + 1 + paxBody.length; + const paxBytes = encoder.encode(`${paxLen} ${paxBody}`); + const longLinkBytes = encoder.encode(longTarget); + const r = recorder(); + await untarInto( + r.target, + tarball([ + header({ name: "repo-abc/PaxHeader", type: "x", size: paxBytes.length }), + contentBlocks(paxBytes), + header({ name: "repo-abc/pax-link", type: "2", linkTarget: "short" }), + header({ name: "repo-abc/@LongLink", type: "K", size: longLinkBytes.length }), + contentBlocks(longLinkBytes), + header({ name: "repo-abc/gnu-link", type: "2", linkTarget: "short" }), + header({ name: "repo-abc/plain-link", type: "2", linkTarget: "short" }), + ]), + "/repo", + ); + expect(r.symlinks.get("/repo/pax-link")).toBe(longTarget); + expect(r.symlinks.get("/repo/gnu-link")).toBe(longTarget); + expect(r.symlinks.get("/repo/plain-link")).toBe("short"); + }); + + it("rejects pax linkpath targets that escape the destination", async () => { + const paxBody = "linkpath=../../etc/passwd\n"; + const paxLen = String(paxBody.length + 3).length + 1 + paxBody.length; + const paxBytes = encoder.encode(`${paxLen} ${paxBody}`); + const r = recorder(); + await expect( + untarInto( + r.target, + tarball([ + header({ name: "repo-abc/PaxHeader", type: "x", size: paxBytes.length }), + contentBlocks(paxBytes), + header({ name: "repo-abc/evil", type: "2", linkTarget: "harmless" }), + ]), + "/repo", + ), + ).rejects.toThrow(/escapes the destination/); + expect(r.symlinks.size).toBe(0); + }); + + it("rejects entries whose size field is not strictly octal", async () => { + for (const sizeField of ["10x", "size!", "-0000001"]) { + const block = header({ name: "repo-abc/bad.bin" }); + block.set(encoder.encode(sizeField), 124); + const r = recorder(); + await expect(untarInto(r.target, tarball([block]), "/repo")).rejects.toThrow(/not octal/); + expect(r.files.size).toBe(0); + } + }); + + it("rejects entries whose declared size exceeds the cap", async () => { + const r = recorder(); + await expect( + untarInto( + r.target, + tarball([header({ name: "repo-abc/huge.bin", size: 128 * 1024 * 1024 })]), + "/repo", + ), + ).rejects.toThrow(/size out of range/); + expect(r.files.size).toBe(0); + }); + it("joins the ustar prefix field with the name", async () => { const r = recorder(); await untarInto(