fix: enforce a shared PNG decompression budget

audit/shared-resource-budget.test.ts

@@ -0,0 +1,247 @@
+import { describe, expect, it } from 'vitest';
+import zlib from 'node:zlib';
+import { cleanFile, inspectFile } from '../src/core/pipeline';
+import { readChunks } from '../src/core/png';
+import { MalformedFileError } from '../src/core/types';
+import { fixture, runBytes } from './harness';
+
+const crc = (buf: Buffer): number => {
+  let c = ~0;
+  for (const b of buf) {
+    c ^= b;
+    for (let k = 0; k < 8; k++) c = (c >>> 1) ^ (0xedb88320 & -(c & 1));
+  }
+  return (~c) >>> 0;
+};
+
+const makeChunk = (type: string, payload: Buffer): Buffer => {
+  const body = Buffer.concat([Buffer.from(type, 'latin1'), payload]);
+  const out = Buffer.alloc(8 + payload.length + 4);
+  out.writeUInt32BE(payload.length, 0);
+  body.copy(out, 4);
+  out.writeUInt32BE(crc(body), 8 + payload.length);
+  return out;
+};
+
+const makeIccChunk = (decompressedSize: number, name = 'custom'): Buffer => {
+  const iccBuf = Buffer.alloc(decompressedSize, 0);
+  iccBuf.writeUInt32BE(decompressedSize, 0);
+  iccBuf.set(Buffer.from('acsp', 'latin1'), 36);
+
+  const compressedIcc = zlib.deflateSync(iccBuf);
+  return makeChunk('iCCP', Buffer.concat([
+    Buffer.from(`${name}\0`, 'latin1'),
+    Buffer.from([0]),
+    compressedIcc,
+  ]));
+};
+
+const makeZtxtChunk = (decompressedSize: number, keyword = 'comment'): Buffer => {
+  const textBuf = Buffer.alloc(decompressedSize, 0x41);
+  const compressedText = zlib.deflateSync(textBuf);
+  return makeChunk('zTXt', Buffer.concat([
+    Buffer.from(`${keyword}\0\0`, 'latin1'),
+    compressedText,
+  ]));
+};
+
+const insertChunks = (basePng: Uint8Array, chunks: Buffer[]): Uint8Array => {
+  const source = Buffer.from(basePng);
+  const idat = readChunks(new Uint8Array(source)).find((c) => c.type === 'IDAT')!;
+  return new Uint8Array(Buffer.concat([
+    source.subarray(0, idat.start),
+    ...chunks,
+    source.subarray(idat.start),
+  ]));
+};
+
+describe('shared decompression resource budget', () => {
+  const base = fixture('clean.png');
+
+  it('text alone under limit (8 MiB ok)', async () => {
+    // 8 chunks of 1 MiB each = 8 MiB text
+    const chunks = Array.from({ length: 8 }, (_, i) => makeZtxtChunk(1024 * 1024, `txt${i}`));
+    const png = insertChunks(base, chunks);
+    const report = await inspectFile(png);
+    expect(report.format).toBe('png');
+    expect(report.findings.filter((f) => f.container === 'zTXt')).toHaveLength(8);
+  }, 30000);
+
+  it('ICC alone under limit (16 MiB ok)', async () => {
+    // 1 ICC profile of 16 MiB
+    const icc = makeIccChunk(16 * 1024 * 1024);
+    const png = insertChunks(base, [icc]);
+    const report = await inspectFile(png);
+    expect(report.format).toBe('png');
+    expect(report.findings.some((f) => f.container === 'iCCP-profile')).toBe(true);
+  }, 30000);
+
+  it('text + ICC together under total limit (8 MiB text + 8 MiB ICC = 16 MiB ok)', async () => {
+    const icc = makeIccChunk(8 * 1024 * 1024);
+    const textChunks = Array.from({ length: 8 }, (_, i) => makeZtxtChunk(1024 * 1024, `txt${i}`));
+    const png = insertChunks(base, [icc, ...textChunks]);
+    const report = await inspectFile(png);
+    expect(report.format).toBe('png');
+    expect(report.findings.filter((f) => f.container === 'zTXt')).toHaveLength(8);
+    expect(report.findings.some((f) => f.container === 'iCCP-profile')).toBe(true);
+  }, 30000);
+
+  it('text + ICC together over total limit (8 MiB text + 8.1 MiB ICC refused with MalformedFileError)', async () => {
+    // 8 MiB text + 8.1 MiB ICC = 16.1 MiB (> 16 MiB total)
+    const icc = makeIccChunk(8 * 1024 * 1024 + 100 * 1024);
+    const textChunks = Array.from({ length: 8 }, (_, i) => makeZtxtChunk(1024 * 1024, `txt${i}`));
+    const png = insertChunks(base, [icc, ...textChunks]);
+
+    await expect(inspectFile(png)).rejects.toThrow(MalformedFileError);
+    await expect(inspectFile(png)).rejects.toThrow(
+      'The metadata in this image unpacks to more than FilePass will read, so it will not vouch for it.'
+    );
+  }, 30000);
+
+  it('text over 8 MiB alone refused (even without ICC)', async () => {
+    // 9 chunks of 1 MiB = 9 MiB (> 8 MiB text budget)
+    const textChunks = Array.from({ length: 9 }, (_, i) => makeZtxtChunk(1024 * 1024, `txt${i}`));
+    const png = insertChunks(base, textChunks);
+
+    await expect(inspectFile(png)).rejects.toThrow(MalformedFileError);
+    await expect(inspectFile(png)).rejects.toThrow(
+      'The text in this image unpacks to more than FilePass will read, so it will not vouch for it.'
+    );
+  }, 30000);
+
+  it('text chunk over 1 MiB refused', async () => {
+    // Single chunk unpacking to 1.1 MiB (> 1 MiB single chunk ceiling)
+    const largeChunk = makeZtxtChunk(1024 * 1024 + 100 * 1024, 'large');
+    const png = insertChunks(base, [largeChunk]);
+
+    await expect(inspectFile(png)).rejects.toThrow(MalformedFileError);
+    await expect(inspectFile(png)).rejects.toThrow(
+      'The text in this image unpacks to more than FilePass will read, so it will not vouch for it.'
+    );
+  }, 30000);
+
+  it('multiple chunks cumulative refusal', async () => {
+    // 17 chunks of 500 KB = 8.5 MiB (> 8 MiB text budget)
+    const textChunks = Array.from({ length: 17 }, (_, i) => makeZtxtChunk(500 * 1024, `txt${i}`));
+    const png = insertChunks(base, textChunks);
+
+    await expect(inspectFile(png)).rejects.toThrow(MalformedFileError);
+    await expect(inspectFile(png)).rejects.toThrow(
+      'The text in this image unpacks to more than FilePass will read, so it will not vouch for it.'
+    );
+  }, 30000);
+
+  it('corrupt/broken streams inside budget handled gracefully', async () => {
+    // Truncated deflate stream that unpacks 50 KB then ends abruptly
+    const textBuf = Buffer.alloc(50 * 1024, 0x42);
+    const compressed = zlib.deflateSync(textBuf);
+    const truncatedCompressed = compressed.subarray(0, compressed.length - 10);
+    const brokenChunk = makeChunk('zTXt', Buffer.concat([
+      Buffer.from('broken\0\0', 'latin1'),
+      truncatedCompressed,
+    ]));
+
+    const png = insertChunks(base, [brokenChunk]);
+    const report = await inspectFile(png);
+    expect(report.format).toBe('png');
+    const brokenFinding = report.findings.find((f) => f.container === 'zTXt');
+    expect(brokenFinding).toBeDefined();
+    expect(brokenFinding?.value).toBe('compressed text FilePass can remove but did not decode');
+  });
+
+  it('failed decompression after partial consumption charges the budget', async () => {
+    // 8 broken zTXt chunks that each produce ~950 KB before truncating
+    // Total partially decompressed ~ 7.6 MiB
+    const brokenChunks = Array.from({ length: 8 }, (_, i) => {
+      const textBuf = Buffer.alloc(950 * 1024, 0x43);
+      const compressed = zlib.deflateSync(textBuf);
+      // Truncate last 15 bytes to force decompression error at end of stream
+      const truncated = compressed.subarray(0, compressed.length - 15);
+      return makeChunk('zTXt', Buffer.concat([
+        Buffer.from(`broken${i}\0\0`, 'latin1'),
+        truncated,
+      ]));
+    });
+
+    // 9th valid chunk of 900 KB.
+    // If broken chunks were free, 0 + 900 KB <= 8 MiB (would pass).
+    // Because broken chunks charged ~7.6 MiB, 7.6 MiB + 900 KB > 8 MiB (refused).
+    const finalValidChunk = makeZtxtChunk(900 * 1024, 'final');
+
+    const png = insertChunks(base, [...brokenChunks, finalValidChunk]);
+    await expect(inspectFile(png)).rejects.toThrow(MalformedFileError);
+    await expect(inspectFile(png)).rejects.toThrow(
+      'The text in this image unpacks to more than FilePass will read, so it will not vouch for it.'
+    );
+  }, 30000);
+
+  /**
+   * The case this fix was written for, kept as a regression: against 71f320b a 23 KiB PNG
+   * unpacked to about 22.5 MiB of metadata and was accepted, and the clean copy verified.
+   */
+  it('the reported adversarial file - 15 MiB ICC plus 7.5 MiB text - is refused end to end', async () => {
+    const png = insertChunks(base, [
+      makeIccChunk(15 * 1024 * 1024),
+      ...Array.from({ length: 8 }, (_, i) => makeZtxtChunk(960 * 1024, `txt${i}`)),
+    ]);
+    expect(png.length, 'a small file asking for a lot').toBeLessThan(64 * 1024);
+
+    await expect(inspectFile(png)).rejects.toThrow(
+      'The metadata in this image unpacks to more than FilePass will read, so it will not vouch for it.',
+    );
+
+    // and through the product path, where the refusal has to end in no verdict and no download
+    const outcome = await runBytes(png, 'metadata-bomb.png');
+    expect(outcome.verdict).toBeUndefined();
+    expect(outcome.downloadable).toBe(false);
+    expect(outcome.error).toMatch(/unpacks to more than FilePass will read/);
+  }, 60000);
+
+  it('the shared budget does not depend on which of the two comes first', async () => {
+    const text = () => Array.from({ length: 8 }, (_, i) => makeZtxtChunk(1024 * 1024, `txt${i}`));
+    // 8 MiB text + 8.5 MiB ICC = 16.5 MiB either way round
+    const iccFirst = insertChunks(base, [makeIccChunk(8 * 1024 * 1024 + 512 * 1024), ...text()]);
+    const textFirst = insertChunks(base, [...text(), makeIccChunk(8 * 1024 * 1024 + 512 * 1024)]);
+
+    for (const [name, png] of [['ICC first', iccFirst], ['text first', textFirst]] as const) {
+      await expect(inspectFile(png), name).rejects.toThrow(
+        'The metadata in this image unpacks to more than FilePass will read, so it will not vouch for it.',
+      );
+    }
+  }, 60000);
+
+  it('an ICC that fills the whole budget leaves no room for even a small text chunk', async () => {
+    // Neither ceiling is breached on its own: the profile is under 16 MiB and the text is
+    // far under 8 MiB. Only the shared total refuses this.
+    const png = insertChunks(base, [makeIccChunk(16 * 1024 * 1024 - 4096), makeZtxtChunk(64 * 1024, 'small')]);
+    await expect(inspectFile(png)).rejects.toThrow(
+      'The metadata in this image unpacks to more than FilePass will read, so it will not vouch for it.',
+    );
+  }, 60000);
+
+  /**
+   * What the cleaner can and cannot do here. It never inflates text - it drops those chunks
+   * whole - so the shared total cannot bind on this path, and the budget threaded into
+   * clean() is future-proofing rather than a second enforced gate. What it does enforce is
+   * the profile ceiling: a colour profile FilePass cannot vouch for is never rewritten.
+   */
+  it('the cleaner still refuses a profile it cannot vouch for, so cleaning is not a way round', async () => {
+    const png = insertChunks(base, [makeIccChunk(16 * 1024 * 1024 + 100 * 1024)]);
+    const report = { format: 'png' as const, byteLength: png.length, findings: [], notes: [] };
+    await expect(cleanFile(png, report)).rejects.toThrow(MalformedFileError);
+  }, 60000);
+
+  it('the cleaner does not decompress text at all, which is why the total cannot bind there', async () => {
+    // 8 MiB of text plus a profile that on its own is within every ceiling. Inspection
+    // refuses it on the shared total; the cleaner, asked directly, only reads the profile.
+    const png = insertChunks(base, [
+      makeIccChunk(15 * 1024 * 1024),
+      ...Array.from({ length: 8 }, (_, i) => makeZtxtChunk(960 * 1024, `txt${i}`)),
+    ]);
+    await expect(inspectFile(png)).rejects.toThrow(MalformedFileError);
+
+    const report = { format: 'png' as const, byteLength: png.length, findings: [], notes: [] };
+    await expect(cleanFile(png, report), 'documented, not desired: the cleaner sees only the profile')
+      .resolves.toBeTruthy();
+  }, 60000);
+});

