fix(dictionary): reject zips with corrupted central directory records

- Walk every central-directory record and validate its signature and variable-length field bounds so an in-place corrupted record is rejected even when the archive's overall size still matches
- Add tests covering a corrupted record signature and an out-of-bounds name length
This commit is contained in:
2026-08-10 22:48:23 -07:00
parent 8a56b11d0c
commit 0e8599dd1d
2 changed files with 64 additions and 1 deletions
@@ -143,6 +143,21 @@ test('readDictionaryZipRevision reads the built revision and rejects foreign arc
assert.equal(readDictionaryZipRevision(cutPath), null, `cut of ${missingBytes} bytes`);
}
// Same size, corrupt directory: a record overwritten in place has to be rejected too.
const centralStart = archive.readUInt32LE(archive.length - 22 + 16);
const brokenSignaturePath = path.join(dir, 'broken-signature.zip');
const brokenSignature = Buffer.from(archive);
brokenSignature.writeUInt32LE(0xdeadbeef, centralStart);
fs.writeFileSync(brokenSignaturePath, brokenSignature);
assert.equal(readDictionaryZipRevision(brokenSignaturePath), null);
const brokenLengthPath = path.join(dir, 'broken-length.zip');
const brokenLength = Buffer.from(archive);
// Name length that runs the walk past the end of the directory.
brokenLength.writeUInt16LE(0xffff, centralStart + 28);
fs.writeFileSync(brokenLengthPath, brokenLength);
assert.equal(readDictionaryZipRevision(brokenLengthPath), null);
const foreignPath = path.join(dir, 'foreign.zip');
fs.writeFileSync(foreignPath, Buffer.from('not a zip at all', 'utf8'));
assert.equal(readDictionaryZipRevision(foreignPath), null);
+49 -1
View File
@@ -152,6 +152,50 @@ const LOCAL_FILE_HEADER_SIGNATURE = 0x04034b50;
const LOCAL_FILE_HEADER_SIZE = 30;
const END_OF_CENTRAL_DIRECTORY_SIGNATURE = 0x06054b50;
const END_OF_CENTRAL_DIRECTORY_SIZE = 22;
const CENTRAL_FILE_HEADER_SIGNATURE = 0x02014b50;
const CENTRAL_FILE_HEADER_SIZE = 46;
// 65535 entries with names of a few dozen bytes stay far under this; the cap only stops a corrupt
// record length from asking for an allocation the size of the archive.
const MAX_CENTRAL_DIRECTORY_BYTES = 16 * 1024 * 1024;
/**
* Walks every declared central-directory record, checking each signature and keeping the
* variable-length name/extra/comment fields inside the directory. The walk has to land exactly on
* the end of the directory, so a record that was overwritten in place fails even though the file
* kept its size.
*/
function isCentralDirectoryIntact(
fd: number,
centralStart: number,
centralSize: number,
entryCount: number,
): boolean {
if (centralSize === 0 || centralSize > MAX_CENTRAL_DIRECTORY_BYTES) {
return false;
}
const central = Buffer.alloc(centralSize);
if (fs.readSync(fd, central, 0, centralSize, centralStart) !== centralSize) {
return false;
}
let cursor = 0;
for (let index = 0; index < entryCount; index += 1) {
if (cursor + CENTRAL_FILE_HEADER_SIZE > centralSize) {
return false;
}
if (central.readUInt32LE(cursor) !== CENTRAL_FILE_HEADER_SIGNATURE) {
return false;
}
const nameLength = central.readUInt16LE(cursor + 28);
const extraLength = central.readUInt16LE(cursor + 30);
const commentLength = central.readUInt16LE(cursor + 32);
cursor += CENTRAL_FILE_HEADER_SIZE + nameLength + extraLength + commentLength;
if (cursor > centralSize) {
return false;
}
}
return cursor === centralSize;
}
/**
* Start of the central directory, or null when the archive is not a complete one of ours. The
@@ -171,7 +215,8 @@ function readCentralDirectoryStart(fd: number, fileSize: number): number | null
if (end.readUInt32LE(0) !== END_OF_CENTRAL_DIRECTORY_SIGNATURE || end.readUInt16LE(20) !== 0) {
return null;
}
if (end.readUInt16LE(10) === 0) {
const entryCount = end.readUInt16LE(10);
if (entryCount === 0) {
return null;
}
const centralSize = end.readUInt32LE(12);
@@ -179,6 +224,9 @@ function readCentralDirectoryStart(fd: number, fileSize: number): number | null
if (centralStart + centralSize !== endOffset) {
return null;
}
if (!isCentralDirectoryIntact(fd, centralStart, centralSize, entryCount)) {
return null;
}
return centralStart;
}