From 3469b1ba340c09a46149d1f1a806dc4877e34491 Mon Sep 17 00:00:00 2001 From: Matteo Collina Date: Tue, 4 Aug 2026 09:00:50 +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 --- 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 aadfa1ca3cde..859a9cbd3ee4 100644 --- a/lib/internal/zip/headers.js +++ b/lib/internal/zip/headers.js @@ -500,6 +500,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