src/core/png.ts

@@ -67,7 +67,11 @@ type Inflated =
   | { status: 'overflow' }
   | { status: 'unreadable' };
 
-async function inflateBounded(data: Uint8Array, limit: number, budget?: TextBudget): Promise<Inflated> {
+async function inflateBounded(
+  data: Uint8Array,
+  limit: number,
+  onBytes?: (chunkLength: number) => void,
+): Promise<Inflated> {
   if (typeof DecompressionStream === 'undefined') return { status: 'unreadable' };
   try {
     const stream = new Blob([data as BlobPart]).stream().pipeThrough(new DecompressionStream('deflate'));
@@ -78,11 +82,15 @@ async function inflateBounded(data: Uint8Array, limit: number, budget?: TextBudg
       const { done, value } = await reader.read();
       if (done) break;
       total += value.length;
-      // Charged as it arrives. A stream that hands over most of its bytes and then dies has
-      // still spent them, and counting only streams that finish would let a broken one work
-      // for free.
-      budget?.spend(value.length);
+      // The chunk that breaks the ceiling is not charged, which is only safe because every
+      // caller turns 'overflow' into a refusal. If one ever tolerates it, charge first.
       if (total > limit) { await reader.cancel(); return { status: 'overflow' }; }
+      try {
+        onBytes?.(value.length);
+      } catch (error) {
+        await reader.cancel();
+        throw error;
+      }
       parts.push(value);
     }
     return { status: 'ok', bytes: concat(parts) };
@@ -93,27 +101,42 @@ async function inflateBounded(data: Uint8Array, limit: number, budget?: TextBudg
 }
 
 /**
- * What FilePass is willing to unpack from one text chunk, and from a whole file. Compressed
- * text metadata is small in every real image; these are product limits chosen so a file
- * cannot make FilePass spend unbounded memory, not a rule the PNG format states.
+ * What FilePass is willing to unpack from text chunks, ICC profiles, and a whole file.
+ * Compressed metadata is small in every real image; these are product limits chosen so a file
+ * cannot make FilePass spend unbounded memory, not rules the PNG format states.
  */
 const MAX_TEXT_BYTES = 1024 * 1024;
 const MAX_TEXT_BUDGET = 8 * 1024 * 1024;
+const MAX_PROFILE_BYTES = 16 * 1024 * 1024;
+const MAX_TOTAL_DECOMPRESSION_BUDGET = 16 * 1024 * 1024;
 
 /** Raised when a file asks FilePass to unpack more than it is willing to. */
-class TextBudget {
-  private spent = 0;
+class DecompressionBudget {
+  private totalSpent = 0;
+  private textSpent = 0;
 
-  spend(amount: number): void {
-    this.spent += amount;
-    if (this.spent > MAX_TEXT_BUDGET) {
+  spendText(amount: number): void {
+    this.textSpent += amount;
+    if (this.textSpent > MAX_TEXT_BUDGET) {
       throw new MalformedFileError('The text in this image unpacks to more than FilePass will read, so it will not vouch for it.');
     }
+    this.spendTotal(amount);
+  }
+
+  spendIcc(amount: number): void {
+    this.spendTotal(amount);
+  }
+
+  private spendTotal(amount: number): void {
+    this.totalSpent += amount;
+    if (this.totalSpent > MAX_TOTAL_DECOMPRESSION_BUDGET) {
+      throw new MalformedFileError('The metadata in this image unpacks to more than FilePass will read, so it will not vouch for it.');
+    }
   }
 }
 
-async function inflate(data: Uint8Array, budget: TextBudget): Promise<Uint8Array | undefined> {
-  const unpacked = await inflateBounded(data, MAX_TEXT_BYTES, budget);
+async function inflate(data: Uint8Array, budget: DecompressionBudget): Promise<Uint8Array | undefined> {
+  const unpacked = await inflateBounded(data, MAX_TEXT_BYTES, (n) => budget.spendText(n));
   if (unpacked.status === 'overflow') {
     throw new MalformedFileError('The text in this image unpacks to more than FilePass will read, so it will not vouch for it.');
   }
@@ -122,7 +145,7 @@ async function inflate(data: Uint8Array, budget: TextBudget): Promise<Uint8Array
 }
 
 /** Reads keyword and text out of a tEXt / zTXt / iTXt chunk. Values may be untrusted text. */
-async function readTextChunk(chunk: Chunk, budget: TextBudget): Promise<{ keyword: string; text: string }> {
+async function readTextChunk(chunk: Chunk, budget: DecompressionBudget): Promise<{ keyword: string; text: string }> {
   const nul = chunk.data.indexOf(0);
   const keyword = utf8(chunk.data.subarray(0, nul < 0 ? chunk.data.length : nul));
   const rest = chunk.data.subarray(nul + 1);
@@ -146,9 +169,6 @@ const clip = (s: string) => (s.length > 160 ? `${s.slice(0, 157)}...` : s);
 /** The label FilePass writes in place of whatever text a profile was carrying. */
 const PROFILE_LABEL = 'ICC profile';
 
-/** No real profile comes close to this, and it stops a crafted chunk from expanding forever. */
-const MAX_PROFILE_BYTES = 16 * 1024 * 1024;
-
 const CRC_TABLE = (() => {
   const table = new Uint32Array(256);
   for (let n = 0; n < 256; n++) {
@@ -315,8 +335,8 @@ function assertChunkCardinality(chunks: Chunk[]): void {
  * be large enough to hold an ICC header, agree with its own declared size, and carry the
  * ICC signature. This is a validity check for the claim FilePass makes, not colour management.
  */
-async function profileBytes(rest: Uint8Array): Promise<Uint8Array> {
-  const unpacked = await inflateBounded(rest.subarray(1), MAX_PROFILE_BYTES);
+async function profileBytes(rest: Uint8Array, budget: DecompressionBudget): Promise<Uint8Array> {
+  const unpacked = await inflateBounded(rest.subarray(1), MAX_PROFILE_BYTES, (n) => budget.spendIcc(n));
   const profile = unpacked.status === 'ok' ? unpacked.bytes : undefined;
   if (!profile) {
     throw new MalformedFileError('The colour profile in this image could not be unpacked, so FilePass will not vouch for it.');
@@ -361,15 +381,15 @@ export async function inspect(bytes: Uint8Array): Promise<InspectionReport> {
   assertRetainedChunkShapes(chunks);
   const findings: Finding[] = [];
   const notes: string[] = [];
-  const budget = new TextBudget();
+  const budget = new DecompressionBudget();
 
   for (const chunk of chunks) {
     if (chunk.type === 'iCCP') {
       // The colour profile itself has to stay, and FilePass says so rather than letting it
       // ride along undisclosed. Its name is free text with no effect on rendering, so that
       // part is replaced with a plain label.
       const { name, rest } = readProfileChunk(chunk);
-      const profile = await profileBytes(rest);
+      const profile = await profileBytes(rest, budget);
       if (name && name !== PROFILE_LABEL) {
         findings.push({
           id: 'iCCP#name',
@@ -468,11 +488,12 @@ export async function clean(bytes: Uint8Array, report: InspectionReport): Promis
   assertRetainedChunkShapes(chunks);
   const parts: Uint8Array[] = [bytes.subarray(0, 8)];
   const removedContainers = new Set<string>();
+  const budget = new DecompressionBudget();
 
   for (const chunk of chunks) {
     if (chunk.type === 'iCCP') {
       const { name, rest } = readProfileChunk(chunk);
-      await profileBytes(rest);   // never rewrite a chunk whose profile FilePass cannot vouch for
+      await profileBytes(rest, budget);   // never rewrite a chunk whose profile FilePass cannot vouch for
       if (name && name !== PROFILE_LABEL) {
         const label = Uint8Array.from(PROFILE_LABEL, (ch) => ch.charCodeAt(0));
         parts.push(buildChunk('iCCP', concat([label, Uint8Array.of(0), rest])));