Skip to content

Commit bea4e75

Browse files
committed
fix(file-parsers): charge hidden central-directory entries against the cap
sumDeclaredUncompressedSize walked only the entry count the EOCD declares, while verification walks the contiguous run of records. JSZip's readCentralDir loops on the record signature and keeps every entry it finds — a count mismatch is explicitly not an error there — so an archive that under-reported its count could hide honestly-large entries from the total-size cap and still have the parser expand them. The sum now walks the same contiguous run as the verification pass and readZipCentralDirectoryStats, and fails closed when the run is shorter than the declared count. Caught by Cursor Bugbot review.
1 parent fac1c7c commit bea4e75

2 files changed

Lines changed: 42 additions & 8 deletions

File tree

apps/sim/lib/file-parsers/zip-guard.test.ts

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -231,6 +231,29 @@ describe('assertOoxmlArchiveWithinLimits', () => {
231231
)
232232
})
233233

234+
it('charges entries hidden behind an under-reported EOCD count against the cap', async () => {
235+
// JSZip's readCentralDir loops on the record signature and keeps every
236+
// entry it finds — a count mismatch is explicitly not an error there — so
237+
// entries past the declared count must still be charged against the cap.
238+
const buffer = await buildZip({
239+
'a.xml': 'A'.repeat(60_000),
240+
'b.xml': 'B'.repeat(60_000),
241+
'c.xml': 'C'.repeat(60_000),
242+
})
243+
const eocdOffset = buffer.length - 22
244+
expect(buffer.readUInt32LE(eocdOffset)).toBe(0x06054b50)
245+
buffer.writeUInt16LE(1, eocdOffset + 8) // entries on this disk
246+
buffer.writeUInt16LE(1, eocdOffset + 10) // total entries
247+
248+
expect(() =>
249+
assertOoxmlArchiveWithinLimits(buffer, {
250+
maxTotalUncompressedBytes: 100_000,
251+
maxCompressionRatio: 10_000,
252+
ratioCheckFloorBytes: 1024 * 1024 * 1024,
253+
})
254+
).toThrow(/exceeds the maximum allowed/)
255+
})
256+
234257
it('accepts a multi-entry archive whose entries all inflate to what they declare', async () => {
235258
const buffer = await buildZip({
236259
'[Content_Types].xml': '<?xml version="1.0"?><Types/>',

apps/sim/lib/file-parsers/zip-guard.ts

Lines changed: 19 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -217,6 +217,13 @@ function readCentralDirectoryEntry(
217217
* `null` when the buffer is not a parseable ZIP archive (e.g. legacy binary
218218
* `.xls`/`.doc`, or a misidentified plaintext file) so the caller can defer to
219219
* the downstream parser. Stops early once the running total exceeds the limit.
220+
*
221+
* Like {@link readZipCentralDirectoryStats}, this charges the CONTIGUOUS run of
222+
* records rather than the EOCD's declared count. JSZip's `readCentralDir` loops
223+
* on the record signature and keeps every entry it finds — a count mismatch is
224+
* explicitly not an error there — so an archive that under-reports its count
225+
* would otherwise hide honestly-large entries from this cap while the parser
226+
* still expanded them.
220227
*/
221228
function sumDeclaredUncompressedSize(buffer: Buffer, abortAboveBytes: number): number | null {
222229
if (buffer.length < EOCD_MIN_SIZE) {
@@ -234,15 +241,12 @@ function sumDeclaredUncompressedSize(buffer: Buffer, abortAboveBytes: number): n
234241
}
235242

236243
let total = 0
244+
let counted = 0
237245
let cursor = location.offset
238-
for (let entry = 0; entry < location.entryCount; entry++) {
239-
if (cursor + CENTRAL_DIRECTORY_HEADER_MIN_SIZE > buffer.length) {
240-
return null
241-
}
242-
if (buffer.readUInt32LE(cursor) !== CENTRAL_DIRECTORY_HEADER_SIGNATURE) {
243-
return null
244-
}
245-
246+
while (
247+
cursor + CENTRAL_DIRECTORY_HEADER_MIN_SIZE <= buffer.length &&
248+
buffer.readUInt32LE(cursor) === CENTRAL_DIRECTORY_HEADER_SIGNATURE
249+
) {
246250
const fileNameLength = buffer.readUInt16LE(cursor + 28)
247251
const extraFieldLength = buffer.readUInt16LE(cursor + 30)
248252
const commentLength = buffer.readUInt16LE(cursor + 32)
@@ -257,9 +261,16 @@ function sumDeclaredUncompressedSize(buffer: Buffer, abortAboveBytes: number): n
257261
return total
258262
}
259263

264+
counted += 1
260265
cursor += CENTRAL_DIRECTORY_HEADER_MIN_SIZE + fileNameLength + extraFieldLength + commentLength
261266
}
262267

268+
// Fewer records than the archive claims means the directory is malformed;
269+
// fail closed rather than charging a partial total against the cap.
270+
if (counted < location.entryCount) {
271+
return null
272+
}
273+
263274
return total
264275
}
265276

0 commit comments

Comments
 (0)