Commit c5f5cb5b968 for nodejs
commit c5f5cb5b9685b6d69b22c79b001055f385505ae7
Author: Matteo Collina <hello@matteocollina.com>
Date: Mon Sep 7 10:43:54 2026 +0200
zlib: avoid caching invalid ZIP local headers
The lazy file-backed ZIP resolver cached a parsed local header before
checking it against the central directory. A failed check therefore left
the entry with a cached header and unresolved content offset, allowing a
later read to use the file descriptor's current position.
Validate the local header and calculate its content offset before
committing either value to the entry.
Assisted-by: Pi
Signed-off-by: Matteo Collina <hello@matteocollina.com>
PR-URL: https://github.com/nodejs-private/node-private/pull/981
Refs: https://hackerone.com/reports/4004290
Reviewed-By: Robert Nagy <ronagy@icloud.com>
CVE-ID: CVE-2026-106070
diff --git a/lib/internal/zip/entry.js b/lib/internal/zip/entry.js
index cfe54903a12..c9d46c9688a 100644
--- a/lib/internal/zip/entry.js
+++ b/lib/internal/zip/entry.js
@@ -410,12 +410,14 @@ class ZipEntry {
if (length > 30) {
await readFdFully(fd, full.subarray(30), this.#localOffset + 30);
}
- this.#local = new LocalFileHeader(full, 0);
+ const local = new LocalFileHeader(full, 0);
if (this.#central !== null) {
- assertConsistentLocalHeader(this.#local, this.#central);
+ assertConsistentLocalHeader(local, this.#central);
}
- this.#contentOffset = this.#localOffset + length;
- return this.#local;
+ const contentOffset = this.#localOffset + length;
+ this.#local = local;
+ this.#contentOffset = contentOffset;
+ return local;
}
// Sync counterpart of #resolveLocalHeader().
#resolveLocalHeaderSync() {
@@ -433,12 +435,14 @@ class ZipEntry {
if (length > 30) {
readFdFullySync(fd, full.subarray(30), this.#localOffset + 30);
}
- this.#local = new LocalFileHeader(full, 0);
+ const local = new LocalFileHeader(full, 0);
if (this.#central !== null) {
- assertConsistentLocalHeader(this.#local, this.#central);
+ assertConsistentLocalHeader(local, this.#central);
}
- this.#contentOffset = this.#localOffset + length;
- return this.#local;
+ const contentOffset = this.#localOffset + length;
+ this.#local = local;
+ this.#contentOffset = contentOffset;
+ return local;
}
// The offset where this file-backed entry's compressed data begins, reading
// the local header first if that has not been resolved yet.
diff --git a/test/parallel/test-zlib-zip-security-hardening.js b/test/parallel/test-zlib-zip-security-hardening.js
index 97d5105a616..83e7b6e86ac 100644
--- a/test/parallel/test-zlib-zip-security-hardening.js
+++ b/test/parallel/test-zlib-zip-security-hardening.js
@@ -18,6 +18,7 @@ require('../common');
const assert = require('assert');
const { test } = require('node:test');
const zlib = require('zlib');
+const fs = require('fs');
const path = require('path');
const { spawnSync } = require('child_process');
const tmpdir = require('../common/tmpdir');
@@ -76,6 +77,116 @@ test('a local/central encryption-flag disagreement is rejected', () => {
readsThrow(buf, 'a.txt');
});
+// 1d. A failed lazy local-header check must not leave a file-backed entry in
+// partially resolved state. Otherwise, a later read uses -1 as its file offset
+// and reads from the descriptor's current position.
+test('failed local-header validation does not poison file-backed entries', async () => {
+ const decoy = Buffer.from('SCANNER-SEES-BENIGN!!');
+ const payload = Buffer.from('UNZIP-SEES-PAYLOAD!!!');
+ const entry = zlib.ZipEntry.createSync(
+ 'entry.txt', payload, { method: 'store' });
+ const chunks = [];
+ for (const chunk of zlib.createZipArchiveSync(
+ [entry], { baseOffset: decoy.length })) {
+ chunks.push(chunk);
+ }
+ const buf = Buffer.concat([decoy, ...chunks]);
+ buf.writeUInt32LE(zlib.crc32(decoy), centralOffset(buf) + 16);
+
+ tmpdir.refresh();
+ const file = path.join(tmpdir.path, 'mismatched.zip');
+ fs.writeFileSync(file, buf);
+ const invalid = { code: 'ERR_ZIP_INVALID_ARCHIVE' };
+
+ {
+ const zip = zlib.ZipFile.openSync(file);
+ try {
+ const fileEntry = zip.getSync('entry.txt');
+ assert.ok(fileEntry.modified instanceof Date);
+ assert.throws(() => fileEntry.contentSync(), invalid);
+ assert.throws(() => fileEntry.contentSync(), invalid);
+ } finally {
+ zip.closeSync();
+ }
+ }
+
+ {
+ const zip = zlib.ZipFile.openSync(file);
+ try {
+ const fileEntry = zip.getSync('entry.txt');
+ assert.throws(() => fileEntry.contentSync(), invalid);
+ assert.throws(() => fileEntry.contentSync(), invalid);
+ } finally {
+ zip.closeSync();
+ }
+ }
+
+ {
+ const zip = await zlib.ZipFile.open(file);
+ try {
+ const fileEntry = await zip.get('entry.txt');
+ assert.ok(fileEntry.modified instanceof Date);
+ await assert.rejects(fileEntry.content(), invalid);
+ await assert.rejects(fileEntry.content(), invalid);
+ } finally {
+ await zip.close();
+ }
+ }
+
+ {
+ const zip = await zlib.ZipFile.open(file);
+ try {
+ const fileEntry = await zip.get('entry.txt');
+ await assert.rejects(fileEntry.content(), invalid);
+ await assert.rejects(fileEntry.content(), invalid);
+ } finally {
+ await zip.close();
+ }
+ }
+
+ {
+ const zip = await zlib.ZipFile.open(file);
+ try {
+ const fileEntry = await zip.get('entry.txt');
+ assert.ok(fileEntry.modified instanceof Date);
+ await assert.rejects(async () => {
+ for await (const chunk of fileEntry.contentIterator()) {
+ assert.ok(Buffer.isBuffer(chunk));
+ }
+ }, invalid);
+ await assert.rejects(async () => {
+ for await (const chunk of await zip.stream('entry.txt')) {
+ assert.ok(Buffer.isBuffer(chunk));
+ }
+ }, invalid);
+ } finally {
+ await zip.close();
+ }
+ }
+
+ {
+ const zip = zlib.ZipFile.openSync(file);
+ try {
+ assert.throws(() => zip.compactSync(), invalid);
+ } finally {
+ zip.closeSync();
+ }
+ }
+
+ {
+ const zip = await zlib.ZipFile.open(file);
+ try {
+ await assert.rejects(async () => {
+ for await (const chunk of zip.compact()) {
+ assert.ok(Buffer.isBuffer(chunk));
+ }
+ }, invalid);
+ } finally {
+ await zip.close();
+ }
+ }
+});
+
// 2. zipFiles must reject a FIFO source rather than block on open() forever.
test('zipFiles rejects a FIFO source instead of hanging', () => {
if (process.platform === 'win32') return; // no mkfifo