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