diff --git a/packages/zip/lib/extract.js b/packages/zip/lib/extract.js index 4cc6d8b80..8af095b27 100644 --- a/packages/zip/lib/extract.js +++ b/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) { @@ -163,8 +169,6 @@ module.exports = async (zipToExtract, destination, options) => { let totalUncompressedBytes = 0; let entriesProcessed = 0; - const extract = require('extract-zip'); - opts.dir = destination; const originalOnEntry = opts.onEntry; @@ -190,7 +194,130 @@ module.exports = async (zipToExtract, destination, options) => { } }; - await extract(zipToExtract, opts); + await extractZip(zipToExtract, opts, limits); return { path: destination }; }; + +// Minimal yauzl-based replacement for extract-zip. Two reasons we don't use +// extract-zip directly: +// 1. extract-zip is unmaintained (no release since 2020). +// 2. yauzl's built-in `AssertByteCountStream` is supposed to error when an +// entry's actual decompressed bytes exceed its declared `uncompressedSize`, +// but yauzl overrides that stream's `destroy` so the error never surfaces +// to `extract-zip`'s pipeline — which then hangs forever. We open yauzl +// with `validateEntrySizes: false` and run our own size guard below. +async function extractZip(zipPath, opts, limits) { + const yauzl = require('yauzl'); + await fs.promises.mkdir(opts.dir, { recursive: true }); + const dir = await fs.promises.realpath(opts.dir); + + const zipfile = await new Promise((resolve, reject) => { + yauzl.open(zipPath, { lazyEntries: true, validateEntrySizes: false }, (err, z) => { + if (err) return reject(err); + resolve(z); + }); + }); + + try { + await new Promise((resolve, reject) => { + let settled = false; + const fail = (err) => { + if (settled) return; + settled = true; + reject(err); + }; + + zipfile.on('error', fail); + zipfile.on('end', () => { + if (settled) return; + settled = true; + resolve(); + }); + zipfile.on('entry', (entry) => { + if (settled) return; + extractEntry(entry, zipfile, opts, limits, dir) + .then(() => settled || zipfile.readEntry()) + .catch(fail); + }); + + zipfile.readEntry(); + }); + } finally { + try { + zipfile.close(); + } catch { + // best-effort + } + } +} + +async function extractEntry(entry, zipfile, opts, limits, dir) { + // Skip macOS resource-fork entries — extract-zip did the same. + if (entry.fileName.startsWith('__MACOSX/')) return; + + // Existing pre-flight checks (size, symlinks, filename length, onEntry hook). + opts.onEntry(entry, zipfile); + + // Stat the entry: mode bits, directory marker, filesystem-mode resolution. + // Mirrors extract-zip's logic verbatim so behaviour is preserved. + const rawMode = (entry.externalFileAttributes >> 16) & 0xffff; + const IFMT = 61440; + const IFDIR = 16384; + const isDir = + (rawMode & IFMT) === IFDIR || + entry.fileName.endsWith('/') || + (entry.versionMadeBy >> 8 === 0 && entry.externalFileAttributes === 16); + const defaultMode = isDir + ? (opts.defaultDirMode && parseInt(opts.defaultDirMode, 10)) || 0o755 + : (opts.defaultFileMode && parseInt(opts.defaultFileMode, 10)) || 0o644; + const mode = (rawMode === 0 ? defaultMode : rawMode) & 0o777; + + // Path-traversal guard — port of extract-zip's realpath/relative check. + const entryPath = path.join(dir, entry.fileName); + const containerDir = isDir ? entryPath : path.dirname(entryPath); + await fs.promises.mkdir(containerDir, { recursive: true }); + const canonicalContainerDir = await fs.promises.realpath(containerDir); + if (path.relative(dir, canonicalContainerDir).split(path.sep).includes('..')) { + throw new Error( + `Out of bound path "${canonicalContainerDir}" found while processing file ${entry.fileName}`, + ); + } + + if (isDir) { + await fs.promises.chmod(entryPath, mode).catch(() => {}); + return; + } + + const readStream = await new Promise((resolve, reject) => { + zipfile.openReadStream(entry, (err, s) => (err ? reject(err) : resolve(s))); + }); + + // Streaming size enforcement on ACTUAL decompressed bytes. The pre-flight + // check above bounds declared metadata; this catches archives whose central + // directory lies about its uncompressed size (zip-bomb shape). + let entryActualBytes = 0; + const counter = new Transform({ + transform(chunk, _encoding, cb) { + entryActualBytes += chunk.length; + if (entryActualBytes > limits.perEntryUncompressedBytes) { + return cb( + 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', + errorDetails: { + entryName: entry.fileName, + observedBytes: entryActualBytes, + limitBytes: limits.perEntryUncompressedBytes, + }, + }), + ); + } + cb(null, chunk); + }, + }); + + await pipelineAsync(readStream, counter, fs.createWriteStream(entryPath, { mode })); +} diff --git a/packages/zip/package.json b/packages/zip/package.json index 07e97c99e..306711632 100644 --- a/packages/zip/package.json +++ b/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", diff --git a/packages/zip/test/zip.test.js b/packages/zip/test/zip.test.js index 563e1974b..75e09cd6b 100644 --- a/packages/zip/test/zip.test.js +++ b/packages/zip/test/zip.test.js @@ -62,6 +62,18 @@ function createZipWithEntries(zipPath, entries) { }); } +// Rewrite every central-directory header's uncompressedSize field to simulate +// a zip whose declared metadata understates the real payload. +function forgeCentralUncompressedSize(zipPath, fake) { + const buf = fs.readFileSync(zipPath); + for (let i = 0; i < buf.length - 4; i++) { + if (buf.readUInt32LE(i) === 0x02014b50) { + buf.writeUInt32LE(fake, i + 24); + } + } + fs.writeFileSync(zipPath, buf); +} + describe('Compress and Extract should be opposite functions', function () { let symlinkPath, themeFolder, zipDestination, unzipDestination; @@ -227,38 +239,56 @@ describe('Extract zip', function () { assert.equal(called, true); }); - it('preserves existing behaviour for entries without uncompressed size when no limits are configured', async function () { + function mockYauzlSingleEntry(entry) { const originalLoad = Module._load; Module._load = function (request, parent, isMain) { - if (request === 'extract-zip') { - return async (zipPath, opts) => { - opts.onEntry({ fileName: 'missing-size.txt', externalFileAttributes: 0 }, {}); + if (request === 'yauzl') { + return { + open(_zipPath, _opts, cb) { + const zipfile = new EventEmitter(); + let emitted = false; + zipfile.readEntry = () => { + setImmediate(() => { + if (emitted) return zipfile.emit('end'); + emitted = true; + zipfile.emit('entry', entry); + }); + }; + zipfile.close = () => {}; + cb(null, zipfile); + }, }; } - return originalLoad(request, parent, isMain); }; + return () => { + Module._load = originalLoad; + }; + } + + it('preserves existing behaviour for entries without uncompressed size when no limits are configured', async function () { + // Trailing slash → treated as directory, so we don't need a mock openReadStream. + const restore = mockYauzlSingleEntry({ + fileName: 'missing-size-dir/', + externalFileAttributes: 0, + versionMadeBy: 0, + }); try { const result = await extract(zipDestination, unzipDestination); assert.equal(result.path, unzipDestination); } finally { - Module._load = originalLoad; + restore(); } }); it('throws when a limited entry has no uncompressed size', async function () { - const originalLoad = Module._load; - Module._load = function (request, parent, isMain) { - if (request === 'extract-zip') { - return async (zipPath, opts) => { - opts.onEntry({ fileName: 'missing-size.txt', externalFileAttributes: 0 }, {}); - }; - } - - return originalLoad(request, parent, isMain); - }; + const restore = mockYauzlSingleEntry({ + fileName: 'missing-size.txt', + externalFileAttributes: 0, + versionMadeBy: 0, + }); try { await assert.rejects( @@ -284,28 +314,17 @@ describe('Extract zip', function () { }, ); } finally { - Module._load = originalLoad; + restore(); } }); it('throws when a limited entry has a non-numeric uncompressed size', async function () { - const originalLoad = Module._load; - Module._load = function (request, parent, isMain) { - if (request === 'extract-zip') { - return async (zipPath, opts) => { - opts.onEntry( - { - fileName: 'invalid-size.txt', - externalFileAttributes: 0, - uncompressedSize: Number.NaN, - }, - {}, - ); - }; - } - - return originalLoad(request, parent, isMain); - }; + const restore = mockYauzlSingleEntry({ + fileName: 'invalid-size.txt', + externalFileAttributes: 0, + versionMadeBy: 0, + uncompressedSize: Number.NaN, + }); try { await assert.rejects( @@ -326,10 +345,33 @@ describe('Extract zip', function () { }, ); } finally { - Module._load = originalLoad; + restore(); } }); + it('rejects a zip whose central directory understates an entry size (streaming defence)', async function () { + // The metadata-only pre-flight check sees a tiny declared size and lets it + // through; the streaming counter sees the real 1 MB payload and rejects. + await createZipWithEntries(zipDestination, [ + { name: 'lying.txt', content: Buffer.alloc(1024 * 1024, 'a') }, + ]); + forgeCentralUncompressedSize(zipDestination, 5); + + await assert.rejects( + () => + extract(zipDestination, unzipDestination, { + limits: { perEntryUncompressedBytes: 100 }, + }), + (err) => { + assert.equal(err.code, 'ENTRY_TOO_LARGE'); + assert.equal(err.errorDetails.entryName, 'lying.txt'); + assert.equal(err.errorDetails.limitBytes, 100); + assert.equal(err.errorDetails.observedBytes > 100, true); + return true; + }, + ); + }); + it('throws when an entry exceeds the per-entry uncompressed size limit', async function () { await createZipWithEntries(zipDestination, [{ name: 'big-file.txt', content: '12345' }]); diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 22f9015c1..4f8340338 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -748,9 +748,9 @@ importers: archiver: specifier: 7.0.1 version: 7.0.1 - extract-zip: - specifier: 2.0.1 - version: 2.0.1 + yauzl: + specifier: 3.3.0 + version: 3.3.0 devDependencies: folder-hash: specifier: 4.1.2 @@ -2579,9 +2579,6 @@ packages: '@types/yargs@17.0.35': resolution: {integrity: sha512-qUHkeCyQFxMXg79wQfTtfndEC+N9ZZg76HJftDJp+qH2tV7Gj4OJi7l+PiWwJ+pWtW8GwSmqsDj/oymhrTWXjg==} - '@types/yauzl@2.10.3': - resolution: {integrity: sha512-oJoftv0LSuaDZE3Le4DbKX+KS9G36NzOeSap90UIK0yMA/NhKJhqlSGtNDORNRaIbQfzjXDrQa0ytJ6mNRGz/Q==} - '@ungap/structured-clone@1.3.1': resolution: {integrity: sha512-mUFwbeTqrVgDQxFveS+df2yfap6iuP20NAKAsBt5jDEoOTDew+zwLAOilHCeQJOVSvmgCX4ogqIrA0mnyr08yQ==} @@ -3590,11 +3587,6 @@ packages: resolution: {integrity: sha512-hIS4idWWai69NezIdRt2xFVofaF4j+6INOpJlVOLDO8zXGpUVEVzIYk12UUi2JzjEzWL3IOAxcTubgz9Po0yXw==} engines: {node: '>= 18'} - extract-zip@2.0.1: - resolution: {integrity: sha512-GDhU9ntwuKyGXdZBUgTIe+vXnWj0fppUEtMDL0+idd5Sta8TGpHssn/eusA9mrPr9qNDym6SxAYZjNvCn/9RBg==} - engines: {node: '>= 10.17.0'} - hasBin: true - fast-deep-equal@3.1.3: resolution: {integrity: sha512-f3qQ9oQy9j2AhBe/H9VC91wLmKBCCU/gDOnKNAYG5hswO7BLKj09Hc5HYNz9cGI++xlpDCIgDaitVs03ATR84Q==} @@ -3623,9 +3615,6 @@ packages: fb-watchman@2.0.2: resolution: {integrity: sha512-p5161BqbuCaSnB8jIbzQHOlpgsPmK5rJVDfDKO91Axs5NC1uu3HRQm6wt9cd9/+GtQQIO53JdGXXoyDpTAsgYA==} - fd-slicer@1.1.0: - resolution: {integrity: sha512-cE1qsB/VwyQozZ+q1dGxR8LBYNZeofhEdUNGSMbQD3Gw2lAzX9Zb3uIU6Ebc/Fmyjo9AWWfnn0AUCHqtevs/8g==} - fdir@6.5.0: resolution: {integrity: sha512-tIbYtZbucOs0BRGqPJkshJUYdL+SDH7dVM8gjy+ERp3WAUjLEFJE+02kanyHtwjWOnwrKYBiwAmM0p4kLJAnXg==} engines: {node: '>=12.0.0'} @@ -3779,10 +3768,6 @@ packages: resolution: {integrity: sha512-sTSfBjoXBp89JvIKIefqw7U2CCebsc74kiY6awiGogKtoSGbgjYE/G/+l9sF3MWFPNc9IcoOC4ODfKHfxFmp0g==} engines: {node: '>= 0.4'} - get-stream@5.2.0: - resolution: {integrity: sha512-nBF+F1rAZVCu/p7rjzgA+Yb4lfYXrpl7a6VmJrU8wF9I1CKvP/QwPNZHnOlwbTkY6dvtFIzFMSyQXbLoTQPRpA==} - engines: {node: '>=8'} - get-stream@9.0.1: resolution: {integrity: sha512-kVCxPF3vQM/N0B1PmoqVUqgHP+EeVjmZSQn+1oCRPxd2P21P2F19lIgbR3HBosbB1PUhOAoctJnfEn2GbN2eZA==} engines: {node: '>=18'} @@ -5491,8 +5476,9 @@ packages: resolution: {integrity: sha512-7dSzzRQ++CKnNI/krKnYRV7JKKPUXMEh61soaHKg9mrWEhzFWhFnxPxGl+69cD1Ou63C13NUPCnmIcrvqCuM6w==} engines: {node: '>=12'} - yauzl@2.10.0: - resolution: {integrity: sha512-p4a9I6X6nu6IhoGmBqAcbJy1mlC4j27vEPZX9F4L4/vZT3Lyq1VkFHw/V/PUcB9Buo+DG3iHkT0x3Qya58zc3g==} + yauzl@3.3.0: + resolution: {integrity: sha512-PtGEvEP30p7sbIBJKUBjUnqgTVOyMURc4dLo9iNyAJnNIEz9pm88cCXF21w94Kg3k6RXkeZh5DHOGS0qEONvNQ==} + engines: {node: '>=12'} yocto-queue@0.1.0: resolution: {integrity: sha512-rVksvsnNCdJ/ohGc6xgPwyN8eheCxsiLM8mxuE/t/mOVqJewPuO1miLpTHQiRgTKCLexL4MeAFVagts7HmNZ2Q==} @@ -7624,11 +7610,6 @@ snapshots: dependencies: '@types/yargs-parser': 21.0.3 - '@types/yauzl@2.10.3': - dependencies: - '@types/node': 24.12.3 - optional: true - '@ungap/structured-clone@1.3.1': {} '@vitest/coverage-v8@4.1.5(vitest@4.1.5)': @@ -8563,16 +8544,6 @@ snapshots: transitivePeerDependencies: - supports-color - extract-zip@2.0.1: - dependencies: - debug: 4.4.3 - get-stream: 5.2.0 - yauzl: 2.10.0 - optionalDependencies: - '@types/yauzl': 2.10.3 - transitivePeerDependencies: - - supports-color - fast-deep-equal@3.1.3: {} fast-fifo@1.3.2: {} @@ -8603,10 +8574,6 @@ snapshots: dependencies: bser: 2.1.1 - fd-slicer@1.1.0: - dependencies: - pend: 1.2.0 - fdir@6.5.0(picomatch@4.0.4): optionalDependencies: picomatch: 4.0.4 @@ -8740,10 +8707,6 @@ snapshots: dunder-proto: 1.0.1 es-object-atoms: 1.1.1 - get-stream@5.2.0: - dependencies: - pump: 3.0.4 - get-stream@9.0.1: dependencies: '@sec-ant/readable-stream': 0.4.1 @@ -10620,10 +10583,10 @@ snapshots: y18n: 5.0.8 yargs-parser: 21.1.1 - yauzl@2.10.0: + yauzl@3.3.0: dependencies: buffer-crc32: 0.2.13 - fd-slicer: 1.1.0 + pend: 1.2.0 yocto-queue@0.1.0: {}