zlib: reject ambiguous ZIP archive ends

ZIP archives can contain more than one structurally plausible EOCD
record. Selecting based on whether a comment reaches EOF can make the
chosen central directory depend on trailing padding.
Inspect all candidates in the common tail window and reject archives
with multiple plausible interpretations.
Signed-off-by: Matteo Collina <hello@matteocollina.com>
PR-URL: #65007
Reviewed-By: Filip Skokan <panva.ip@gmail.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>

Author: mcollina

lib/internal/zip/headers.js

@@ -27,6 +27,7 @@ const {
 MADE_BY_UNIX,
 SENTINEL16,
 SENTINEL32,
+TAIL_LENGTH,
 ZIP64_EOCD_MAX_LENGTH,
 S_IFLNK,
 S_IFMT,
@@ -310,6 +311,51 @@ class LocalFileHeader {
 }
 }
 
+// Returns whether an EOCD-looking record could describe an archive this
+// implementation supports. This is deliberately only a cheap preflight: the
+// selected record still receives the complete Zip64 and central-directory
+// validation below.
+functionisPlausibleArchiveEnd(buffer,eocdPos,scanStart){
+constdiskNumber=buffer.readUInt16LE(eocdPos+4);
+constcentralDirectoryDiskNumber=buffer.readUInt16LE(eocdPos+6);
+constdiskRecords=buffer.readUInt16LE(eocdPos+8);
+consttotalRecords=buffer.readUInt16LE(eocdPos+10);
+constcentralDirectorySize=buffer.readUInt32LE(eocdPos+12);
+constcentralDirectoryOffset=buffer.readUInt32LE(eocdPos+16);
+constneedsZip64=
+diskNumber===SENTINEL16||
+centralDirectoryDiskNumber===SENTINEL16||
+diskRecords===SENTINEL16||
+totalRecords===SENTINEL16||
+centralDirectorySize===SENTINEL32||
+centralDirectoryOffset===SENTINEL32;
+
+constlocatorPos=eocdPos-20;
+consthasZip64Locator=locatorPos>=0&&
+buffer.readUInt32LE(locatorPos)===SIG_ZIP64_EOCD_LOCATOR;
+if(needsZip64)returnhasZip64Locator;
+// A Zip64 end record may accompany authoritative, non-sentinel classic
+// fields. Its central directory does not immediately precede this EOCD.
+if(hasZip64Locator)returntrue;
+if(
+diskNumber!==0||
+centralDirectoryDiskNumber!==0||
+diskRecords!==totalRecords||
+totalRecords*46>centralDirectorySize
+){
+returnfalse;
+}
+
+constcentralDirectoryPos=eocdPos-centralDirectorySize;
+if(centralDirectoryPos<scanStart){
+// Use the same logical tail window for memory- and file-backed archives.
+// The directory is not available for this cheap preflight in either case.
+returntrue;
+}
+if(totalRecords===0)returncentralDirectorySize===0;
+returnbuffer.readUInt32LE(centralDirectoryPos)===SIG_CENTRAL_FILE_HEADER;
+}
+
 /**
  * Locates and validates the end-of-archive structures (EOCD, and the Zip64
  * EOCD locator/record when present) in `buffer`. `base` is the absolute
@@ -334,26 +380,30 @@ function findArchiveEnd(buffer, base = 0) {
 if(buffer.length<22){
 thrownewERR_ZIP_INVALID_ARCHIVE('no end of central directory record found');
 }
-constmin=MathMax(0,buffer.length-(22+SENTINEL16));
+// Use the same tail-sized search window for full buffers and ZipFile's tail
+// reads. Besides keeping candidate selection consistent, the extra tail
+// slack permits a maximum-length comment followed by modest writer padding.
+constmin=MathMax(0,buffer.length-TAIL_LENGTH);
 leteocdPos=-1;
-// Pass 1: the comment must reach exactly to the end of the buffer (this
-// rejects a stray EOCD-looking signature inside an earlier comment).
+letfallbackPos=-1;
+letexactFallbackPos=-1;
 for(letpos=buffer.length-22;pos>=min;pos--){
 if(buffer.readUInt32LE(pos)!==SIG_EOCD)continue;
-if(pos+22+buffer.readUInt16LE(pos+20)!==buffer.length)continue;
+constend=pos+22+buffer.readUInt16LE(pos+20);
+if(end>buffer.length)continue;
+if(fallbackPos<0)fallbackPos=pos;
+if(end===buffer.length&&exactFallbackPos<0)exactFallbackPos=pos;
+if(!isPlausibleArchiveEnd(buffer,pos,min))continue;
+if(eocdPos>=0){
+thrownewERR_ZIP_INVALID_ARCHIVE(
+'ambiguous end of central directory records');
+}
 eocdPos=pos;
-break;
 }
+// Preserve the targeted validation errors for a sole malformed or
+// unsupported candidate. Plausible candidates always take precedence.
 if(eocdPos<0){
-// Pass 2: tolerate trailing padding after the EOCD (some streaming
-// writers pad their output to a fixed block size); take the last
-// candidate found.
-for(letpos=buffer.length-22;pos>=min;pos--){
-if(buffer.readUInt32LE(pos)!==SIG_EOCD)continue;
-if(pos+22+buffer.readUInt16LE(pos+20)>buffer.length)continue;
-eocdPos=pos;
-break;
-}
+eocdPos=exactFallbackPos>=0 ? exactFallbackPos : fallbackPos;
 }
 if(eocdPos<0){
 thrownewERR_ZIP_INVALID_ARCHIVE('no end of central directory record found');
@@ -474,7 +524,10 @@ function findArchiveEnd(buffer, base = 0) {
 if(prefix<0){
 thrownewERR_ZIP_INVALID_ARCHIVE('central directory does not fit inside the archive');
 }
-if(totalRecords*46>centralDirectorySize){
+if(
+(totalRecords===0&&centralDirectorySize!==0)||
+totalRecords*46>centralDirectorySize
+){
 thrownewERR_ZIP_INVALID_ARCHIVE(
 'central directory record count is inconsistent with its size');
 }

test/parallel/test-zlib-zip-hardening.js

@@ -3,8 +3,10 @@
 require('../common');
 
 constassert=require('node:assert');
+constfs=require('node:fs');
 constzlib=require('node:zlib');
 const{ test }=require('node:test');
+consttmpdir=require('../common/tmpdir');
 
 asyncfunctionbuildArchive(entries,comment){
 constchunks=[];
@@ -47,13 +49,71 @@ test('an EOCD-looking signature inside a trailing comment is not mistaken for th
 // before it reaches the genuine EOCD signature; embedding 4 bytes that
 // look like one partway through must not be mistaken for the real record.
 constfakeSignature=String.fromCharCode(0x50,0x4b,0x05,0x06);
-constarchive=awaitbuildArchive([entry],`before ${fakeSignature} after`);
+constarchive=awaitbuildArchive(
+[entry],`before ${fakeSignature} this is not a valid EOCD record after`);
 
 constread=[...zlib.ZipEntry.read(archive)];
 assert.strictEqual(read.length,1);
 assert.strictEqual(read[0].name,'f.txt');
 });
 
+test('multiple plausible EOCD records describing different archives are rejected',async()=>{
+constfirst=awaitbuildArchive([
+awaitzlib.ZipEntry.create('install.sh',Buffer.from('malicious'),{method: 'store'}),
+]);
+constsecond=awaitbuildArchive([
+awaitzlib.ZipEntry.create('install.sh',Buffer.from('benign'),{method: 'store'}),
+]);
+constarchive=Buffer.concat([first,second,Buffer.from([0])]);
+constfirstEocd=first.length-22;
+
+// Make the first EOCD exact-to-EOF by treating the second archive and its
+// padding as a comment. The second EOCD remains a plausible archive end for
+// readers which tolerate trailing padding and select the rightmost record.
+archive.writeUInt16LE(archive.length-firstEocd-22,firstEocd+20);
+
+constexpected={
+code: 'ERR_ZIP_INVALID_ARCHIVE',
+message: /ambiguousendofcentraldirectory/,
+};
+assert.throws(()=>[...zlib.ZipEntry.read(archive)],expected);
+assert.throws(()=>newzlib.ZipBuffer(archive),expected);
+
+tmpdir.refresh();
+constfile=tmpdir.resolve('ambiguous.zip');
+fs.writeFileSync(file,archive);
+awaitassert.rejects(zlib.ZipFile.open(file),expected);
+assert.throws(()=>zlib.ZipFile.openSync(file),expected);
+});
+
+test('an exact EOCD embedded in a genuine comment is rejected as ambiguous',async()=>{
+constarchive=awaitbuildArchive([
+awaitzlib.ZipEntry.create('f.txt',Buffer.from('content'),{method: 'store'}),
+]);
+constnested=Buffer.concat([archive,buildEocd()]);
+nested.writeUInt16LE(22,archive.length-2);
+
+assert.throws(()=>[...zlib.ZipEntry.read(nested)],{
+code: 'ERR_ZIP_INVALID_ARCHIVE',
+message: /ambiguousendofcentraldirectory/,
+});
+});
+
+test('multiple padded EOCD records are rejected as ambiguous',async()=>{
+constfirst=awaitbuildArchive([
+awaitzlib.ZipEntry.create('a.txt',Buffer.from('first'),{method: 'store'}),
+]);
+constsecond=awaitbuildArchive([
+awaitzlib.ZipEntry.create('b.txt',Buffer.from('second'),{method: 'store'}),
+]);
+constarchive=Buffer.concat([first,second,Buffer.from('\0\0')]);
+
+assert.throws(()=>newzlib.ZipBuffer(archive),{
+code: 'ERR_ZIP_INVALID_ARCHIVE',
+message: /ambiguousendofcentraldirectory/,
+});
+});
+
 test('a declared-size mismatch is rejected as corrupt',async()=>{
 constentry=awaitzlib.ZipEntry.create('f.txt',Buffer.from('hello world'),{method: 'store'});
 constarchive=awaitbuildArchive([entry]);
@@ -212,7 +272,7 @@ test('every possible truncation of an archive is rejected, deterministically', a
 
 test('trailing padding after the EOCD is tolerated',async()=>{
 // Some streaming writers pad their output to a block size; CPython
-// tolerates trailing newlines/NULs and so does the pass-2 EOCD scan.
+// tolerates trailing newlines/NULs and so does the EOCD scan.
 constarchive=awaitbuildArchive(
 [awaitzlib.ZipEntry.create('f.txt',Buffer.from('hi'),{method: 'store'})]);
 constpadded=Buffer.concat([archive,Buffer.from('\r\n\0\0\0')]);
@@ -221,6 +281,15 @@ test('trailing padding after the EOCD is tolerated', async () => {
 assert.strictEqual((awaitentry.content()).toString(),'hi');
 });
 
+test('a maximum-length comment followed by block padding is tolerated',async()=>{
+constcomment='x'.repeat(0xffff);
+constarchive=awaitbuildArchive([],comment);
+constpadded=Buffer.concat([archive,Buffer.alloc(4096)]);
+constzip=newzlib.ZipBuffer(padded);
+
+assert.strictEqual(zip.comment,comment);
+});
+
 test('junk appended past a declared comment is tolerated and the comment preserved',async()=>{
 constarchive=awaitbuildArchive(
 [awaitzlib.ZipEntry.create('f.txt',Buffer.from('hi'),{method: 'store'})],
@@ -262,6 +331,10 @@ test('a record count inconsistent with the directory size is rejected', () => {
 constarchive=Buffer.concat([Buffer.alloc(46),eocd]);
 assert.throws(()=>[...zlib.ZipEntry.read(archive)],
 {code: 'ERR_ZIP_INVALID_ARCHIVE',message: /inconsistent/});
+
+constzeroRecords=Buffer.concat([Buffer.alloc(46),buildEocd({cdSize: 46})]);
+assert.throws(()=>[...zlib.ZipEntry.read(zeroRecords)],
+{code: 'ERR_ZIP_INVALID_ARCHIVE',message: /inconsistent/});
 });
 
 test('a corrupted or overrunning central directory header is rejected',async()=>{

lib/internal/zip/headers.js

@@ -27,6 +27,7 @@ const {
 MADE_BY_UNIX,
 SENTINEL16,
 SENTINEL32,
+TAIL_LENGTH,
 ZIP64_EOCD_MAX_LENGTH,
 S_IFLNK,
 S_IFMT,
@@ -310,6 +311,51 @@ class LocalFileHeader {
 }
 }
 
+// Returns whether an EOCD-looking record could describe an archive this
+// implementation supports. This is deliberately only a cheap preflight: the
+// selected record still receives the complete Zip64 and central-directory
+// validation below.
+functionisPlausibleArchiveEnd(buffer,eocdPos,scanStart){
+constdiskNumber=buffer.readUInt16LE(eocdPos+4);
+constcentralDirectoryDiskNumber=buffer.readUInt16LE(eocdPos+6);
+constdiskRecords=buffer.readUInt16LE(eocdPos+8);
+consttotalRecords=buffer.readUInt16LE(eocdPos+10);
+constcentralDirectorySize=buffer.readUInt32LE(eocdPos+12);
+constcentralDirectoryOffset=buffer.readUInt32LE(eocdPos+16);
+constneedsZip64=
+diskNumber===SENTINEL16||
+centralDirectoryDiskNumber===SENTINEL16||
+diskRecords===SENTINEL16||
+totalRecords===SENTINEL16||
+centralDirectorySize===SENTINEL32||
+centralDirectoryOffset===SENTINEL32;
+
+constlocatorPos=eocdPos-20;
+consthasZip64Locator=locatorPos>=0&&
+buffer.readUInt32LE(locatorPos)===SIG_ZIP64_EOCD_LOCATOR;
+if(needsZip64)returnhasZip64Locator;
+// A Zip64 end record may accompany authoritative, non-sentinel classic
+// fields. Its central directory does not immediately precede this EOCD.
+if(hasZip64Locator)returntrue;
+if(
+diskNumber!==0||
+centralDirectoryDiskNumber!==0||
+diskRecords!==totalRecords||
+totalRecords*46>centralDirectorySize
+){
+returnfalse;
+}
+
+constcentralDirectoryPos=eocdPos-centralDirectorySize;
+if(centralDirectoryPos<scanStart){
+// Use the same logical tail window for memory- and file-backed archives.
+// The directory is not available for this cheap preflight in either case.
+returntrue;
+}
+if(totalRecords===0)returncentralDirectorySize===0;
+returnbuffer.readUInt32LE(centralDirectoryPos)===SIG_CENTRAL_FILE_HEADER;
+}
+
 /**
  * Locates and validates the end-of-archive structures (EOCD, and the Zip64
  * EOCD locator/record when present) in `buffer`. `base` is the absolute
@@ -334,26 +380,30 @@ function findArchiveEnd(buffer, base = 0) {
 if(buffer.length<22){
 thrownewERR_ZIP_INVALID_ARCHIVE('no end of central directory record found');
 }
-constmin=MathMax(0,buffer.length-(22+SENTINEL16));
+// Use the same tail-sized search window for full buffers and ZipFile's tail
+// reads. Besides keeping candidate selection consistent, the extra tail
+// slack permits a maximum-length comment followed by modest writer padding.
+constmin=MathMax(0,buffer.length-TAIL_LENGTH);
 leteocdPos=-1;
-// Pass 1: the comment must reach exactly to the end of the buffer (this
-// rejects a stray EOCD-looking signature inside an earlier comment).
+letfallbackPos=-1;
+letexactFallbackPos=-1;
 for(letpos=buffer.length-22;pos>=min;pos--){
 if(buffer.readUInt32LE(pos)!==SIG_EOCD)continue;
-if(pos+22+buffer.readUInt16LE(pos+20)!==buffer.length)continue;
+constend=pos+22+buffer.readUInt16LE(pos+20);
+if(end>buffer.length)continue;
+if(fallbackPos<0)fallbackPos=pos;
+if(end===buffer.length&&exactFallbackPos<0)exactFallbackPos=pos;
+if(!isPlausibleArchiveEnd(buffer,pos,min))continue;
+if(eocdPos>=0){
+thrownewERR_ZIP_INVALID_ARCHIVE(
+'ambiguous end of central directory records');
+}
 eocdPos=pos;
-break;
 }
+// Preserve the targeted validation errors for a sole malformed or
+// unsupported candidate. Plausible candidates always take precedence.
 if(eocdPos<0){
-// Pass 2: tolerate trailing padding after the EOCD (some streaming
-// writers pad their output to a fixed block size); take the last
-// candidate found.
-for(letpos=buffer.length-22;pos>=min;pos--){
-if(buffer.readUInt32LE(pos)!==SIG_EOCD)continue;
-if(pos+22+buffer.readUInt16LE(pos+20)>buffer.length)continue;
-eocdPos=pos;
-break;
-}
+eocdPos=exactFallbackPos>=0 ? exactFallbackPos : fallbackPos;
 }
 if(eocdPos<0){
 thrownewERR_ZIP_INVALID_ARCHIVE('no end of central directory record found');
@@ -474,7 +524,10 @@ function findArchiveEnd(buffer, base = 0) {
 if(prefix<0){
 thrownewERR_ZIP_INVALID_ARCHIVE('central directory does not fit inside the archive');
 }
-if(totalRecords*46>centralDirectorySize){
+if(
+(totalRecords===0&&centralDirectorySize!==0)||
+totalRecords*46>centralDirectorySize
+){
 thrownewERR_ZIP_INVALID_ARCHIVE(
 'central directory record count is inconsistent with its size');
 }

test/parallel/test-zlib-zip-hardening.js

@@ -3,8 +3,10 @@
 require('../common');
 
 constassert=require('node:assert');
+constfs=require('node:fs');
 constzlib=require('node:zlib');
 const{ test }=require('node:test');
+consttmpdir=require('../common/tmpdir');
 
 asyncfunctionbuildArchive(entries,comment){
 constchunks=[];
@@ -47,13 +49,71 @@ test('an EOCD-looking signature inside a trailing comment is not mistaken for th
 // before it reaches the genuine EOCD signature; embedding 4 bytes that
 // look like one partway through must not be mistaken for the real record.
 constfakeSignature=String.fromCharCode(0x50,0x4b,0x05,0x06);
-constarchive=awaitbuildArchive([entry],`before ${fakeSignature} after`);
+constarchive=awaitbuildArchive(
+[entry],`before ${fakeSignature} this is not a valid EOCD record after`);
 
 constread=[...zlib.ZipEntry.read(archive)];
 assert.strictEqual(read.length,1);
 assert.strictEqual(read[0].name,'f.txt');
 });
 
+test('multiple plausible EOCD records describing different archives are rejected',async()=>{
+constfirst=awaitbuildArchive([
+awaitzlib.ZipEntry.create('install.sh',Buffer.from('malicious'),{method: 'store'}),
+]);
+constsecond=awaitbuildArchive([
+awaitzlib.ZipEntry.create('install.sh',Buffer.from('benign'),{method: 'store'}),
+]);
+constarchive=Buffer.concat([first,second,Buffer.from([0])]);
+constfirstEocd=first.length-22;
+
+// Make the first EOCD exact-to-EOF by treating the second archive and its
+// padding as a comment. The second EOCD remains a plausible archive end for
+// readers which tolerate trailing padding and select the rightmost record.
+archive.writeUInt16LE(archive.length-firstEocd-22,firstEocd+20);
+
+constexpected={
+code: 'ERR_ZIP_INVALID_ARCHIVE',
+message: /ambiguousendofcentraldirectory/,
+};
+assert.throws(()=>[...zlib.ZipEntry.read(archive)],expected);
+assert.throws(()=>newzlib.ZipBuffer(archive),expected);
+
+tmpdir.refresh();
+constfile=tmpdir.resolve('ambiguous.zip');
+fs.writeFileSync(file,archive);
+awaitassert.rejects(zlib.ZipFile.open(file),expected);
+assert.throws(()=>zlib.ZipFile.openSync(file),expected);
+});
+
+test('an exact EOCD embedded in a genuine comment is rejected as ambiguous',async()=>{
+constarchive=awaitbuildArchive([
+awaitzlib.ZipEntry.create('f.txt',Buffer.from('content'),{method: 'store'}),
+]);
+constnested=Buffer.concat([archive,buildEocd()]);
+nested.writeUInt16LE(22,archive.length-2);
+
+assert.throws(()=>[...zlib.ZipEntry.read(nested)],{
+code: 'ERR_ZIP_INVALID_ARCHIVE',
+message: /ambiguousendofcentraldirectory/,
+});
+});
+
+test('multiple padded EOCD records are rejected as ambiguous',async()=>{
+constfirst=awaitbuildArchive([
+awaitzlib.ZipEntry.create('a.txt',Buffer.from('first'),{method: 'store'}),
+]);
+constsecond=awaitbuildArchive([
+awaitzlib.ZipEntry.create('b.txt',Buffer.from('second'),{method: 'store'}),
+]);
+constarchive=Buffer.concat([first,second,Buffer.from('\0\0')]);
+
+assert.throws(()=>newzlib.ZipBuffer(archive),{
+code: 'ERR_ZIP_INVALID_ARCHIVE',
+message: /ambiguousendofcentraldirectory/,
+});
+});
+
 test('a declared-size mismatch is rejected as corrupt',async()=>{
 constentry=awaitzlib.ZipEntry.create('f.txt',Buffer.from('hello world'),{method: 'store'});
 constarchive=awaitbuildArchive([entry]);
@@ -212,7 +272,7 @@ test('every possible truncation of an archive is rejected, deterministically', a
 
 test('trailing padding after the EOCD is tolerated',async()=>{
 // Some streaming writers pad their output to a block size; CPython
-// tolerates trailing newlines/NULs and so does the pass-2 EOCD scan.
+// tolerates trailing newlines/NULs and so does the EOCD scan.
 constarchive=awaitbuildArchive(
 [awaitzlib.ZipEntry.create('f.txt',Buffer.from('hi'),{method: 'store'})]);
 constpadded=Buffer.concat([archive,Buffer.from('\r\n\0\0\0')]);
@@ -221,6 +281,15 @@ test('trailing padding after the EOCD is tolerated', async () => {
 assert.strictEqual((awaitentry.content()).toString(),'hi');
 });
 
+test('a maximum-length comment followed by block padding is tolerated',async()=>{
+constcomment='x'.repeat(0xffff);
+constarchive=awaitbuildArchive([],comment);
+constpadded=Buffer.concat([archive,Buffer.alloc(4096)]);
+constzip=newzlib.ZipBuffer(padded);
+
+assert.strictEqual(zip.comment,comment);
+});
+
 test('junk appended past a declared comment is tolerated and the comment preserved',async()=>{
 constarchive=awaitbuildArchive(
 [awaitzlib.ZipEntry.create('f.txt',Buffer.from('hi'),{method: 'store'})],
@@ -262,6 +331,10 @@ test('a record count inconsistent with the directory size is rejected', () => {
 constarchive=Buffer.concat([Buffer.alloc(46),eocd]);
 assert.throws(()=>[...zlib.ZipEntry.read(archive)],
 {code: 'ERR_ZIP_INVALID_ARCHIVE',message: /inconsistent/});
+
+constzeroRecords=Buffer.concat([Buffer.alloc(46),buildEocd({cdSize: 46})]);
+assert.throws(()=>[...zlib.ZipEntry.read(zeroRecords)],
+{code: 'ERR_ZIP_INVALID_ARCHIVE',message: /inconsistent/});
 });
 
 test('a corrupted or overrunning central directory header is rejected',async()=>{

lib/internal/zip/headers.js

@@ -27,6 +27,7 @@ const {
 MADE_BY_UNIX,
 SENTINEL16,
 SENTINEL32,
+TAIL_LENGTH,
 ZIP64_EOCD_MAX_LENGTH,
 S_IFLNK,
 S_IFMT,
@@ -310,6 +311,51 @@ class LocalFileHeader {
 }
 }
 
+// Returns whether an EOCD-looking record could describe an archive this
+// implementation supports. This is deliberately only a cheap preflight: the
+// selected record still receives the complete Zip64 and central-directory
+// validation below.
+functionisPlausibleArchiveEnd(buffer,eocdPos,scanStart){
+constdiskNumber=buffer.readUInt16LE(eocdPos+4);
+constcentralDirectoryDiskNumber=buffer.readUInt16LE(eocdPos+6);
+constdiskRecords=buffer.readUInt16LE(eocdPos+8);
+consttotalRecords=buffer.readUInt16LE(eocdPos+10);
+constcentralDirectorySize=buffer.readUInt32LE(eocdPos+12);
+constcentralDirectoryOffset=buffer.readUInt32LE(eocdPos+16);
+constneedsZip64=
+diskNumber===SENTINEL16||
+centralDirectoryDiskNumber===SENTINEL16||
+diskRecords===SENTINEL16||
+totalRecords===SENTINEL16||
+centralDirectorySize===SENTINEL32||
+centralDirectoryOffset===SENTINEL32;
+
+constlocatorPos=eocdPos-20;
+consthasZip64Locator=locatorPos>=0&&
+buffer.readUInt32LE(locatorPos)===SIG_ZIP64_EOCD_LOCATOR;
+if(needsZip64)returnhasZip64Locator;
+// A Zip64 end record may accompany authoritative, non-sentinel classic
+// fields. Its central directory does not immediately precede this EOCD.
+if(hasZip64Locator)returntrue;
+if(
+diskNumber!==0||
+centralDirectoryDiskNumber!==0||
+diskRecords!==totalRecords||
+totalRecords*46>centralDirectorySize
+){
+returnfalse;
+}
+
+constcentralDirectoryPos=eocdPos-centralDirectorySize;
+if(centralDirectoryPos<scanStart){
+// Use the same logical tail window for memory- and file-backed archives.
+// The directory is not available for this cheap preflight in either case.
+returntrue;
+}
+if(totalRecords===0)returncentralDirectorySize===0;
+returnbuffer.readUInt32LE(centralDirectoryPos)===SIG_CENTRAL_FILE_HEADER;
+}
+
 /**
  * Locates and validates the end-of-archive structures (EOCD, and the Zip64
  * EOCD locator/record when present) in `buffer`. `base` is the absolute
@@ -334,26 +380,30 @@ function findArchiveEnd(buffer, base = 0) {
 if(buffer.length<22){
 thrownewERR_ZIP_INVALID_ARCHIVE('no end of central directory record found');
 }
-constmin=MathMax(0,buffer.length-(22+SENTINEL16));
+// Use the same tail-sized search window for full buffers and ZipFile's tail
+// reads. Besides keeping candidate selection consistent, the extra tail
+// slack permits a maximum-length comment followed by modest writer padding.
+constmin=MathMax(0,buffer.length-TAIL_LENGTH);
 leteocdPos=-1;
-// Pass 1: the comment must reach exactly to the end of the buffer (this
-// rejects a stray EOCD-looking signature inside an earlier comment).
+letfallbackPos=-1;
+letexactFallbackPos=-1;
 for(letpos=buffer.length-22;pos>=min;pos--){
 if(buffer.readUInt32LE(pos)!==SIG_EOCD)continue;
-if(pos+22+buffer.readUInt16LE(pos+20)!==buffer.length)continue;
+constend=pos+22+buffer.readUInt16LE(pos+20);
+if(end>buffer.length)continue;
+if(fallbackPos<0)fallbackPos=pos;
+if(end===buffer.length&&exactFallbackPos<0)exactFallbackPos=pos;
+if(!isPlausibleArchiveEnd(buffer,pos,min))continue;
+if(eocdPos>=0){
+thrownewERR_ZIP_INVALID_ARCHIVE(
+'ambiguous end of central directory records');
+}
 eocdPos=pos;
-break;
 }
+// Preserve the targeted validation errors for a sole malformed or
+// unsupported candidate. Plausible candidates always take precedence.
 if(eocdPos<0){
-// Pass 2: tolerate trailing padding after the EOCD (some streaming
-// writers pad their output to a fixed block size); take the last
-// candidate found.
-for(letpos=buffer.length-22;pos>=min;pos--){
-if(buffer.readUInt32LE(pos)!==SIG_EOCD)continue;
-if(pos+22+buffer.readUInt16LE(pos+20)>buffer.length)continue;
-eocdPos=pos;
-break;
-}
+eocdPos=exactFallbackPos>=0 ? exactFallbackPos : fallbackPos;
 }
 if(eocdPos<0){
 thrownewERR_ZIP_INVALID_ARCHIVE('no end of central directory record found');
@@ -474,7 +524,10 @@ function findArchiveEnd(buffer, base = 0) {
 if(prefix<0){
 thrownewERR_ZIP_INVALID_ARCHIVE('central directory does not fit inside the archive');
 }
-if(totalRecords*46>centralDirectorySize){
+if(
+(totalRecords===0&&centralDirectorySize!==0)||
+totalRecords*46>centralDirectorySize
+){
 thrownewERR_ZIP_INVALID_ARCHIVE(
 'central directory record count is inconsistent with its size');
 }

test/parallel/test-zlib-zip-hardening.js

@@ -3,8 +3,10 @@
 require('../common');
 
 constassert=require('node:assert');
+constfs=require('node:fs');
 constzlib=require('node:zlib');
 const{ test }=require('node:test');
+consttmpdir=require('../common/tmpdir');
 
 asyncfunctionbuildArchive(entries,comment){
 constchunks=[];
@@ -47,13 +49,71 @@ test('an EOCD-looking signature inside a trailing comment is not mistaken for th
 // before it reaches the genuine EOCD signature; embedding 4 bytes that
 // look like one partway through must not be mistaken for the real record.
 constfakeSignature=String.fromCharCode(0x50,0x4b,0x05,0x06);
-constarchive=awaitbuildArchive([entry],`before ${fakeSignature} after`);
+constarchive=awaitbuildArchive(
+[entry],`before ${fakeSignature} this is not a valid EOCD record after`);
 
 constread=[...zlib.ZipEntry.read(archive)];
 assert.strictEqual(read.length,1);
 assert.strictEqual(read[0].name,'f.txt');
 });
 
+test('multiple plausible EOCD records describing different archives are rejected',async()=>{
+constfirst=awaitbuildArchive([
+awaitzlib.ZipEntry.create('install.sh',Buffer.from('malicious'),{method: 'store'}),
+]);
+constsecond=awaitbuildArchive([
+awaitzlib.ZipEntry.create('install.sh',Buffer.from('benign'),{method: 'store'}),
+]);
+constarchive=Buffer.concat([first,second,Buffer.from([0])]);
+constfirstEocd=first.length-22;
+
+// Make the first EOCD exact-to-EOF by treating the second archive and its
+// padding as a comment. The second EOCD remains a plausible archive end for
+// readers which tolerate trailing padding and select the rightmost record.
+archive.writeUInt16LE(archive.length-firstEocd-22,firstEocd+20);
+
+constexpected={
+code: 'ERR_ZIP_INVALID_ARCHIVE',
+message: /ambiguousendofcentraldirectory/,
+};
+assert.throws(()=>[...zlib.ZipEntry.read(archive)],expected);
+assert.throws(()=>newzlib.ZipBuffer(archive),expected);
+
+tmpdir.refresh();
+constfile=tmpdir.resolve('ambiguous.zip');
+fs.writeFileSync(file,archive);
+awaitassert.rejects(zlib.ZipFile.open(file),expected);
+assert.throws(()=>zlib.ZipFile.openSync(file),expected);
+});
+
+test('an exact EOCD embedded in a genuine comment is rejected as ambiguous',async()=>{
+constarchive=awaitbuildArchive([
+awaitzlib.ZipEntry.create('f.txt',Buffer.from('content'),{method: 'store'}),
+]);
+constnested=Buffer.concat([archive,buildEocd()]);
+nested.writeUInt16LE(22,archive.length-2);
+
+assert.throws(()=>[...zlib.ZipEntry.read(nested)],{
+code: 'ERR_ZIP_INVALID_ARCHIVE',
+message: /ambiguousendofcentraldirectory/,
+});
+});
+
+test('multiple padded EOCD records are rejected as ambiguous',async()=>{
+constfirst=awaitbuildArchive([
+awaitzlib.ZipEntry.create('a.txt',Buffer.from('first'),{method: 'store'}),
+]);
+constsecond=awaitbuildArchive([
+awaitzlib.ZipEntry.create('b.txt',Buffer.from('second'),{method: 'store'}),
+]);
+constarchive=Buffer.concat([first,second,Buffer.from('\0\0')]);
+
+assert.throws(()=>newzlib.ZipBuffer(archive),{
+code: 'ERR_ZIP_INVALID_ARCHIVE',
+message: /ambiguousendofcentraldirectory/,
+});
+});
+
 test('a declared-size mismatch is rejected as corrupt',async()=>{
 constentry=awaitzlib.ZipEntry.create('f.txt',Buffer.from('hello world'),{method: 'store'});
 constarchive=awaitbuildArchive([entry]);
@@ -212,7 +272,7 @@ test('every possible truncation of an archive is rejected, deterministically', a
 
 test('trailing padding after the EOCD is tolerated',async()=>{
 // Some streaming writers pad their output to a block size; CPython
-// tolerates trailing newlines/NULs and so does the pass-2 EOCD scan.
+// tolerates trailing newlines/NULs and so does the EOCD scan.
 constarchive=awaitbuildArchive(
 [awaitzlib.ZipEntry.create('f.txt',Buffer.from('hi'),{method: 'store'})]);
 constpadded=Buffer.concat([archive,Buffer.from('\r\n\0\0\0')]);
@@ -221,6 +281,15 @@ test('trailing padding after the EOCD is tolerated', async () => {
 assert.strictEqual((awaitentry.content()).toString(),'hi');
 });
 
+test('a maximum-length comment followed by block padding is tolerated',async()=>{
+constcomment='x'.repeat(0xffff);
+constarchive=awaitbuildArchive([],comment);
+constpadded=Buffer.concat([archive,Buffer.alloc(4096)]);
+constzip=newzlib.ZipBuffer(padded);
+
+assert.strictEqual(zip.comment,comment);
+});
+
 test('junk appended past a declared comment is tolerated and the comment preserved',async()=>{
 constarchive=awaitbuildArchive(
 [awaitzlib.ZipEntry.create('f.txt',Buffer.from('hi'),{method: 'store'})],
@@ -262,6 +331,10 @@ test('a record count inconsistent with the directory size is rejected', () => {
 constarchive=Buffer.concat([Buffer.alloc(46),eocd]);
 assert.throws(()=>[...zlib.ZipEntry.read(archive)],
 {code: 'ERR_ZIP_INVALID_ARCHIVE',message: /inconsistent/});
+
+constzeroRecords=Buffer.concat([Buffer.alloc(46),buildEocd({cdSize: 46})]);
+assert.throws(()=>[...zlib.ZipEntry.read(zeroRecords)],
+{code: 'ERR_ZIP_INVALID_ARCHIVE',message: /inconsistent/});
 });
 
 test('a corrupted or overrunning central directory header is rejected',async()=>{

lib/internal/zip/headers.js

@@ -27,6 +27,7 @@ const {
 MADE_BY_UNIX,
 SENTINEL16,
 SENTINEL32,
+TAIL_LENGTH,
 ZIP64_EOCD_MAX_LENGTH,
 S_IFLNK,
 S_IFMT,
@@ -310,6 +311,51 @@ class LocalFileHeader {
 }
 }
 
+// Returns whether an EOCD-looking record could describe an archive this
+// implementation supports. This is deliberately only a cheap preflight: the
+// selected record still receives the complete Zip64 and central-directory
+// validation below.
+functionisPlausibleArchiveEnd(buffer,eocdPos,scanStart){
+constdiskNumber=buffer.readUInt16LE(eocdPos+4);
+constcentralDirectoryDiskNumber=buffer.readUInt16LE(eocdPos+6);
+constdiskRecords=buffer.readUInt16LE(eocdPos+8);
+consttotalRecords=buffer.readUInt16LE(eocdPos+10);
+constcentralDirectorySize=buffer.readUInt32LE(eocdPos+12);
+constcentralDirectoryOffset=buffer.readUInt32LE(eocdPos+16);
+constneedsZip64=
+diskNumber===SENTINEL16||
+centralDirectoryDiskNumber===SENTINEL16||
+diskRecords===SENTINEL16||
+totalRecords===SENTINEL16||
+centralDirectorySize===SENTINEL32||
+centralDirectoryOffset===SENTINEL32;
+
+constlocatorPos=eocdPos-20;
+consthasZip64Locator=locatorPos>=0&&
+buffer.readUInt32LE(locatorPos)===SIG_ZIP64_EOCD_LOCATOR;
+if(needsZip64)returnhasZip64Locator;
+// A Zip64 end record may accompany authoritative, non-sentinel classic
+// fields. Its central directory does not immediately precede this EOCD.
+if(hasZip64Locator)returntrue;
+if(
+diskNumber!==0||
+centralDirectoryDiskNumber!==0||
+diskRecords!==totalRecords||
+totalRecords*46>centralDirectorySize
+){
+returnfalse;
+}
+
+constcentralDirectoryPos=eocdPos-centralDirectorySize;
+if(centralDirectoryPos<scanStart){
+// Use the same logical tail window for memory- and file-backed archives.
+// The directory is not available for this cheap preflight in either case.
+returntrue;
+}
+if(totalRecords===0)returncentralDirectorySize===0;
+returnbuffer.readUInt32LE(centralDirectoryPos)===SIG_CENTRAL_FILE_HEADER;
+}
+
 /**
  * Locates and validates the end-of-archive structures (EOCD, and the Zip64
  * EOCD locator/record when present) in `buffer`. `base` is the absolute
@@ -334,26 +380,30 @@ function findArchiveEnd(buffer, base = 0) {
 if(buffer.length<22){
 thrownewERR_ZIP_INVALID_ARCHIVE('no end of central directory record found');
 }
-constmin=MathMax(0,buffer.length-(22+SENTINEL16));
+// Use the same tail-sized search window for full buffers and ZipFile's tail
+// reads. Besides keeping candidate selection consistent, the extra tail
+// slack permits a maximum-length comment followed by modest writer padding.
+constmin=MathMax(0,buffer.length-TAIL_LENGTH);
 leteocdPos=-1;
-// Pass 1: the comment must reach exactly to the end of the buffer (this
-// rejects a stray EOCD-looking signature inside an earlier comment).
+letfallbackPos=-1;
+letexactFallbackPos=-1;
 for(letpos=buffer.length-22;pos>=min;pos--){
 if(buffer.readUInt32LE(pos)!==SIG_EOCD)continue;
-if(pos+22+buffer.readUInt16LE(pos+20)!==buffer.length)continue;
+constend=pos+22+buffer.readUInt16LE(pos+20);
+if(end>buffer.length)continue;
+if(fallbackPos<0)fallbackPos=pos;
+if(end===buffer.length&&exactFallbackPos<0)exactFallbackPos=pos;
+if(!isPlausibleArchiveEnd(buffer,pos,min))continue;
+if(eocdPos>=0){
+thrownewERR_ZIP_INVALID_ARCHIVE(
+'ambiguous end of central directory records');
+}
 eocdPos=pos;
-break;
 }
+// Preserve the targeted validation errors for a sole malformed or
+// unsupported candidate. Plausible candidates always take precedence.
 if(eocdPos<0){
-// Pass 2: tolerate trailing padding after the EOCD (some streaming
-// writers pad their output to a fixed block size); take the last
-// candidate found.
-for(letpos=buffer.length-22;pos>=min;pos--){
-if(buffer.readUInt32LE(pos)!==SIG_EOCD)continue;
-if(pos+22+buffer.readUInt16LE(pos+20)>buffer.length)continue;
-eocdPos=pos;
-break;
-}
+eocdPos=exactFallbackPos>=0 ? exactFallbackPos : fallbackPos;
 }
 if(eocdPos<0){
 thrownewERR_ZIP_INVALID_ARCHIVE('no end of central directory record found');
@@ -474,7 +524,10 @@ function findArchiveEnd(buffer, base = 0) {
 if(prefix<0){
 thrownewERR_ZIP_INVALID_ARCHIVE('central directory does not fit inside the archive');
 }
-if(totalRecords*46>centralDirectorySize){
+if(
+(totalRecords===0&&centralDirectorySize!==0)||
+totalRecords*46>centralDirectorySize
+){
 thrownewERR_ZIP_INVALID_ARCHIVE(
 'central directory record count is inconsistent with its size');
 }

test/parallel/test-zlib-zip-hardening.js

@@ -3,8 +3,10 @@
 require('../common');
 
 constassert=require('node:assert');
+constfs=require('node:fs');
 constzlib=require('node:zlib');
 const{ test }=require('node:test');
+consttmpdir=require('../common/tmpdir');
 
 asyncfunctionbuildArchive(entries,comment){
 constchunks=[];
@@ -47,13 +49,71 @@ test('an EOCD-looking signature inside a trailing comment is not mistaken for th
 // before it reaches the genuine EOCD signature; embedding 4 bytes that
 // look like one partway through must not be mistaken for the real record.
 constfakeSignature=String.fromCharCode(0x50,0x4b,0x05,0x06);
-constarchive=awaitbuildArchive([entry],`before ${fakeSignature} after`);
+constarchive=awaitbuildArchive(
+[entry],`before ${fakeSignature} this is not a valid EOCD record after`);
 
 constread=[...zlib.ZipEntry.read(archive)];
 assert.strictEqual(read.length,1);
 assert.strictEqual(read[0].name,'f.txt');
 });
 
+test('multiple plausible EOCD records describing different archives are rejected',async()=>{
+constfirst=awaitbuildArchive([
+awaitzlib.ZipEntry.create('install.sh',Buffer.from('malicious'),{method: 'store'}),
+]);
+constsecond=awaitbuildArchive([
+awaitzlib.ZipEntry.create('install.sh',Buffer.from('benign'),{method: 'store'}),
+]);
+constarchive=Buffer.concat([first,second,Buffer.from([0])]);
+constfirstEocd=first.length-22;
+
+// Make the first EOCD exact-to-EOF by treating the second archive and its
+// padding as a comment. The second EOCD remains a plausible archive end for
+// readers which tolerate trailing padding and select the rightmost record.
+archive.writeUInt16LE(archive.length-firstEocd-22,firstEocd+20);
+
+constexpected={
+code: 'ERR_ZIP_INVALID_ARCHIVE',
+message: /ambiguousendofcentraldirectory/,
+};
+assert.throws(()=>[...zlib.ZipEntry.read(archive)],expected);
+assert.throws(()=>newzlib.ZipBuffer(archive),expected);
+
+tmpdir.refresh();
+constfile=tmpdir.resolve('ambiguous.zip');
+fs.writeFileSync(file,archive);
+awaitassert.rejects(zlib.ZipFile.open(file),expected);
+assert.throws(()=>zlib.ZipFile.openSync(file),expected);
+});
+
+test('an exact EOCD embedded in a genuine comment is rejected as ambiguous',async()=>{
+constarchive=awaitbuildArchive([
+awaitzlib.ZipEntry.create('f.txt',Buffer.from('content'),{method: 'store'}),
+]);
+constnested=Buffer.concat([archive,buildEocd()]);
+nested.writeUInt16LE(22,archive.length-2);
+
+assert.throws(()=>[...zlib.ZipEntry.read(nested)],{
+code: 'ERR_ZIP_INVALID_ARCHIVE',
+message: /ambiguousendofcentraldirectory/,
+});
+});
+
+test('multiple padded EOCD records are rejected as ambiguous',async()=>{
+constfirst=awaitbuildArchive([
+awaitzlib.ZipEntry.create('a.txt',Buffer.from('first'),{method: 'store'}),
+]);
+constsecond=awaitbuildArchive([
+awaitzlib.ZipEntry.create('b.txt',Buffer.from('second'),{method: 'store'}),
+]);
+constarchive=Buffer.concat([first,second,Buffer.from('\0\0')]);
+
+assert.throws(()=>newzlib.ZipBuffer(archive),{
+code: 'ERR_ZIP_INVALID_ARCHIVE',
+message: /ambiguousendofcentraldirectory/,
+});
+});
+
 test('a declared-size mismatch is rejected as corrupt',async()=>{
 constentry=awaitzlib.ZipEntry.create('f.txt',Buffer.from('hello world'),{method: 'store'});
 constarchive=awaitbuildArchive([entry]);
@@ -212,7 +272,7 @@ test('every possible truncation of an archive is rejected, deterministically', a
 
 test('trailing padding after the EOCD is tolerated',async()=>{
 // Some streaming writers pad their output to a block size; CPython
-// tolerates trailing newlines/NULs and so does the pass-2 EOCD scan.
+// tolerates trailing newlines/NULs and so does the EOCD scan.
 constarchive=awaitbuildArchive(
 [awaitzlib.ZipEntry.create('f.txt',Buffer.from('hi'),{method: 'store'})]);
 constpadded=Buffer.concat([archive,Buffer.from('\r\n\0\0\0')]);
@@ -221,6 +281,15 @@ test('trailing padding after the EOCD is tolerated', async () => {
 assert.strictEqual((awaitentry.content()).toString(),'hi');
 });
 
+test('a maximum-length comment followed by block padding is tolerated',async()=>{
+constcomment='x'.repeat(0xffff);
+constarchive=awaitbuildArchive([],comment);
+constpadded=Buffer.concat([archive,Buffer.alloc(4096)]);
+constzip=newzlib.ZipBuffer(padded);
+
+assert.strictEqual(zip.comment,comment);
+});
+
 test('junk appended past a declared comment is tolerated and the comment preserved',async()=>{
 constarchive=awaitbuildArchive(
 [awaitzlib.ZipEntry.create('f.txt',Buffer.from('hi'),{method: 'store'})],
@@ -262,6 +331,10 @@ test('a record count inconsistent with the directory size is rejected', () => {
 constarchive=Buffer.concat([Buffer.alloc(46),eocd]);
 assert.throws(()=>[...zlib.ZipEntry.read(archive)],
 {code: 'ERR_ZIP_INVALID_ARCHIVE',message: /inconsistent/});
+
+constzeroRecords=Buffer.concat([Buffer.alloc(46),buildEocd({cdSize: 46})]);
+assert.throws(()=>[...zlib.ZipEntry.read(zeroRecords)],
+{code: 'ERR_ZIP_INVALID_ARCHIVE',message: /inconsistent/});
 });
 
 test('a corrupted or overrunning central directory header is rejected',async()=>{

lib/internal/zip/headers.js

@@ -27,6 +27,7 @@ const {
 MADE_BY_UNIX,
 SENTINEL16,
 SENTINEL32,
+TAIL_LENGTH,
 ZIP64_EOCD_MAX_LENGTH,
 S_IFLNK,
 S_IFMT,
@@ -310,6 +311,51 @@ class LocalFileHeader {
 }
 }
 
+// Returns whether an EOCD-looking record could describe an archive this
+// implementation supports. This is deliberately only a cheap preflight: the
+// selected record still receives the complete Zip64 and central-directory
+// validation below.
+functionisPlausibleArchiveEnd(buffer,eocdPos,scanStart){
+constdiskNumber=buffer.readUInt16LE(eocdPos+4);
+constcentralDirectoryDiskNumber=buffer.readUInt16LE(eocdPos+6);
+constdiskRecords=buffer.readUInt16LE(eocdPos+8);
+consttotalRecords=buffer.readUInt16LE(eocdPos+10);
+constcentralDirectorySize=buffer.readUInt32LE(eocdPos+12);
+constcentralDirectoryOffset=buffer.readUInt32LE(eocdPos+16);
+constneedsZip64=
+diskNumber===SENTINEL16||
+centralDirectoryDiskNumber===SENTINEL16||
+diskRecords===SENTINEL16||
+totalRecords===SENTINEL16||
+centralDirectorySize===SENTINEL32||
+centralDirectoryOffset===SENTINEL32;
+
+constlocatorPos=eocdPos-20;
+consthasZip64Locator=locatorPos>=0&&
+buffer.readUInt32LE(locatorPos)===SIG_ZIP64_EOCD_LOCATOR;
+if(needsZip64)returnhasZip64Locator;
+// A Zip64 end record may accompany authoritative, non-sentinel classic
+// fields. Its central directory does not immediately precede this EOCD.
+if(hasZip64Locator)returntrue;
+if(
+diskNumber!==0||
+centralDirectoryDiskNumber!==0||
+diskRecords!==totalRecords||
+totalRecords*46>centralDirectorySize
+){
+returnfalse;
+}
+
+constcentralDirectoryPos=eocdPos-centralDirectorySize;
+if(centralDirectoryPos<scanStart){
+// Use the same logical tail window for memory- and file-backed archives.
+// The directory is not available for this cheap preflight in either case.
+returntrue;
+}
+if(totalRecords===0)returncentralDirectorySize===0;
+returnbuffer.readUInt32LE(centralDirectoryPos)===SIG_CENTRAL_FILE_HEADER;
+}
+
 /**
  * Locates and validates the end-of-archive structures (EOCD, and the Zip64
  * EOCD locator/record when present) in `buffer`. `base` is the absolute
@@ -334,26 +380,30 @@ function findArchiveEnd(buffer, base = 0) {
 if(buffer.length<22){
 thrownewERR_ZIP_INVALID_ARCHIVE('no end of central directory record found');
 }
-constmin=MathMax(0,buffer.length-(22+SENTINEL16));
+// Use the same tail-sized search window for full buffers and ZipFile's tail
+// reads. Besides keeping candidate selection consistent, the extra tail
+// slack permits a maximum-length comment followed by modest writer padding.
+constmin=MathMax(0,buffer.length-TAIL_LENGTH);
 leteocdPos=-1;
-// Pass 1: the comment must reach exactly to the end of the buffer (this
-// rejects a stray EOCD-looking signature inside an earlier comment).
+letfallbackPos=-1;
+letexactFallbackPos=-1;
 for(letpos=buffer.length-22;pos>=min;pos--){
 if(buffer.readUInt32LE(pos)!==SIG_EOCD)continue;
-if(pos+22+buffer.readUInt16LE(pos+20)!==buffer.length)continue;
+constend=pos+22+buffer.readUInt16LE(pos+20);
+if(end>buffer.length)continue;
+if(fallbackPos<0)fallbackPos=pos;
+if(end===buffer.length&&exactFallbackPos<0)exactFallbackPos=pos;
+if(!isPlausibleArchiveEnd(buffer,pos,min))continue;
+if(eocdPos>=0){
+thrownewERR_ZIP_INVALID_ARCHIVE(
+'ambiguous end of central directory records');
+}
 eocdPos=pos;
-break;
 }
+// Preserve the targeted validation errors for a sole malformed or
+// unsupported candidate. Plausible candidates always take precedence.
 if(eocdPos<0){
-// Pass 2: tolerate trailing padding after the EOCD (some streaming
-// writers pad their output to a fixed block size); take the last
-// candidate found.
-for(letpos=buffer.length-22;pos>=min;pos--){
-if(buffer.readUInt32LE(pos)!==SIG_EOCD)continue;
-if(pos+22+buffer.readUInt16LE(pos+20)>buffer.length)continue;
-eocdPos=pos;
-break;
-}
+eocdPos=exactFallbackPos>=0 ? exactFallbackPos : fallbackPos;
 }
 if(eocdPos<0){
 thrownewERR_ZIP_INVALID_ARCHIVE('no end of central directory record found');
@@ -474,7 +524,10 @@ function findArchiveEnd(buffer, base = 0) {
 if(prefix<0){
 thrownewERR_ZIP_INVALID_ARCHIVE('central directory does not fit inside the archive');
 }
-if(totalRecords*46>centralDirectorySize){
+if(
+(totalRecords===0&&centralDirectorySize!==0)||
+totalRecords*46>centralDirectorySize
+){
 thrownewERR_ZIP_INVALID_ARCHIVE(
 'central directory record count is inconsistent with its size');
 }

test/parallel/test-zlib-zip-hardening.js

@@ -3,8 +3,10 @@
 require('../common');
 
 constassert=require('node:assert');
+constfs=require('node:fs');
 constzlib=require('node:zlib');
 const{ test }=require('node:test');
+consttmpdir=require('../common/tmpdir');
 
 asyncfunctionbuildArchive(entries,comment){
 constchunks=[];
@@ -47,13 +49,71 @@ test('an EOCD-looking signature inside a trailing comment is not mistaken for th
 // before it reaches the genuine EOCD signature; embedding 4 bytes that
 // look like one partway through must not be mistaken for the real record.
 constfakeSignature=String.fromCharCode(0x50,0x4b,0x05,0x06);
-constarchive=awaitbuildArchive([entry],`before ${fakeSignature} after`);
+constarchive=awaitbuildArchive(
+[entry],`before ${fakeSignature} this is not a valid EOCD record after`);
 
 constread=[...zlib.ZipEntry.read(archive)];
 assert.strictEqual(read.length,1);
 assert.strictEqual(read[0].name,'f.txt');
 });
 
+test('multiple plausible EOCD records describing different archives are rejected',async()=>{
+constfirst=awaitbuildArchive([
+awaitzlib.ZipEntry.create('install.sh',Buffer.from('malicious'),{method: 'store'}),
+]);
+constsecond=awaitbuildArchive([
+awaitzlib.ZipEntry.create('install.sh',Buffer.from('benign'),{method: 'store'}),
+]);
+constarchive=Buffer.concat([first,second,Buffer.from([0])]);
+constfirstEocd=first.length-22;
+
+// Make the first EOCD exact-to-EOF by treating the second archive and its
+// padding as a comment. The second EOCD remains a plausible archive end for
+// readers which tolerate trailing padding and select the rightmost record.
+archive.writeUInt16LE(archive.length-firstEocd-22,firstEocd+20);
+
+constexpected={
+code: 'ERR_ZIP_INVALID_ARCHIVE',
+message: /ambiguousendofcentraldirectory/,
+};
+assert.throws(()=>[...zlib.ZipEntry.read(archive)],expected);
+assert.throws(()=>newzlib.ZipBuffer(archive),expected);
+
+tmpdir.refresh();
+constfile=tmpdir.resolve('ambiguous.zip');
+fs.writeFileSync(file,archive);
+awaitassert.rejects(zlib.ZipFile.open(file),expected);
+assert.throws(()=>zlib.ZipFile.openSync(file),expected);
+});
+
+test('an exact EOCD embedded in a genuine comment is rejected as ambiguous',async()=>{
+constarchive=awaitbuildArchive([
+awaitzlib.ZipEntry.create('f.txt',Buffer.from('content'),{method: 'store'}),
+]);
+constnested=Buffer.concat([archive,buildEocd()]);
+nested.writeUInt16LE(22,archive.length-2);
+
+assert.throws(()=>[...zlib.ZipEntry.read(nested)],{
+code: 'ERR_ZIP_INVALID_ARCHIVE',
+message: /ambiguousendofcentraldirectory/,
+});
+});
+
+test('multiple padded EOCD records are rejected as ambiguous',async()=>{
+constfirst=awaitbuildArchive([
+awaitzlib.ZipEntry.create('a.txt',Buffer.from('first'),{method: 'store'}),
+]);
+constsecond=awaitbuildArchive([
+awaitzlib.ZipEntry.create('b.txt',Buffer.from('second'),{method: 'store'}),
+]);
+constarchive=Buffer.concat([first,second,Buffer.from('\0\0')]);
+
+assert.throws(()=>newzlib.ZipBuffer(archive),{
+code: 'ERR_ZIP_INVALID_ARCHIVE',
+message: /ambiguousendofcentraldirectory/,
+});
+});
+
 test('a declared-size mismatch is rejected as corrupt',async()=>{
 constentry=awaitzlib.ZipEntry.create('f.txt',Buffer.from('hello world'),{method: 'store'});
 constarchive=awaitbuildArchive([entry]);
@@ -212,7 +272,7 @@ test('every possible truncation of an archive is rejected, deterministically', a
 
 test('trailing padding after the EOCD is tolerated',async()=>{
 // Some streaming writers pad their output to a block size; CPython
-// tolerates trailing newlines/NULs and so does the pass-2 EOCD scan.
+// tolerates trailing newlines/NULs and so does the EOCD scan.
 constarchive=awaitbuildArchive(
 [awaitzlib.ZipEntry.create('f.txt',Buffer.from('hi'),{method: 'store'})]);
 constpadded=Buffer.concat([archive,Buffer.from('\r\n\0\0\0')]);
@@ -221,6 +281,15 @@ test('trailing padding after the EOCD is tolerated', async () => {
 assert.strictEqual((awaitentry.content()).toString(),'hi');
 });
 
+test('a maximum-length comment followed by block padding is tolerated',async()=>{
+constcomment='x'.repeat(0xffff);
+constarchive=awaitbuildArchive([],comment);
+constpadded=Buffer.concat([archive,Buffer.alloc(4096)]);
+constzip=newzlib.ZipBuffer(padded);
+
+assert.strictEqual(zip.comment,comment);
+});
+
 test('junk appended past a declared comment is tolerated and the comment preserved',async()=>{
 constarchive=awaitbuildArchive(
 [awaitzlib.ZipEntry.create('f.txt',Buffer.from('hi'),{method: 'store'})],
@@ -262,6 +331,10 @@ test('a record count inconsistent with the directory size is rejected', () => {
 constarchive=Buffer.concat([Buffer.alloc(46),eocd]);
 assert.throws(()=>[...zlib.ZipEntry.read(archive)],
 {code: 'ERR_ZIP_INVALID_ARCHIVE',message: /inconsistent/});
+
+constzeroRecords=Buffer.concat([Buffer.alloc(46),buildEocd({cdSize: 46})]);
+assert.throws(()=>[...zlib.ZipEntry.read(zeroRecords)],
+{code: 'ERR_ZIP_INVALID_ARCHIVE',message: /inconsistent/});
 });
 
 test('a corrupted or overrunning central directory header is rejected',async()=>{

lib/internal/zip/headers.js

@@ -27,6 +27,7 @@ const {
 MADE_BY_UNIX,
 SENTINEL16,
 SENTINEL32,
+TAIL_LENGTH,
 ZIP64_EOCD_MAX_LENGTH,
 S_IFLNK,
 S_IFMT,
@@ -310,6 +311,51 @@ class LocalFileHeader {
 }
 }
 
+// Returns whether an EOCD-looking record could describe an archive this
+// implementation supports. This is deliberately only a cheap preflight: the
+// selected record still receives the complete Zip64 and central-directory
+// validation below.
+functionisPlausibleArchiveEnd(buffer,eocdPos,scanStart){
+constdiskNumber=buffer.readUInt16LE(eocdPos+4);
+constcentralDirectoryDiskNumber=buffer.readUInt16LE(eocdPos+6);
+constdiskRecords=buffer.readUInt16LE(eocdPos+8);
+consttotalRecords=buffer.readUInt16LE(eocdPos+10);
+constcentralDirectorySize=buffer.readUInt32LE(eocdPos+12);
+constcentralDirectoryOffset=buffer.readUInt32LE(eocdPos+16);
+constneedsZip64=
+diskNumber===SENTINEL16||
+centralDirectoryDiskNumber===SENTINEL16||
+diskRecords===SENTINEL16||
+totalRecords===SENTINEL16||
+centralDirectorySize===SENTINEL32||
+centralDirectoryOffset===SENTINEL32;
+
+constlocatorPos=eocdPos-20;
+consthasZip64Locator=locatorPos>=0&&
+buffer.readUInt32LE(locatorPos)===SIG_ZIP64_EOCD_LOCATOR;
+if(needsZip64)returnhasZip64Locator;
+// A Zip64 end record may accompany authoritative, non-sentinel classic
+// fields. Its central directory does not immediately precede this EOCD.
+if(hasZip64Locator)returntrue;
+if(
+diskNumber!==0||
+centralDirectoryDiskNumber!==0||
+diskRecords!==totalRecords||
+totalRecords*46>centralDirectorySize
+){
+returnfalse;
+}
+
+constcentralDirectoryPos=eocdPos-centralDirectorySize;
+if(centralDirectoryPos<scanStart){
+// Use the same logical tail window for memory- and file-backed archives.
+// The directory is not available for this cheap preflight in either case.
+returntrue;
+}
+if(totalRecords===0)returncentralDirectorySize===0;
+returnbuffer.readUInt32LE(centralDirectoryPos)===SIG_CENTRAL_FILE_HEADER;
+}
+
 /**
  * Locates and validates the end-of-archive structures (EOCD, and the Zip64
  * EOCD locator/record when present) in `buffer`. `base` is the absolute
@@ -334,26 +380,30 @@ function findArchiveEnd(buffer, base = 0) {
 if(buffer.length<22){
 thrownewERR_ZIP_INVALID_ARCHIVE('no end of central directory record found');
 }
-constmin=MathMax(0,buffer.length-(22+SENTINEL16));
+// Use the same tail-sized search window for full buffers and ZipFile's tail
+// reads. Besides keeping candidate selection consistent, the extra tail
+// slack permits a maximum-length comment followed by modest writer padding.
+constmin=MathMax(0,buffer.length-TAIL_LENGTH);
 leteocdPos=-1;
-// Pass 1: the comment must reach exactly to the end of the buffer (this
-// rejects a stray EOCD-looking signature inside an earlier comment).
+letfallbackPos=-1;
+letexactFallbackPos=-1;
 for(letpos=buffer.length-22;pos>=min;pos--){
 if(buffer.readUInt32LE(pos)!==SIG_EOCD)continue;
-if(pos+22+buffer.readUInt16LE(pos+20)!==buffer.length)continue;
+constend=pos+22+buffer.readUInt16LE(pos+20);
+if(end>buffer.length)continue;
+if(fallbackPos<0)fallbackPos=pos;
+if(end===buffer.length&&exactFallbackPos<0)exactFallbackPos=pos;
+if(!isPlausibleArchiveEnd(buffer,pos,min))continue;
+if(eocdPos>=0){
+thrownewERR_ZIP_INVALID_ARCHIVE(
+'ambiguous end of central directory records');
+}
 eocdPos=pos;
-break;
 }
+// Preserve the targeted validation errors for a sole malformed or
+// unsupported candidate. Plausible candidates always take precedence.
 if(eocdPos<0){
-// Pass 2: tolerate trailing padding after the EOCD (some streaming
-// writers pad their output to a fixed block size); take the last
-// candidate found.
-for(letpos=buffer.length-22;pos>=min;pos--){
-if(buffer.readUInt32LE(pos)!==SIG_EOCD)continue;
-if(pos+22+buffer.readUInt16LE(pos+20)>buffer.length)continue;
-eocdPos=pos;
-break;
-}
+eocdPos=exactFallbackPos>=0 ? exactFallbackPos : fallbackPos;
 }
 if(eocdPos<0){
 thrownewERR_ZIP_INVALID_ARCHIVE('no end of central directory record found');
@@ -474,7 +524,10 @@ function findArchiveEnd(buffer, base = 0) {
 if(prefix<0){
 thrownewERR_ZIP_INVALID_ARCHIVE('central directory does not fit inside the archive');
 }
-if(totalRecords*46>centralDirectorySize){
+if(
+(totalRecords===0&&centralDirectorySize!==0)||
+totalRecords*46>centralDirectorySize
+){
 thrownewERR_ZIP_INVALID_ARCHIVE(
 'central directory record count is inconsistent with its size');
 }

test/parallel/test-zlib-zip-hardening.js

@@ -3,8 +3,10 @@
 require('../common');
 
 constassert=require('node:assert');
+constfs=require('node:fs');
 constzlib=require('node:zlib');
 const{ test }=require('node:test');
+consttmpdir=require('../common/tmpdir');
 
 asyncfunctionbuildArchive(entries,comment){
 constchunks=[];
@@ -47,13 +49,71 @@ test('an EOCD-looking signature inside a trailing comment is not mistaken for th
 // before it reaches the genuine EOCD signature; embedding 4 bytes that
 // look like one partway through must not be mistaken for the real record.
 constfakeSignature=String.fromCharCode(0x50,0x4b,0x05,0x06);
-constarchive=awaitbuildArchive([entry],`before ${fakeSignature} after`);
+constarchive=awaitbuildArchive(
+[entry],`before ${fakeSignature} this is not a valid EOCD record after`);
 
 constread=[...zlib.ZipEntry.read(archive)];
 assert.strictEqual(read.length,1);
 assert.strictEqual(read[0].name,'f.txt');
 });
 
+test('multiple plausible EOCD records describing different archives are rejected',async()=>{
+constfirst=awaitbuildArchive([
+awaitzlib.ZipEntry.create('install.sh',Buffer.from('malicious'),{method: 'store'}),
+]);
+constsecond=awaitbuildArchive([
+awaitzlib.ZipEntry.create('install.sh',Buffer.from('benign'),{method: 'store'}),
+]);
+constarchive=Buffer.concat([first,second,Buffer.from([0])]);
+constfirstEocd=first.length-22;
+
+// Make the first EOCD exact-to-EOF by treating the second archive and its
+// padding as a comment. The second EOCD remains a plausible archive end for
+// readers which tolerate trailing padding and select the rightmost record.
+archive.writeUInt16LE(archive.length-firstEocd-22,firstEocd+20);
+
+constexpected={
+code: 'ERR_ZIP_INVALID_ARCHIVE',
+message: /ambiguousendofcentraldirectory/,
+};
+assert.throws(()=>[...zlib.ZipEntry.read(archive)],expected);
+assert.throws(()=>newzlib.ZipBuffer(archive),expected);
+
+tmpdir.refresh();
+constfile=tmpdir.resolve('ambiguous.zip');
+fs.writeFileSync(file,archive);
+awaitassert.rejects(zlib.ZipFile.open(file),expected);
+assert.throws(()=>zlib.ZipFile.openSync(file),expected);
+});
+
+test('an exact EOCD embedded in a genuine comment is rejected as ambiguous',async()=>{
+constarchive=awaitbuildArchive([
+awaitzlib.ZipEntry.create('f.txt',Buffer.from('content'),{method: 'store'}),
+]);
+constnested=Buffer.concat([archive,buildEocd()]);
+nested.writeUInt16LE(22,archive.length-2);
+
+assert.throws(()=>[...zlib.ZipEntry.read(nested)],{
+code: 'ERR_ZIP_INVALID_ARCHIVE',
+message: /ambiguousendofcentraldirectory/,
+});
+});
+
+test('multiple padded EOCD records are rejected as ambiguous',async()=>{
+constfirst=awaitbuildArchive([
+awaitzlib.ZipEntry.create('a.txt',Buffer.from('first'),{method: 'store'}),
+]);
+constsecond=awaitbuildArchive([
+awaitzlib.ZipEntry.create('b.txt',Buffer.from('second'),{method: 'store'}),
+]);
+constarchive=Buffer.concat([first,second,Buffer.from('\0\0')]);
+
+assert.throws(()=>newzlib.ZipBuffer(archive),{
+code: 'ERR_ZIP_INVALID_ARCHIVE',
+message: /ambiguousendofcentraldirectory/,
+});
+});
+
 test('a declared-size mismatch is rejected as corrupt',async()=>{
 constentry=awaitzlib.ZipEntry.create('f.txt',Buffer.from('hello world'),{method: 'store'});
 constarchive=awaitbuildArchive([entry]);
@@ -212,7 +272,7 @@ test('every possible truncation of an archive is rejected, deterministically', a
 
 test('trailing padding after the EOCD is tolerated',async()=>{
 // Some streaming writers pad their output to a block size; CPython
-// tolerates trailing newlines/NULs and so does the pass-2 EOCD scan.
+// tolerates trailing newlines/NULs and so does the EOCD scan.
 constarchive=awaitbuildArchive(
 [awaitzlib.ZipEntry.create('f.txt',Buffer.from('hi'),{method: 'store'})]);
 constpadded=Buffer.concat([archive,Buffer.from('\r\n\0\0\0')]);
@@ -221,6 +281,15 @@ test('trailing padding after the EOCD is tolerated', async () => {
 assert.strictEqual((awaitentry.content()).toString(),'hi');
 });
 
+test('a maximum-length comment followed by block padding is tolerated',async()=>{
+constcomment='x'.repeat(0xffff);
+constarchive=awaitbuildArchive([],comment);
+constpadded=Buffer.concat([archive,Buffer.alloc(4096)]);
+constzip=newzlib.ZipBuffer(padded);
+
+assert.strictEqual(zip.comment,comment);
+});
+
 test('junk appended past a declared comment is tolerated and the comment preserved',async()=>{
 constarchive=awaitbuildArchive(
 [awaitzlib.ZipEntry.create('f.txt',Buffer.from('hi'),{method: 'store'})],
@@ -262,6 +331,10 @@ test('a record count inconsistent with the directory size is rejected', () => {
 constarchive=Buffer.concat([Buffer.alloc(46),eocd]);
 assert.throws(()=>[...zlib.ZipEntry.read(archive)],
 {code: 'ERR_ZIP_INVALID_ARCHIVE',message: /inconsistent/});
+
+constzeroRecords=Buffer.concat([Buffer.alloc(46),buildEocd({cdSize: 46})]);
+assert.throws(()=>[...zlib.ZipEntry.read(zeroRecords)],
+{code: 'ERR_ZIP_INVALID_ARCHIVE',message: /inconsistent/});
 });
 
 test('a corrupted or overrunning central directory header is rejected',async()=>{

lib/internal/zip/headers.js

@@ -27,6 +27,7 @@ const {
 MADE_BY_UNIX,
 SENTINEL16,
 SENTINEL32,
+TAIL_LENGTH,
 ZIP64_EOCD_MAX_LENGTH,
 S_IFLNK,
 S_IFMT,
@@ -310,6 +311,51 @@ class LocalFileHeader {
 }
 }
 
+// Returns whether an EOCD-looking record could describe an archive this
+// implementation supports. This is deliberately only a cheap preflight: the
+// selected record still receives the complete Zip64 and central-directory
+// validation below.
+functionisPlausibleArchiveEnd(buffer,eocdPos,scanStart){
+constdiskNumber=buffer.readUInt16LE(eocdPos+4);
+constcentralDirectoryDiskNumber=buffer.readUInt16LE(eocdPos+6);
+constdiskRecords=buffer.readUInt16LE(eocdPos+8);
+consttotalRecords=buffer.readUInt16LE(eocdPos+10);
+constcentralDirectorySize=buffer.readUInt32LE(eocdPos+12);
+constcentralDirectoryOffset=buffer.readUInt32LE(eocdPos+16);
+constneedsZip64=
+diskNumber===SENTINEL16||
+centralDirectoryDiskNumber===SENTINEL16||
+diskRecords===SENTINEL16||
+totalRecords===SENTINEL16||
+centralDirectorySize===SENTINEL32||
+centralDirectoryOffset===SENTINEL32;
+
+constlocatorPos=eocdPos-20;
+consthasZip64Locator=locatorPos>=0&&
+buffer.readUInt32LE(locatorPos)===SIG_ZIP64_EOCD_LOCATOR;
+if(needsZip64)returnhasZip64Locator;
+// A Zip64 end record may accompany authoritative, non-sentinel classic
+// fields. Its central directory does not immediately precede this EOCD.
+if(hasZip64Locator)returntrue;
+if(
+diskNumber!==0||
+centralDirectoryDiskNumber!==0||
+diskRecords!==totalRecords||
+totalRecords*46>centralDirectorySize
+){
+returnfalse;
+}
+
+constcentralDirectoryPos=eocdPos-centralDirectorySize;
+if(centralDirectoryPos<scanStart){
+// Use the same logical tail window for memory- and file-backed archives.
+// The directory is not available for this cheap preflight in either case.
+returntrue;
+}
+if(totalRecords===0)returncentralDirectorySize===0;
+returnbuffer.readUInt32LE(centralDirectoryPos)===SIG_CENTRAL_FILE_HEADER;
+}
+
 /**
  * Locates and validates the end-of-archive structures (EOCD, and the Zip64
  * EOCD locator/record when present) in `buffer`. `base` is the absolute
@@ -334,26 +380,30 @@ function findArchiveEnd(buffer, base = 0) {
 if(buffer.length<22){
 thrownewERR_ZIP_INVALID_ARCHIVE('no end of central directory record found');
 }
-constmin=MathMax(0,buffer.length-(22+SENTINEL16));
+// Use the same tail-sized search window for full buffers and ZipFile's tail
+// reads. Besides keeping candidate selection consistent, the extra tail
+// slack permits a maximum-length comment followed by modest writer padding.
+constmin=MathMax(0,buffer.length-TAIL_LENGTH);
 leteocdPos=-1;
-// Pass 1: the comment must reach exactly to the end of the buffer (this
-// rejects a stray EOCD-looking signature inside an earlier comment).
+letfallbackPos=-1;
+letexactFallbackPos=-1;
 for(letpos=buffer.length-22;pos>=min;pos--){
 if(buffer.readUInt32LE(pos)!==SIG_EOCD)continue;
-if(pos+22+buffer.readUInt16LE(pos+20)!==buffer.length)continue;
+constend=pos+22+buffer.readUInt16LE(pos+20);
+if(end>buffer.length)continue;
+if(fallbackPos<0)fallbackPos=pos;
+if(end===buffer.length&&exactFallbackPos<0)exactFallbackPos=pos;
+if(!isPlausibleArchiveEnd(buffer,pos,min))continue;
+if(eocdPos>=0){
+thrownewERR_ZIP_INVALID_ARCHIVE(
+'ambiguous end of central directory records');
+}
 eocdPos=pos;
-break;
 }
+// Preserve the targeted validation errors for a sole malformed or
+// unsupported candidate. Plausible candidates always take precedence.
 if(eocdPos<0){
-// Pass 2: tolerate trailing padding after the EOCD (some streaming
-// writers pad their output to a fixed block size); take the last
-// candidate found.
-for(letpos=buffer.length-22;pos>=min;pos--){
-if(buffer.readUInt32LE(pos)!==SIG_EOCD)continue;
-if(pos+22+buffer.readUInt16LE(pos+20)>buffer.length)continue;
-eocdPos=pos;
-break;
-}
+eocdPos=exactFallbackPos>=0 ? exactFallbackPos : fallbackPos;
 }
 if(eocdPos<0){
 thrownewERR_ZIP_INVALID_ARCHIVE('no end of central directory record found');
@@ -474,7 +524,10 @@ function findArchiveEnd(buffer, base = 0) {
 if(prefix<0){
 thrownewERR_ZIP_INVALID_ARCHIVE('central directory does not fit inside the archive');
 }
-if(totalRecords*46>centralDirectorySize){
+if(
+(totalRecords===0&&centralDirectorySize!==0)||
+totalRecords*46>centralDirectorySize
+){
 thrownewERR_ZIP_INVALID_ARCHIVE(
 'central directory record count is inconsistent with its size');
 }

test/parallel/test-zlib-zip-hardening.js

@@ -3,8 +3,10 @@
 require('../common');
 
 constassert=require('node:assert');
+constfs=require('node:fs');
 constzlib=require('node:zlib');
 const{ test }=require('node:test');
+consttmpdir=require('../common/tmpdir');
 
 asyncfunctionbuildArchive(entries,comment){
 constchunks=[];
@@ -47,13 +49,71 @@ test('an EOCD-looking signature inside a trailing comment is not mistaken for th
 // before it reaches the genuine EOCD signature; embedding 4 bytes that
 // look like one partway through must not be mistaken for the real record.
 constfakeSignature=String.fromCharCode(0x50,0x4b,0x05,0x06);
-constarchive=awaitbuildArchive([entry],`before ${fakeSignature} after`);
+constarchive=awaitbuildArchive(
+[entry],`before ${fakeSignature} this is not a valid EOCD record after`);
 
 constread=[...zlib.ZipEntry.read(archive)];
 assert.strictEqual(read.length,1);
 assert.strictEqual(read[0].name,'f.txt');
 });
 
+test('multiple plausible EOCD records describing different archives are rejected',async()=>{
+constfirst=awaitbuildArchive([
+awaitzlib.ZipEntry.create('install.sh',Buffer.from('malicious'),{method: 'store'}),
+]);
+constsecond=awaitbuildArchive([
+awaitzlib.ZipEntry.create('install.sh',Buffer.from('benign'),{method: 'store'}),
+]);
+constarchive=Buffer.concat([first,second,Buffer.from([0])]);
+constfirstEocd=first.length-22;
+
+// Make the first EOCD exact-to-EOF by treating the second archive and its
+// padding as a comment. The second EOCD remains a plausible archive end for
+// readers which tolerate trailing padding and select the rightmost record.
+archive.writeUInt16LE(archive.length-firstEocd-22,firstEocd+20);
+
+constexpected={
+code: 'ERR_ZIP_INVALID_ARCHIVE',
+message: /ambiguousendofcentraldirectory/,
+};
+assert.throws(()=>[...zlib.ZipEntry.read(archive)],expected);
+assert.throws(()=>newzlib.ZipBuffer(archive),expected);
+
+tmpdir.refresh();
+constfile=tmpdir.resolve('ambiguous.zip');
+fs.writeFileSync(file,archive);
+awaitassert.rejects(zlib.ZipFile.open(file),expected);
+assert.throws(()=>zlib.ZipFile.openSync(file),expected);
+});
+
+test('an exact EOCD embedded in a genuine comment is rejected as ambiguous',async()=>{
+constarchive=awaitbuildArchive([
+awaitzlib.ZipEntry.create('f.txt',Buffer.from('content'),{method: 'store'}),
+]);
+constnested=Buffer.concat([archive,buildEocd()]);
+nested.writeUInt16LE(22,archive.length-2);
+
+assert.throws(()=>[...zlib.ZipEntry.read(nested)],{
+code: 'ERR_ZIP_INVALID_ARCHIVE',
+message: /ambiguousendofcentraldirectory/,
+});
+});
+
+test('multiple padded EOCD records are rejected as ambiguous',async()=>{
+constfirst=awaitbuildArchive([
+awaitzlib.ZipEntry.create('a.txt',Buffer.from('first'),{method: 'store'}),
+]);
+constsecond=awaitbuildArchive([
+awaitzlib.ZipEntry.create('b.txt',Buffer.from('second'),{method: 'store'}),
+]);
+constarchive=Buffer.concat([first,second,Buffer.from('\0\0')]);
+
+assert.throws(()=>newzlib.ZipBuffer(archive),{
+code: 'ERR_ZIP_INVALID_ARCHIVE',
+message: /ambiguousendofcentraldirectory/,
+});
+});
+
 test('a declared-size mismatch is rejected as corrupt',async()=>{
 constentry=awaitzlib.ZipEntry.create('f.txt',Buffer.from('hello world'),{method: 'store'});
 constarchive=awaitbuildArchive([entry]);
@@ -212,7 +272,7 @@ test('every possible truncation of an archive is rejected, deterministically', a
 
 test('trailing padding after the EOCD is tolerated',async()=>{
 // Some streaming writers pad their output to a block size; CPython
-// tolerates trailing newlines/NULs and so does the pass-2 EOCD scan.
+// tolerates trailing newlines/NULs and so does the EOCD scan.
 constarchive=awaitbuildArchive(
 [awaitzlib.ZipEntry.create('f.txt',Buffer.from('hi'),{method: 'store'})]);
 constpadded=Buffer.concat([archive,Buffer.from('\r\n\0\0\0')]);
@@ -221,6 +281,15 @@ test('trailing padding after the EOCD is tolerated', async () => {
 assert.strictEqual((awaitentry.content()).toString(),'hi');
 });
 
+test('a maximum-length comment followed by block padding is tolerated',async()=>{
+constcomment='x'.repeat(0xffff);
+constarchive=awaitbuildArchive([],comment);
+constpadded=Buffer.concat([archive,Buffer.alloc(4096)]);
+constzip=newzlib.ZipBuffer(padded);
+
+assert.strictEqual(zip.comment,comment);
+});
+
 test('junk appended past a declared comment is tolerated and the comment preserved',async()=>{
 constarchive=awaitbuildArchive(
 [awaitzlib.ZipEntry.create('f.txt',Buffer.from('hi'),{method: 'store'})],
@@ -262,6 +331,10 @@ test('a record count inconsistent with the directory size is rejected', () => {
 constarchive=Buffer.concat([Buffer.alloc(46),eocd]);
 assert.throws(()=>[...zlib.ZipEntry.read(archive)],
 {code: 'ERR_ZIP_INVALID_ARCHIVE',message: /inconsistent/});
+
+constzeroRecords=Buffer.concat([Buffer.alloc(46),buildEocd({cdSize: 46})]);
+assert.throws(()=>[...zlib.ZipEntry.read(zeroRecords)],
+{code: 'ERR_ZIP_INVALID_ARCHIVE',message: /inconsistent/});
 });
 
 test('a corrupted or overrunning central directory header is rejected',async()=>{

lib/internal/zip/headers.js

@@ -27,6 +27,7 @@ const {
 MADE_BY_UNIX,
 SENTINEL16,
 SENTINEL32,
+TAIL_LENGTH,
 ZIP64_EOCD_MAX_LENGTH,
 S_IFLNK,
 S_IFMT,
@@ -310,6 +311,51 @@ class LocalFileHeader {
 }
 }
 
+// Returns whether an EOCD-looking record could describe an archive this
+// implementation supports. This is deliberately only a cheap preflight: the
+// selected record still receives the complete Zip64 and central-directory
+// validation below.
+functionisPlausibleArchiveEnd(buffer,eocdPos,scanStart){
+constdiskNumber=buffer.readUInt16LE(eocdPos+4);
+constcentralDirectoryDiskNumber=buffer.readUInt16LE(eocdPos+6);
+constdiskRecords=buffer.readUInt16LE(eocdPos+8);
+consttotalRecords=buffer.readUInt16LE(eocdPos+10);
+constcentralDirectorySize=buffer.readUInt32LE(eocdPos+12);
+constcentralDirectoryOffset=buffer.readUInt32LE(eocdPos+16);
+constneedsZip64=
+diskNumber===SENTINEL16||
+centralDirectoryDiskNumber===SENTINEL16||
+diskRecords===SENTINEL16||
+totalRecords===SENTINEL16||
+centralDirectorySize===SENTINEL32||
+centralDirectoryOffset===SENTINEL32;
+
+constlocatorPos=eocdPos-20;
+consthasZip64Locator=locatorPos>=0&&
+buffer.readUInt32LE(locatorPos)===SIG_ZIP64_EOCD_LOCATOR;
+if(needsZip64)returnhasZip64Locator;
+// A Zip64 end record may accompany authoritative, non-sentinel classic
+// fields. Its central directory does not immediately precede this EOCD.
+if(hasZip64Locator)returntrue;
+if(
+diskNumber!==0||
+centralDirectoryDiskNumber!==0||
+diskRecords!==totalRecords||
+totalRecords*46>centralDirectorySize
+){
+returnfalse;
+}
+
+constcentralDirectoryPos=eocdPos-centralDirectorySize;
+if(centralDirectoryPos<scanStart){
+// Use the same logical tail window for memory- and file-backed archives.
+// The directory is not available for this cheap preflight in either case.
+returntrue;
+}
+if(totalRecords===0)returncentralDirectorySize===0;
+returnbuffer.readUInt32LE(centralDirectoryPos)===SIG_CENTRAL_FILE_HEADER;
+}
+
 /**
  * Locates and validates the end-of-archive structures (EOCD, and the Zip64
  * EOCD locator/record when present) in `buffer`. `base` is the absolute
@@ -334,26 +380,30 @@ function findArchiveEnd(buffer, base = 0) {
 if(buffer.length<22){
 thrownewERR_ZIP_INVALID_ARCHIVE('no end of central directory record found');
 }
-constmin=MathMax(0,buffer.length-(22+SENTINEL16));
+// Use the same tail-sized search window for full buffers and ZipFile's tail
+// reads. Besides keeping candidate selection consistent, the extra tail
+// slack permits a maximum-length comment followed by modest writer padding.
+constmin=MathMax(0,buffer.length-TAIL_LENGTH);
 leteocdPos=-1;
-// Pass 1: the comment must reach exactly to the end of the buffer (this
-// rejects a stray EOCD-looking signature inside an earlier comment).
+letfallbackPos=-1;
+letexactFallbackPos=-1;
 for(letpos=buffer.length-22;pos>=min;pos--){
 if(buffer.readUInt32LE(pos)!==SIG_EOCD)continue;
-if(pos+22+buffer.readUInt16LE(pos+20)!==buffer.length)continue;
+constend=pos+22+buffer.readUInt16LE(pos+20);
+if(end>buffer.length)continue;
+if(fallbackPos<0)fallbackPos=pos;
+if(end===buffer.length&&exactFallbackPos<0)exactFallbackPos=pos;
+if(!isPlausibleArchiveEnd(buffer,pos,min))continue;
+if(eocdPos>=0){
+thrownewERR_ZIP_INVALID_ARCHIVE(
+'ambiguous end of central directory records');
+}
 eocdPos=pos;
-break;
 }
+// Preserve the targeted validation errors for a sole malformed or
+// unsupported candidate. Plausible candidates always take precedence.
 if(eocdPos<0){
-// Pass 2: tolerate trailing padding after the EOCD (some streaming
-// writers pad their output to a fixed block size); take the last
-// candidate found.
-for(letpos=buffer.length-22;pos>=min;pos--){
-if(buffer.readUInt32LE(pos)!==SIG_EOCD)continue;
-if(pos+22+buffer.readUInt16LE(pos+20)>buffer.length)continue;
-eocdPos=pos;
-break;
-}
+eocdPos=exactFallbackPos>=0 ? exactFallbackPos : fallbackPos;
 }
 if(eocdPos<0){
 thrownewERR_ZIP_INVALID_ARCHIVE('no end of central directory record found');
@@ -474,7 +524,10 @@ function findArchiveEnd(buffer, base = 0) {
 if(prefix<0){
 thrownewERR_ZIP_INVALID_ARCHIVE('central directory does not fit inside the archive');
 }
-if(totalRecords*46>centralDirectorySize){
+if(
+(totalRecords===0&&centralDirectorySize!==0)||
+totalRecords*46>centralDirectorySize
+){
 thrownewERR_ZIP_INVALID_ARCHIVE(
 'central directory record count is inconsistent with its size');
 }

test/parallel/test-zlib-zip-hardening.js

@@ -3,8 +3,10 @@
 require('../common');
 
 constassert=require('node:assert');
+constfs=require('node:fs');
 constzlib=require('node:zlib');
 const{ test }=require('node:test');
+consttmpdir=require('../common/tmpdir');
 
 asyncfunctionbuildArchive(entries,comment){
 constchunks=[];
@@ -47,13 +49,71 @@ test('an EOCD-looking signature inside a trailing comment is not mistaken for th
 // before it reaches the genuine EOCD signature; embedding 4 bytes that
 // look like one partway through must not be mistaken for the real record.
 constfakeSignature=String.fromCharCode(0x50,0x4b,0x05,0x06);
-constarchive=awaitbuildArchive([entry],`before ${fakeSignature} after`);
+constarchive=awaitbuildArchive(
+[entry],`before ${fakeSignature} this is not a valid EOCD record after`);
 
 constread=[...zlib.ZipEntry.read(archive)];
 assert.strictEqual(read.length,1);
 assert.strictEqual(read[0].name,'f.txt');
 });
 
+test('multiple plausible EOCD records describing different archives are rejected',async()=>{
+constfirst=awaitbuildArchive([
+awaitzlib.ZipEntry.create('install.sh',Buffer.from('malicious'),{method: 'store'}),
+]);
+constsecond=awaitbuildArchive([
+awaitzlib.ZipEntry.create('install.sh',Buffer.from('benign'),{method: 'store'}),
+]);
+constarchive=Buffer.concat([first,second,Buffer.from([0])]);
+constfirstEocd=first.length-22;
+
+// Make the first EOCD exact-to-EOF by treating the second archive and its
+// padding as a comment. The second EOCD remains a plausible archive end for
+// readers which tolerate trailing padding and select the rightmost record.
+archive.writeUInt16LE(archive.length-firstEocd-22,firstEocd+20);
+
+constexpected={
+code: 'ERR_ZIP_INVALID_ARCHIVE',
+message: /ambiguousendofcentraldirectory/,
+};
+assert.throws(()=>[...zlib.ZipEntry.read(archive)],expected);
+assert.throws(()=>newzlib.ZipBuffer(archive),expected);
+
+tmpdir.refresh();
+constfile=tmpdir.resolve('ambiguous.zip');
+fs.writeFileSync(file,archive);
+awaitassert.rejects(zlib.ZipFile.open(file),expected);
+assert.throws(()=>zlib.ZipFile.openSync(file),expected);
+});
+
+test('an exact EOCD embedded in a genuine comment is rejected as ambiguous',async()=>{
+constarchive=awaitbuildArchive([
+awaitzlib.ZipEntry.create('f.txt',Buffer.from('content'),{method: 'store'}),
+]);
+constnested=Buffer.concat([archive,buildEocd()]);
+nested.writeUInt16LE(22,archive.length-2);
+
+assert.throws(()=>[...zlib.ZipEntry.read(nested)],{
+code: 'ERR_ZIP_INVALID_ARCHIVE',
+message: /ambiguousendofcentraldirectory/,
+});
+});
+
+test('multiple padded EOCD records are rejected as ambiguous',async()=>{
+constfirst=awaitbuildArchive([
+awaitzlib.ZipEntry.create('a.txt',Buffer.from('first'),{method: 'store'}),
+]);
+constsecond=awaitbuildArchive([
+awaitzlib.ZipEntry.create('b.txt',Buffer.from('second'),{method: 'store'}),
+]);
+constarchive=Buffer.concat([first,second,Buffer.from('\0\0')]);
+
+assert.throws(()=>newzlib.ZipBuffer(archive),{
+code: 'ERR_ZIP_INVALID_ARCHIVE',
+message: /ambiguousendofcentraldirectory/,
+});
+});
+
 test('a declared-size mismatch is rejected as corrupt',async()=>{
 constentry=awaitzlib.ZipEntry.create('f.txt',Buffer.from('hello world'),{method: 'store'});
 constarchive=awaitbuildArchive([entry]);
@@ -212,7 +272,7 @@ test('every possible truncation of an archive is rejected, deterministically', a
 
 test('trailing padding after the EOCD is tolerated',async()=>{
 // Some streaming writers pad their output to a block size; CPython
-// tolerates trailing newlines/NULs and so does the pass-2 EOCD scan.
+// tolerates trailing newlines/NULs and so does the EOCD scan.
 constarchive=awaitbuildArchive(
 [awaitzlib.ZipEntry.create('f.txt',Buffer.from('hi'),{method: 'store'})]);
 constpadded=Buffer.concat([archive,Buffer.from('\r\n\0\0\0')]);
@@ -221,6 +281,15 @@ test('trailing padding after the EOCD is tolerated', async () => {
 assert.strictEqual((awaitentry.content()).toString(),'hi');
 });
 
+test('a maximum-length comment followed by block padding is tolerated',async()=>{
+constcomment='x'.repeat(0xffff);
+constarchive=awaitbuildArchive([],comment);
+constpadded=Buffer.concat([archive,Buffer.alloc(4096)]);
+constzip=newzlib.ZipBuffer(padded);
+
+assert.strictEqual(zip.comment,comment);
+});
+
 test('junk appended past a declared comment is tolerated and the comment preserved',async()=>{
 constarchive=awaitbuildArchive(
 [awaitzlib.ZipEntry.create('f.txt',Buffer.from('hi'),{method: 'store'})],
@@ -262,6 +331,10 @@ test('a record count inconsistent with the directory size is rejected', () => {
 constarchive=Buffer.concat([Buffer.alloc(46),eocd]);
 assert.throws(()=>[...zlib.ZipEntry.read(archive)],
 {code: 'ERR_ZIP_INVALID_ARCHIVE',message: /inconsistent/});
+
+constzeroRecords=Buffer.concat([Buffer.alloc(46),buildEocd({cdSize: 46})]);
+assert.throws(()=>[...zlib.ZipEntry.read(zeroRecords)],
+{code: 'ERR_ZIP_INVALID_ARCHIVE',message: /inconsistent/});
 });
 
 test('a corrupted or overrunning central directory header is rejected',async()=>{