From 00f0f8cf8acced9e9e6f7256955a1480c98fb1e8 Mon Sep 17 00:00:00 2001 From: Matteo Collina Date: Wed, 12 Aug 2026 10:02:57 +0100 Subject: [PATCH] zlib: validate central directory record count ZIP readers trusted the EOCD record count. They did not check that the parsed headers consumed the declared central directory size. Reject archives whose count leaves directory bytes unparsed. Signed-off-by: Matteo Collina PR-URL: https://github.com/nodejs/node/pull/65002 Reviewed-By: Filip Skokan Reviewed-By: Antoine du Hamel --- lib/internal/zip/entry.js | 4 +++ lib/internal/zip/headers.js | 4 +++ test/parallel/test-zlib-zip-security.js | 34 +++++++++++++++++++++++++ 3 files changed, 42 insertions(+) diff --git a/lib/internal/zip/entry.js b/lib/internal/zip/entry.js index 4eab161db9b2..c9fb1cac62cd 100644 --- a/lib/internal/zip/entry.js +++ b/lib/internal/zip/entry.js @@ -941,6 +941,10 @@ function* readArchiveEntries(buf, end) { }); pos = central.byteOffset + central.byteLength; } + if (pos !== cdEnd) { + throw new ERR_ZIP_INVALID_ARCHIVE( + 'central directory record count is inconsistent with its size'); + } // Sort a separate range list; entries themselves are yielded in central // directory order. const ranges = ArrayPrototypeSort(ArrayPrototypeSlice(parsed), (a, b) => a.start - b.start); diff --git a/lib/internal/zip/headers.js b/lib/internal/zip/headers.js index 42af8aedeadd..f1f028cb511b 100644 --- a/lib/internal/zip/headers.js +++ b/lib/internal/zip/headers.js @@ -553,6 +553,10 @@ function readCentralDirectory(buffer, count) { ArrayPrototypePush(result, header); pos += header.byteLength; } + if (pos !== buffer.length) { + throw new ERR_ZIP_INVALID_ARCHIVE( + 'central directory record count is inconsistent with its size'); + } return result; } diff --git a/test/parallel/test-zlib-zip-security.js b/test/parallel/test-zlib-zip-security.js index ba63af08fb2d..aa4b37f4c608 100644 --- a/test/parallel/test-zlib-zip-security.js +++ b/test/parallel/test-zlib-zip-security.js @@ -230,6 +230,40 @@ test('open() rejects contradictory classic-EOCD vs Zip64 metadata', async () => } }, { timeout: 120_000 }); +test('central-directory record count must account for its full declared size', async () => { + const chunks = []; + for await (const chunk of zlib.createZipArchive([ + await zlib.ZipEntry.create('visible.txt', Buffer.from('visible'), { method: 'store' }), + await zlib.ZipEntry.create('hidden.txt', Buffer.from('hidden'), { method: 'store' }), + ])) chunks.push(chunk); + const tampered = Buffer.concat(chunks); + const eocd = tampered.length - 22; + assert.strictEqual(tampered.readUInt16LE(eocd + 8), 2); + assert.strictEqual(tampered.readUInt16LE(eocd + 10), 2); + + // Keep the single-disk counts consistent with each other, but make both + // disagree with the two complete records in the declared directory size. + tampered.writeUInt16LE(1, eocd + 8); + tampered.writeUInt16LE(1, eocd + 10); + const expected = { + code: 'ERR_ZIP_INVALID_ARCHIVE', + message: /central directory record count is inconsistent with its size/, + }; + + assert.throws(() => [...zlib.ZipEntry.read(tampered)], expected); + assert.throws(() => new zlib.ZipBuffer(tampered), expected); + + const dir = await fsp.mkdtemp(path.join(tmpdir.path, `zip-sec-${seq++}-`)); + const p = path.join(dir, 'record-count-mismatch.zip'); + try { + await fsp.writeFile(p, tampered); + await assert.rejects(zlib.ZipFile.open(p), expected); + assert.throws(() => zlib.ZipFile.openSync(p), expected); + } finally { + await fsp.rm(dir, { recursive: true, force: true }); + } +}); + // -- Finding 4: the file-backed open-time overlap check uses a 30-byte lower // bound for each local header, while the in-memory reader uses the exact // local-header length. A crafted "quoted overlap" archive therefore passes