Bound mcpb unpack against decompression bombs and malformed central directories

src/cli/unpack.ts

@@ -11,16 +11,28 @@ import { join, resolve, sep } from "path";
 import { extractSignatureBlock } from "../node/sign.js";
 import { getLogger } from "../shared/log.js";
 
+// Guard rails against decompression bombs. The archive is untrusted input;
+// these cap how much the central-directory pass will declare before we
+// decompress the whole thing into memory.
+const DEFAULT_MAX_UNCOMPRESSED_BYTES = 1024 * 1024 * 1024; // 1 GiB
+const DEFAULT_MAX_ENTRIES = 100_000;
+
 interface UnpackOptions {
   mcpbPath: string;
   outputDir?: string;
   silent?: boolean;
+  /** Reject archives whose declared total uncompressed size exceeds this. */
+  maxUncompressedBytes?: number;
+  /** Reject archives that declare more central-directory entries than this. */
+  maxEntries?: number;
 }
 
 export async function unpackExtension({
   mcpbPath,
   outputDir,
   silent,
+  maxUncompressedBytes = DEFAULT_MAX_UNCOMPRESSED_BYTES,
+  maxEntries = DEFAULT_MAX_ENTRIES,
 }: UnpackOptions): Promise<boolean> {
   const logger = getLogger({ silent });
   const resolvedMcpbPath = resolve(mcpbPath);
@@ -40,52 +52,79 @@ export async function unpackExtension({
     const fileContent = readFileSync(resolvedMcpbPath);
     const { originalContent } = extractSignatureBlock(fileContent);
 
-    // Parse file attributes from ZIP central directory
+    // Parse the ZIP central directory to (a) extract Unix file attributes and
+    // (b) enforce decompression-bomb limits before inflating into memory. All
+    // reads are bounds-checked against the buffer length so a malformed or
+    // truncated central directory cannot throw a RangeError mid-parse.
     const fileAttributes = new Map<string, number>();
     const isUnix = process.platform !== "win32";
 
-    if (isUnix) {
-      // Parse ZIP central directory to extract file attributes
-      const zipBuffer = originalContent;
+    const zipBuffer = originalContent;
+    const len = zipBuffer.length;
+
+    // Find end of central directory record (scan backwards from the earliest
+    // position a 22-byte EOCD could start).
+    let eocdOffset = -1;
+    for (let i = len - 22; i >= 0; i--) {
+      if (zipBuffer.readUInt32LE(i) === 0x06054b50) {
+        eocdOffset = i;
+        break;
+      }
+    }
+
+    if (eocdOffset !== -1 && eocdOffset + 20 <= len) {
+      const centralDirOffset = zipBuffer.readUInt32LE(eocdOffset + 16);
+      const centralDirEntries = zipBuffer.readUInt16LE(eocdOffset + 8);
 
-      // Find end of central directory record
-      let eocdOffset = -1;
-      for (let i = zipBuffer.length - 22; i >= 0; i--) {
-        if (zipBuffer.readUInt32LE(i) === 0x06054b50) {
-          eocdOffset = i;
+      if (centralDirEntries > maxEntries) {
+        throw new Error(
+          `Archive declares too many entries (${centralDirEntries} > ${maxEntries})`,
+        );
+      }
+
+      let offset = centralDirOffset;
+      let totalUncompressed = 0;
+
+      for (let i = 0; i < centralDirEntries; i++) {
+        // Need at least the 46-byte fixed central-directory header.
+        if (offset < 0 || offset + 46 > len) {
+          break;
+        }
+        if (zipBuffer.readUInt32LE(offset) !== 0x02014b50) {
           break;
         }
-      }
 
-      if (eocdOffset !== -1) {
-        const centralDirOffset = zipBuffer.readUInt32LE(eocdOffset + 16);
-        const centralDirEntries = zipBuffer.readUInt16LE(eocdOffset + 8);
-
-        let offset = centralDirOffset;
-
-        for (let i = 0; i < centralDirEntries; i++) {
-          if (zipBuffer.readUInt32LE(offset) === 0x02014b50) {
-            const externalAttrs = zipBuffer.readUInt32LE(offset + 38);
-            const filenameLength = zipBuffer.readUInt16LE(offset + 28);
-            const filename = zipBuffer.toString(
-              "utf8",
-              offset + 46,
-              offset + 46 + filenameLength,
-            );
-
-            // Extract Unix permissions from external attributes (upper 16 bits)
-            const mode = (externalAttrs >> 16) & 0o777;
-            if (mode > 0) {
-              fileAttributes.set(filename, mode);
-            }
+        const uncompressedSize = zipBuffer.readUInt32LE(offset + 24);
+        const externalAttrs = zipBuffer.readUInt32LE(offset + 38);
+        const filenameLength = zipBuffer.readUInt16LE(offset + 28);
+        const extraFieldLength = zipBuffer.readUInt16LE(offset + 30);
+        const commentLength = zipBuffer.readUInt16LE(offset + 32);
 
-            const extraFieldLength = zipBuffer.readUInt16LE(offset + 30);
-            const commentLength = zipBuffer.readUInt16LE(offset + 32);
-            offset += 46 + filenameLength + extraFieldLength + commentLength;
-          } else {
-            break;
+        if (offset + 46 + filenameLength > len) {
+          break;
+        }
+
+        totalUncompressed += uncompressedSize;
+        if (totalUncompressed > maxUncompressedBytes) {
+          throw new Error(
+            `Archive uncompressed size exceeds limit (${maxUncompressedBytes} bytes)`,
+          );
+        }
+
+        if (isUnix) {
+          const filename = zipBuffer.toString(
+            "utf8",
+            offset + 46,
+            offset + 46 + filenameLength,
+          );
+          // Extract Unix permissions from external attributes (upper 16 bits)
+          const mode = (externalAttrs >> 16) & 0o777;
+          if (mode > 0) {
+            fileAttributes.set(filename, mode);
           }
         }
+
+        offset += 46 + filenameLength + extraFieldLength + commentLength;
       }
     }
 

test/unpack-limits.test.ts

@@ -0,0 +1,74 @@
+import fs from "node:fs";
+import { join } from "node:path";
+
+import { zipSync } from "fflate";
+
+import { unpackExtension } from "../src/cli/unpack";
+
+describe("unpackExtension decompression limits", () => {
+  const tmpRoot = join(__dirname, "temp-unpack-limits");
+  const mcpbPath = join(tmpRoot, "demo.mcpb");
+  const outDir = join(tmpRoot, "out");
+
+  beforeEach(() => {
+    fs.rmSync(tmpRoot, { recursive: true, force: true });
+    fs.mkdirSync(tmpRoot, { recursive: true });
+  });
+
+  afterAll(() => {
+    fs.rmSync(tmpRoot, { recursive: true, force: true });
+  });
+
+  function writeArchive(files: Record<string, Uint8Array>): void {
+    fs.writeFileSync(mcpbPath, zipSync(files));
+  }
+
+  it("unpacks a normal archive within the default limits", async () => {
+    writeArchive({
+      "manifest.json": new TextEncoder().encode("{}"),
+      "index.js": new TextEncoder().encode("console.log(1);"),
+    });
+
+    const ok = await unpackExtension({
+      mcpbPath,
+      outputDir: outDir,
+      silent: true,
+    });
+
+    expect(ok).toBe(true);
+    expect(fs.existsSync(join(outDir, "index.js"))).toBe(true);
+  });
+
+  it("rejects an archive whose declared uncompressed size exceeds the limit", async () => {
+    writeArchive({
+      "big.bin": new Uint8Array(2000),
+    });
+
+    const ok = await unpackExtension({
+      mcpbPath,
+      outputDir: outDir,
+      silent: true,
+      maxUncompressedBytes: 100,
+    });
+
+    expect(ok).toBe(false);
+    expect(fs.existsSync(join(outDir, "big.bin"))).toBe(false);
+  });
+
+  it("rejects an archive that declares more entries than the limit", async () => {
+    writeArchive({
+      "a.txt": new TextEncoder().encode("a"),
+      "b.txt": new TextEncoder().encode("b"),
+      "c.txt": new TextEncoder().encode("c"),
+    });
+
+    const ok = await unpackExtension({
+      mcpbPath,
+      outputDir: outDir,
+      silent: true,
+      maxEntries: 1,
+    });
+
+    expect(ok).toBe(false);
+  });
+});