Enforce zip size limits on actual decompressed bytes

packages/zip/README.md

@@ -26,6 +26,19 @@ let res = await zip.compress('path/to/a/folder', 'path/to/archive.zip', [options
 let res = await zip.extract('path/to/archive.zip', 'path/to/files', [options])
 ```
 
+`extract` accepts optional uncompressed size limits:
+
+```js
+await zip.extract('path/to/archive.zip', 'path/to/files', {
+    limits: {
+        perEntryUncompressedBytes: 536870912,
+        totalUncompressedBytes: 4294967296,
+    },
+});
+```
+
+Limits default to `Infinity` to preserve existing behaviour when omitted. They are enforced against _actual_ decompressed bytes during streaming, not just the declared central-directory metadata — so a zip whose header lies about its uncompressed size is still caught.
+
 ## Develop
 
 This is a mono repository, managed with [Nx](https://nx.dev).

packages/zip/lib/extract.js

@@ -1,5 +1,11 @@
+const fs = require('fs');
+const path = require('path');
+const { promisify } = require('util');
+const { pipeline, Transform } = require('stream');
 const errors = require('@tryghost/errors');
 
+const pipelineAsync = promisify(pipeline);
+
 const defaultOptions = {};
 
 function normalizeLimit(value, fieldName) {
@@ -70,12 +76,8 @@ function getEntryUncompressedSize(entry, limits) {
     return entry.uncompressedSize;
 }
 
-function throwOnEntryTooLarge(entry, observedBytes, limitBytes) {
-    if (observedBytes <= limitBytes) {
-        return;
-    }
-
-    throw new errors.UnsupportedMediaTypeError({
+function entryTooLargeError(entry, observedBytes, limitBytes) {
+    return new errors.UnsupportedMediaTypeError({
         message: 'Zip entry exceeds maximum uncompressed size.',
         context: 'The zip contains an entry that exceeds the maximum uncompressed size.',
         code: 'ENTRY_TOO_LARGE',
@@ -87,12 +89,8 @@ function throwOnEntryTooLarge(entry, observedBytes, limitBytes) {
     });
 }
 
-function throwOnTotalTooLarge(entry, observedBytes, limitBytes, entriesProcessed) {
-    if (observedBytes <= limitBytes) {
-        return;
-    }
-
-    throw new errors.UnsupportedMediaTypeError({
+function totalTooLargeError(entry, observedBytes, limitBytes, entriesProcessed) {
+    return new errors.UnsupportedMediaTypeError({
         message: 'Zip exceeds maximum total uncompressed size.',
         context: 'The zip exceeds the maximum total uncompressed size.',
         code: 'TOTAL_TOO_LARGE',
@@ -105,6 +103,18 @@ function throwOnTotalTooLarge(entry, observedBytes, limitBytes, entriesProcessed
     });
 }
 
+function throwOnEntryTooLarge(entry, observedBytes, limitBytes) {
+    if (observedBytes > limitBytes) {
+        throw entryTooLargeError(entry, observedBytes, limitBytes);
+    }
+}
+
+function throwOnTotalTooLarge(entry, observedBytes, limitBytes, entriesProcessed) {
+    if (observedBytes > limitBytes) {
+        throw totalTooLargeError(entry, observedBytes, limitBytes, entriesProcessed);
+    }
+}
+
 function throwOnSymlinks(entry) {
     // Check if symlink
     const mode = (entry.externalFileAttributes >> 16) & 0xffff;
@@ -128,6 +138,72 @@ function throwOnLargeFilenames(entry) {
     }
 }
 
+// Ported from extract-zip: decide whether an entry is a directory.
+function isDirectoryEntry(entry) {
+    const mode = (entry.externalFileAttributes >> 16) & 0xffff;
+    const IFMT = 61440;
+    const IFDIR = 16384;
+
+    if ((mode & IFMT) === IFDIR) {
+        return true;
+    }
+
+    if (entry.fileName.endsWith('/')) {
+        return true;
+    }
+
+    // Windows-only marker — DOS-mode 16 means "directory" when madeBy is DOS (0).
+    // See https://github.com/maxogden/extract-zip/issues/13#issuecomment-154494566
+    const madeBy = entry.versionMadeBy >> 8;
+    if (madeBy === 0 && entry.externalFileAttributes === 16) {
+        return true;
+    }
+
+    return false;
+}
+
+// Ported from extract-zip: resolve the filesystem mode bits to apply to an entry.
+function getEntryMode(entry, opts, isDir) {
+    let mode = (entry.externalFileAttributes >> 16) & 0xffff;
+
+    if (mode === 0) {
+        if (isDir) {
+            mode = opts.defaultDirMode ? parseInt(opts.defaultDirMode, 10) : 0o755;
+        } else {
+            mode = opts.defaultFileMode ? parseInt(opts.defaultFileMode, 10) : 0o644;
+        }
+    }
+
+    return mode & 0o777;
+}
+
+// Streaming byte counter — enforces per-entry and total limits against actual
+// decompressed bytes, not the declared central-directory metadata. This is the
+// real zip-bomb defence: a forged uncompressedSize gets past the pre-flight
+// metadata check, but the actual decompressed bytes can't slip past this.
+class StreamingCounter extends Transform {
+    constructor({ entry, perEntryLimit, onBytes }) {
+        super();
+        this.entry = entry;
+        this.perEntryLimit = perEntryLimit;
+        this.entryBytes = 0;
+        this.onBytes = onBytes;
+    }
+
+    _transform(chunk, _encoding, cb) {
+        this.entryBytes += chunk.length;
+        if (this.entryBytes > this.perEntryLimit) {
+            return cb(entryTooLargeError(this.entry, this.entryBytes, this.perEntryLimit));
+        }
+        try {
+            this.onBytes(chunk.length);
+        } catch (err) {
+            return cb(err);
+        }
+        cb(null, chunk);
+    }
+}
+
 /**
  * Extract
  *
@@ -146,37 +222,151 @@ function throwOnLargeFilenames(entry) {
 module.exports = async (zipToExtract, destination, options) => {
     const opts = Object.assign({}, defaultOptions, options);
     const limits = getLimits(opts);
-    let totalUncompressedBytes = 0;
+    let totalActualBytes = 0;
     let entriesProcessed = 0;
 
-    const extract = require('extract-zip');
+    // Lazy require so tests can swap yauzl out via Module._load before each call.
+    const yauzl = require('yauzl');
+    const openZip = promisify(yauzl.open);
 
-    opts.dir = destination;
+    await fs.promises.mkdir(destination, { recursive: true });
+    const dir = await fs.promises.realpath(destination);
 
-    const originalOnEntry = opts.onEntry;
-    opts.onEntry = (entry, zipfile) => {
-        const entryUncompressedBytes = getEntryUncompressedSize(entry, limits);
+    // `validateEntrySizes: false` disables yauzl's built-in AssertByteCountStream
+    // because it overrides the stream's `destroy` in a way that silently swallows
+    // mid-stream errors and leaves any downstream pipeline waiting forever. We
+    // enforce sizes ourselves below via StreamingCounter, which propagates errors
+    // through the pipeline cleanly.
+    const zipfile = await openZip(zipToExtract, {
+        lazyEntries: true,
+        validateEntrySizes: false,
+    });
 
-        throwOnEntryTooLarge(entry, entryUncompressedBytes, limits.perEntryUncompressedBytes);
+    let settled = false;
 
-        totalUncompressedBytes += entryUncompressedBytes;
-        entriesProcessed += 1;
+    try {
+        await new Promise((resolve, reject) => {
+            zipfile.on('error', (err) => {
+                if (settled) {
+                    return;
+                }
+                settled = true;
+                reject(err);
+            });
 
-        throwOnTotalTooLarge(
-            entry,
-            totalUncompressedBytes,
-            limits.totalUncompressedBytes,
-            entriesProcessed,
-        );
+            zipfile.on('end', () => {
+                if (settled) {
+                    return;
+                }
+                settled = true;
+                resolve();
+            });
 
-        throwOnSymlinks(entry);
-        throwOnLargeFilenames(entry);
-        if (originalOnEntry) {
-            originalOnEntry(entry, zipfile);
-        }
-    };
+            zipfile.on('entry', (entry) => {
+                if (settled) {
+                    return;
+                }
 
-    await extract(zipToExtract, opts);
+                Promise.resolve()
+                    .then(() =>
+                        processEntry(entry, zipfile, opts, limits, dir, {
+                            addTotal: (n) => {
+                                totalActualBytes += n;
+                                throwOnTotalTooLarge(
+                                    entry,
+                                    totalActualBytes,
+                                    limits.totalUncompressedBytes,
+                                    entriesProcessed,
+                                );
+                            },
+                            markProcessed: () => {
+                                entriesProcessed += 1;
+                            },
+                        }),
+                    )
+                    .then(() => {
+                        if (!settled) {
+                            zipfile.readEntry();
+                        }
+                    })
+                    .catch((err) => {
+                        if (settled) {
+                            return;
+                        }
+                        settled = true;
+                        reject(err);
+                    });
+            });
+
+            zipfile.readEntry();
+        });
+    } finally {
+        try {
+            zipfile.close();
+        } catch {
+            // best-effort close
+        }
+    }
 
     return { path: destination };
 };
+
+async function processEntry(entry, zipfile, opts, limits, dir, { addTotal, markProcessed }) {
+    // Skip macOS resource-fork entries — they never round-trip cleanly and are noise.
+    if (entry.fileName.startsWith('__MACOSX/')) {
+        return;
+    }
+
+    // Pre-flight metadata check: cheap rejection of zips that honestly declare
+    // themselves too large, before we touch the read stream. The streaming
+    // counter below catches archives that lie about their declared size.
+    const declaredBytes = getEntryUncompressedSize(entry, limits);
+    throwOnEntryTooLarge(entry, declaredBytes, limits.perEntryUncompressedBytes);
+
+    markProcessed();
+
+    throwOnSymlinks(entry);
+    throwOnLargeFilenames(entry);
+    if (opts.onEntry) {
+        opts.onEntry(entry, zipfile);
+    }
+
+    const entryPath = path.join(dir, entry.fileName);
+    const isDir = isDirectoryEntry(entry);
+    const mode = getEntryMode(entry, opts, isDir);
+    const containerDir = isDir ? entryPath : path.dirname(entryPath);
+
+    await fs.promises.mkdir(containerDir, { recursive: true });
+
+    // Path-traversal guard: after mkdir, resolve the actual on-disk path and
+    // make sure it stays within `dir`. Catches both `../` shenanigans and
+    // symlink-target shenanigans created in earlier entries of the same zip.
+    const canonicalContainerDir = await fs.promises.realpath(containerDir);
+    const relative = path.relative(dir, canonicalContainerDir);
+    if (relative.split(path.sep).includes('..')) {
+        throw new Error(
+            `Out of bound path "${canonicalContainerDir}" found while processing file ${entry.fileName}`,
+        );
+    }
+
+    if (isDir) {
+        // Apply requested mode to the directory we just created.
+        await fs.promises.chmod(entryPath, mode).catch(() => {});
+        // Account declared bytes against the total — directories typically declare
+        // 0, but preserve the pre-existing behaviour of including them in the count.
+        addTotal(declaredBytes);
+        return;
+    }
+
+    const openReadStream = promisify(zipfile.openReadStream.bind(zipfile));
+    const readStream = await openReadStream(entry);
+    const writeStream = fs.createWriteStream(entryPath, { mode });
+
+    const counter = new StreamingCounter({
+        entry,
+        perEntryLimit: limits.perEntryUncompressedBytes,
+        onBytes: addTotal,
+    });
+
+    await pipelineAsync(readStream, counter, writeStream);
+}

packages/zip/package.json

@@ -25,7 +25,7 @@
   "dependencies": {
     "@tryghost/errors": "workspace:*",
     "archiver": "7.0.1",
-    "extract-zip": "2.0.1"
+    "yauzl": "3.3.0"
   },
   "devDependencies": {
     "folder-hash": "4.1.2",