diff --git a/src/artifacts/ZipArtifactReader.test.ts b/src/artifacts/ZipArtifactReader.test.ts new file mode 100644 index 000000000..679882ec7 --- /dev/null +++ b/src/artifacts/ZipArtifactReader.test.ts @@ -0,0 +1,191 @@ +import { readFile, writeFile } from "node:fs/promises"; +import { join } from "node:path"; + +import { + TextReader, + Uint8ArrayReader, + Uint8ArrayWriter, + ZipWriter, +} from "@zip.js/zip.js"; +import { describe, expect, it } from "vitest"; + +import { createTestTempDirectory } from "../../tests/fixtures/temporaryDirectory.js"; + +import { ZipArtifactReader } from "./ZipArtifactReader.js"; +import { + ARTIFACT_DECOMPRESSION_RATIO, + artifactDecompressionBudgetBytes, + MINIMUM_ARTIFACT_DECOMPRESSION_BYTES, +} from "../domain/artifactDecompressionBudget.js"; + +/** Small real ZIP carriers with forged sizes; metadata-only controls, never decompressed. */ +const writeZipWithDeclaredSize = async ( + directory: string, + declaredBytes: readonly number[], +): Promise => { + const path = join(directory, "fixture.zip"); + const writer = new ZipWriter(new Uint8ArrayWriter()); + for (const [index] of declaredBytes.entries()) + await writer.add( + `member-${index}.bin`, + new TextReader("real member bytes"), + ); + await writeFile(path, await writer.close()); + const bytes = await readFile(path); + let offset = 0; + for (const declared of declaredBytes) { + const centralDirectory = bytes.indexOf( + Buffer.from([0x50, 0x4b, 0x01, 0x02]), + offset, + ); + if (centralDirectory < 0) + throw new Error("Expected a ZIP central directory"); + // Metadata-only controls: these declarations do not describe valid member streams. + bytes.writeUInt32LE(declared, centralDirectory + 24); + offset = centralDirectory + 46; + } + await writeFile(path, bytes); + return path; +}; + +const enumerate = async (reader: ZipArtifactReader): Promise => { + let files = 0; + for await (const entry of reader.entries()) + if (entry.kind === "file") files += 1; + return files; +}; + +describe("ZipArtifactReader decompression budget", () => { + it("refuses an archive whose entries declare more than its size can justify", async () => { + const directory = await createTestTempDirectory("rea-zip-budget-bomb-"); + // A ~160-byte archive cannot declare 2 GiB of members without being a bomb. + const path = await writeZipWithDeclaredSize(directory, [2_147_483_647]); + const reader = new ZipArtifactReader(path, "zip"); + try { + await expect(enumerate(reader)).rejects.toMatchObject({ + name: "ArtifactReaderFailure", + reason: "limit", + }); + } finally { + await reader.close(); + } + }); + + it("charges all declarations in one traversal before yielding an over-budget member", async () => { + const directory = await createTestTempDirectory("rea-zip-budget-total-"); + const half = MINIMUM_ARTIFACT_DECOMPRESSION_BYTES / 2; + const path = await writeZipWithDeclaredSize(directory, [half, half + 1]); + const reader = new ZipArtifactReader(path, "zip"); + const iterator = reader.entries()[Symbol.asyncIterator](); + try { + expect((await iterator.next()).value?.declaredSize).toBe(half); + await expect(iterator.next()).rejects.toMatchObject({ reason: "limit" }); + } finally { + await iterator.return?.(); + await reader.close(); + } + }); + + it("keeps repeated and interleaved metadata traversals independently charged", async () => { + const directory = await createTestTempDirectory( + "rea-zip-budget-iterators-", + ); + const path = await writeZipWithDeclaredSize(directory, [ + MINIMUM_ARTIFACT_DECOMPRESSION_BYTES / 2, + MINIMUM_ARTIFACT_DECOMPRESSION_BYTES / 2, + ]); + const reader = new ZipArtifactReader(path, "zip"); + const first = reader.entries()[Symbol.asyncIterator](); + const second = reader.entries()[Symbol.asyncIterator](); + try { + expect((await first.next()).done).toBe(false); + expect((await second.next()).done).toBe(false); + expect((await first.next()).done).toBe(false); + expect((await second.next()).done).toBe(false); + expect((await first.next()).done).toBe(true); + expect((await second.next()).done).toBe(true); + expect(await enumerate(reader)).toBe(2); + } finally { + await Promise.all([first.return?.(), second.return?.()]); + await reader.close(); + } + }); + + it("streams a valid small member through open", async () => { + const directory = await createTestTempDirectory("rea-zip-budget-valid-"); + const path = join(directory, "valid.zip"); + const writer = new ZipWriter(new Uint8ArrayWriter()); + await writer.add("member.bin", new TextReader("valid member bytes")); + await writeFile(path, await writer.close()); + const reader = new ZipArtifactReader(path, "zip"); + try { + const entries = []; + for await (const entry of reader.entries()) entries.push(entry); + const entry = entries[0]; + if (entry === undefined) throw new Error("Expected a ZIP member"); + const chunks: Buffer[] = []; + for await (const chunk of await reader.open(entry)) + chunks.push(Buffer.from(chunk)); + expect(Buffer.concat(chunks).toString()).toBe("valid member bytes"); + } finally { + await reader.close(); + } + }); + + it("rejects under-declared member output before delivering its complete actual bytes", async () => { + const directory = await createTestTempDirectory("rea-zip-underdeclared-"); + const path = join(directory, "underdeclared.zip"); + const content = new Uint8Array(128 * 1024); + const writer = new ZipWriter(new Uint8ArrayWriter()); + await writer.add("member.bin", new Uint8ArrayReader(content), { + dataDescriptor: false, + bufferedWrite: true, + }); + const bytes = Buffer.from(await writer.close()); + const local = bytes.indexOf(Buffer.from([0x50, 0x4b, 0x03, 0x04])); + const central = bytes.indexOf(Buffer.from([0x50, 0x4b, 0x01, 0x02])); + if (local < 0 || central < 0) throw new Error("Expected ZIP entry headers"); + // Both headers agree and no descriptor remains: rejection must enforce + // actual decompressed output, not merely detect conflicting metadata. + expect(bytes.readUInt16LE(local + 6) & 8).toBe(0); + expect(bytes.readUInt16LE(local + 8)).toBe(8); + expect(bytes.readUInt32LE(local + 22)).toBe(content.length); + expect(bytes.readUInt32LE(central + 24)).toBe(content.length); + bytes.writeUInt32LE(1, local + 22); + bytes.writeUInt32LE(1, central + 24); + await writeFile(path, bytes); + const reader = new ZipArtifactReader(path, "zip"); + try { + const entries = []; + for await (const entry of reader.entries()) entries.push(entry); + const entry = entries[0]; + if (entry === undefined) throw new Error("Expected a ZIP member"); + expect(entry.declaredSize).toBe(1); + let deliveredBytes = 0; + await expect( + (async () => { + for await (const chunk of await reader.open(entry)) + deliveredBytes += Buffer.from(chunk).length; + })(), + ).rejects.toThrow("Invalid uncompressed size"); + expect(deliveredBytes).toBeLessThan(content.length); + } finally { + await reader.close(); + } + }); +}); + +describe("artifactDecompressionBudgetBytes", () => { + it("grants at least the floor and grows with the compressed size", () => { + expect(artifactDecompressionBudgetBytes(0)).toBe( + MINIMUM_ARTIFACT_DECOMPRESSION_BYTES, + ); + expect(artifactDecompressionBudgetBytes(1024)).toBe( + MINIMUM_ARTIFACT_DECOMPRESSION_BYTES, + ); + const large = MINIMUM_ARTIFACT_DECOMPRESSION_BYTES * 4; + expect(artifactDecompressionBudgetBytes(large)).toBe( + large * ARTIFACT_DECOMPRESSION_RATIO, + ); + }); +}); diff --git a/src/artifacts/ZipArtifactReader.ts b/src/artifacts/ZipArtifactReader.ts index 5ce2ee417..11c68557b 100644 --- a/src/artifacts/ZipArtifactReader.ts +++ b/src/artifacts/ZipArtifactReader.ts @@ -12,6 +12,11 @@ import { type ArtifactReader, } from "./ArtifactReader.js"; import type { ZipPackageFormat } from "../domain/zipPackageFormat.js"; +import { + ARTIFACT_DECOMPRESSION_RATIO, + artifactDecompressionBudgetBytes, + MINIMUM_ARTIFACT_DECOMPRESSION_BYTES, +} from "../domain/artifactDecompressionBudget.js"; import { OwnedFileHandle } from "../filesystem/OwnedFileHandle.js"; import { NonRegularFileReadError, @@ -132,7 +137,6 @@ export class ZipArtifactReader implements ArtifactReader { string, { readonly source: Entry; readonly metadata: ArtifactEntry } >(); - /** Optionally bound each metadata read before allocating its backing buffer. */ constructor( path: string, @@ -153,6 +157,8 @@ export class ZipArtifactReader implements ArtifactReader { async *entries(signal?: AbortSignal): AsyncIterable { let index = 0; + // A traversal owns its accounting, including overlapping or repeated passes. + let declaredFileBytes = 0; for await (const entry of this.#reader.getEntriesGenerator({ decodeText: decodeZipEntryText, })) { @@ -160,6 +166,22 @@ export class ZipArtifactReader implements ArtifactReader { // ZIP member identity is its directory occurrence, not its filename. // Logical path uniqueness belongs to the artifact inventory boundary. const metadata = projectZipEntry(entry, index++); + // zip.js enforces each member's declared output size, so this sum bounds streamed work. + if ( + metadata.kind === "file" && + !metadata.encrypted && + metadata.declaredSize !== null && + Number.isSafeInteger(metadata.declaredSize) && + metadata.declaredSize >= 0 + ) { + declaredFileBytes += metadata.declaredSize; + const budget = artifactDecompressionBudgetBytes(this.#source.size); + if (declaredFileBytes > budget) + throw new ArtifactReaderFailure( + "limit", + `ZIP entries declare ${String(declaredFileBytes)} uncompressed bytes, exceeding this archive's ${String(budget)}-byte decompression budget (${String(MINIMUM_ARTIFACT_DECOMPRESSION_BYTES)}-byte minimum, ${String(ARTIFACT_DECOMPRESSION_RATIO)}x archive size); a small archive cannot expand this far`, + ); + } const stored = this.#entries.get(metadata.adapterKey); if (stored !== undefined && !sameArtifactEntry(metadata, stored.metadata)) throw new ArtifactReaderFailure( diff --git a/src/domain/artifactDecompressionBudget.ts b/src/domain/artifactDecompressionBudget.ts new file mode 100644 index 000000000..2f9396f47 --- /dev/null +++ b/src/domain/artifactDecompressionBudget.ts @@ -0,0 +1,35 @@ +/** + * Aggregate decompression ceiling for one archive-backed artifact operation. + * + * A ZIP/APK/IPA/MSIX member is attacker-controlled and can expand far beyond the + * archive that carries it, so a small input can drive unbounded decompression + * work during inventory hashing and extraction. A fixed per-entry byte cap is + * the wrong shape: it rejects legitimately large members inside large packages + * and cannot distinguish a bomb from a large file. + * + * The ceiling instead scales with the archive's own compressed size, so a large + * package keeps a proportionally large budget while a small archive cannot + * amplify into unbounded CPU time or disk writes. The floor keeps ordinary + * small archives usable, and the ratio sits above real-world artifact + * compression (measured ~3:1-15:1, up to ~350:1 for pure text) but below the + * DEFLATE ceiling (~1030:1) that a single-entry bomb relies on. + */ + +/** Allow a whole archive to expand by this factor before decompression is refused. */ +export const ARTIFACT_DECOMPRESSION_RATIO = 500; + +/** + * Smallest aggregate budget granted to any archive, so small packages stay usable. + * This floor keeps ordinary archives extractable and stays well above the largest + * single members real packages carry; it is not a per-entry cap. + */ +export const MINIMUM_ARTIFACT_DECOMPRESSION_BYTES = 256 * 1024 * 1024; + +/** Largest total decompressed bytes allowed for an archive of this compressed size. */ +export const artifactDecompressionBudgetBytes = ( + compressedBytes: number, +): number => + Math.max( + MINIMUM_ARTIFACT_DECOMPRESSION_BYTES, + Math.floor(compressedBytes) * ARTIFACT_DECOMPRESSION_RATIO, + );