From 0e8599dd1d55599d0b513edd3e7fafa50817580e Mon Sep 17 00:00:00 2001 From: sudacode Date: Mon, 10 Aug 2026 22:48:23 -0700 Subject: [PATCH] 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 --- .../character-dictionary-runtime/zip.test.ts | 15 ++++++ src/shared/stored-zip.ts | 50 ++++++++++++++++++- 2 files changed, 64 insertions(+), 1 deletion(-) diff --git a/src/main/character-dictionary-runtime/zip.test.ts b/src/main/character-dictionary-runtime/zip.test.ts index 6b2f73b1..e8fe7a49 100644 --- a/src/main/character-dictionary-runtime/zip.test.ts +++ b/src/main/character-dictionary-runtime/zip.test.ts @@ -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); diff --git a/src/shared/stored-zip.ts b/src/shared/stored-zip.ts index 9323a007..0e87df45 100644 --- a/src/shared/stored-zip.ts +++ b/src/shared/stored-zip.ts @@ -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; }