Skip to content

Commit 556c786

Browse files
committed
fix(file-parsers): match JSZip's ZIP64 trigger and keep non-ZIP inputs on the no-op path
Treat any saturated EOCD field as ZIP64 in the shared central-directory lookup, as JSZip and the spec do, and share one ZIP64 record reader between the guard and the JSZip-view check. The JSZip-view check now mirrors readEndOfCentral's locator and record search and only flags buffers in which JSZip would actually find central-directory records, so a non-ZIP file with EOCD-like bytes and a saturated field is no longer rejected. readZipCentralDirectoryStats applies the same check so the workspace unzip path fails closed alongside the guard.
1 parent 0ecfabc commit 556c786

2 files changed

Lines changed: 142 additions & 78 deletions

File tree

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

Lines changed: 18 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -332,37 +332,39 @@ describe('assertOoxmlArchiveWithinLimits', () => {
332332
expect(await zip.file('word/document.xml')?.async('string')).toHaveLength(200_000)
333333
}
334334

335-
it('rejects an archive with bytes prepended before the first local header', async () => {
336-
const prefixed = Buffer.concat([Buffer.from('JUNKJUNK'), await oversizedEntry()])
337-
await expectJsZipReadsOversizedEntry(prefixed)
335+
it.each([
336+
['prepended bytes', Buffer.from('JUNKJUNK'), Buffer.alloc(0)],
337+
['prepended and trailing bytes', Buffer.from('JUNKJUNK'), Buffer.from('TRAILER')],
338+
])('rejects an archive with %s that JSZip still opens', async (_, prefix, suffix) => {
339+
const wrapped = Buffer.concat([prefix, await oversizedEntry(), suffix])
340+
await expectJsZipReadsOversizedEntry(wrapped)
338341

339-
expect(() => assertOoxmlArchiveWithinLimits(prefixed, PER_ENTRY_LIMITS)).toThrow(
342+
expect(() => assertOoxmlArchiveWithinLimits(wrapped, PER_ENTRY_LIMITS)).toThrow(
340343
ArchiveIntegrityError
341344
)
342345
})
343346

344-
it('rejects a prepended archive that also carries trailing bytes', async () => {
345-
const wrapped = Buffer.concat([
346-
Buffer.from('JUNKJUNK'),
347-
await oversizedEntry(),
348-
Buffer.from('TRAILER'),
349-
])
350-
await expectJsZipReadsOversizedEntry(wrapped)
347+
it('treats any saturated EOCD field as ZIP64, as JSZip does', async () => {
348+
const buffer = await buildZip({ 'word/document.xml': '<xml>hello</xml>' })
349+
buffer.writeUInt16LE(0xffff, buffer.length - 22 + 8) // entries on this disk
350+
await expect(JSZip.loadAsync(buffer)).rejects.toThrow(/ZIP64/)
351351

352-
expect(() => assertOoxmlArchiveWithinLimits(wrapped, PER_ENTRY_LIMITS)).toThrow(
352+
expect(() => assertOoxmlArchiveWithinLimits(buffer, HIGH_LIMITS)).toThrow(
353353
ArchiveIntegrityError
354354
)
355355
})
356356

357-
it('still no-ops for a non-ZIP buffer that merely contains an EOCD signature', async () => {
358-
const eocd = Buffer.alloc(22)
359-
eocd.writeUInt32LE(0x06054b50, 0)
357+
it.each([
358+
['an EOCD signature', 0],
359+
['an EOCD signature with a saturated ZIP64 field', 0xffff],
360+
])('still no-ops for a non-ZIP buffer that merely contains %s', async (_, entryCount) => {
361+
const eocd = buildCentralDirectoryOnly(0)
362+
eocd.writeUInt16LE(entryCount, 10)
360363
const buffer = Buffer.concat([
361364
Buffer.from('%PDF-1.7 binary body '),
362365
eocd,
363366
Buffer.from('tail'),
364367
])
365-
expect(Object.keys((await JSZip.loadAsync(buffer)).files)).toHaveLength(0)
366368

367369
expect(() => assertOoxmlArchiveWithinLimits(buffer)).not.toThrow()
368370
})

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

Lines changed: 124 additions & 62 deletions
Original file line numberDiff line numberDiff line change
@@ -113,69 +113,135 @@ export function findEocdOffset(buffer: Buffer): number {
113113
return -1
114114
}
115115

116-
const EOCD_SIGNATURE_BYTES = Buffer.from([0x50, 0x4b, 0x05, 0x06])
116+
function signatureBytes(signature: number): Buffer {
117+
const bytes = Buffer.alloc(4)
118+
bytes.writeUInt32LE(signature)
119+
return bytes
120+
}
121+
122+
const EOCD_SIGNATURE_BYTES = signatureBytes(EOCD_SIGNATURE)
123+
const ZIP64_EOCD_LOCATOR_SIGNATURE_BYTES = signatureBytes(ZIP64_EOCD_LOCATOR_SIGNATURE)
124+
const ZIP64_EOCD_SIGNATURE_BYTES = signatureBytes(ZIP64_EOCD_SIGNATURE)
125+
126+
function hasSignatureAt(buffer: Buffer, offset: number, signature: number): boolean {
127+
return offset >= 0 && offset + 4 <= buffer.length && buffer.readUInt32LE(offset) === signature
128+
}
117129

118130
/**
119-
* Whether JSZip (which mammoth and the Word document helpers parse through)
120-
* would read central-directory records from a different place than the guard
121-
* inspects. JSZip does not anchor on the buffer tail: it takes the LAST EOCD
122-
* signature anywhere in the buffer, and treats any gap between that record and
123-
* the end of the central directory it declares as prepended data, rebasing
124-
* every offset past it. A buffer the guard reads as "not a ZIP" (or as a
125-
* different ZIP) can therefore still be inflated in full downstream.
126-
*
127-
* Returns `false` when JSZip would refuse the buffer or find no records — a
128-
* non-ZIP file that merely contains the EOCD bytes is not flagged.
131+
* Whether the EOCD defers to the ZIP64 record. JSZip (and the spec) switch on
132+
* ANY saturated field, not just the count and offset — so must the guard, or a
133+
* lone saturated disk field sends JSZip down a chain the guard never follows.
129134
*/
130-
function zipParserReadsDifferentArchive(buffer: Buffer): boolean {
131-
const parserEocdOffset = buffer.lastIndexOf(EOCD_SIGNATURE_BYTES)
132-
if (parserEocdOffset < 0 || parserEocdOffset + EOCD_MIN_SIZE > buffer.length) {
133-
return false
135+
function eocdUsesZip64(buffer: Buffer, eocdOffset: number): boolean {
136+
return (
137+
buffer.readUInt16LE(eocdOffset + 4) === UINT16_SENTINEL ||
138+
buffer.readUInt16LE(eocdOffset + 6) === UINT16_SENTINEL ||
139+
buffer.readUInt16LE(eocdOffset + 8) === UINT16_SENTINEL ||
140+
buffer.readUInt16LE(eocdOffset + 10) === UINT16_SENTINEL ||
141+
buffer.readUInt32LE(eocdOffset + 12) === UINT32_SENTINEL ||
142+
buffer.readUInt32LE(eocdOffset + 16) === UINT32_SENTINEL
143+
)
144+
}
145+
146+
interface Zip64EocdRecord {
147+
entryCount: number
148+
centralDirectorySize: number
149+
centralDirectoryOffset: number
150+
/** Bytes from the record's signature through its last field. */
151+
recordLength: number
152+
}
153+
154+
function readZip64EocdRecord(buffer: Buffer, recordOffset: number): Zip64EocdRecord | null {
155+
if (
156+
recordOffset + 56 > buffer.length ||
157+
!hasSignatureAt(buffer, recordOffset, ZIP64_EOCD_SIGNATURE)
158+
) {
159+
return null
160+
}
161+
return {
162+
entryCount: Number(buffer.readBigUInt64LE(recordOffset + 32)),
163+
centralDirectorySize: Number(buffer.readBigUInt64LE(recordOffset + 40)),
164+
centralDirectoryOffset: Number(buffer.readBigUInt64LE(recordOffset + 48)),
165+
// The size field excludes the leading signature (4) and itself (8).
166+
recordLength: 12 + Number(buffer.readBigUInt64LE(recordOffset + 4)),
134167
}
168+
}
135169

136-
let centralDirectorySize = buffer.readUInt32LE(parserEocdOffset + 12)
137-
let centralDirectoryOffset = buffer.readUInt32LE(parserEocdOffset + 16)
138-
let expectedEocdOffset = centralDirectoryOffset + centralDirectorySize
139-
const usesZip64 =
140-
buffer.readUInt16LE(parserEocdOffset + 10) === UINT16_SENTINEL ||
141-
centralDirectorySize === UINT32_SENTINEL ||
142-
centralDirectoryOffset === UINT32_SENTINEL
143-
if (usesZip64) {
144-
// JSZip falls back to a signature scan when the locator does not resolve,
145-
// so a ZIP64 chain the guard cannot follow from offset 0 is unverifiable.
146-
const locatorOffset = parserEocdOffset - ZIP64_EOCD_LOCATOR_SIZE
147-
if (locatorOffset < 0 || buffer.readUInt32LE(locatorOffset) !== ZIP64_EOCD_LOCATOR_SIGNATURE) {
148-
return true
170+
interface ZipParserView {
171+
eocdOffset: number
172+
/** Bytes JSZip treats as prepended data and rebases every offset past. */
173+
baseOffset: number
174+
/** Absolute offset JSZip starts reading central-directory records from. */
175+
centralDirectoryStart: number
176+
}
177+
178+
/**
179+
* Where JSZip — which mammoth and the Word document helpers parse through —
180+
* reads the archive from, mirroring `readEndOfCentral` in jszip 3.10. Unlike
181+
* the guard it does not anchor on the buffer tail: it takes the LAST EOCD
182+
* signature anywhere, the last ZIP64 locator signature anywhere (and the last
183+
* ZIP64 record signature if the locator's pointer misses), and treats any gap
184+
* before the central directory as prepended data. Returns `null` when JSZip
185+
* would refuse the buffer.
186+
*/
187+
function locateAsZipParser(buffer: Buffer): ZipParserView | null {
188+
const eocdOffset = buffer.lastIndexOf(EOCD_SIGNATURE_BYTES)
189+
if (eocdOffset < 0 || eocdOffset + EOCD_MIN_SIZE > buffer.length) {
190+
return null
191+
}
192+
193+
let centralDirectoryOffset = buffer.readUInt32LE(eocdOffset + 16)
194+
let expectedEocdOffset = centralDirectoryOffset + buffer.readUInt32LE(eocdOffset + 12)
195+
if (eocdUsesZip64(buffer, eocdOffset)) {
196+
const locatorOffset = buffer.lastIndexOf(ZIP64_EOCD_LOCATOR_SIGNATURE_BYTES)
197+
if (locatorOffset < 0 || locatorOffset + ZIP64_EOCD_LOCATOR_SIZE > buffer.length) {
198+
return null
149199
}
150-
const zip64EocdOffset = Number(buffer.readBigUInt64LE(locatorOffset + 8))
151-
if (
152-
zip64EocdOffset + 56 > buffer.length ||
153-
buffer.readUInt32LE(zip64EocdOffset) !== ZIP64_EOCD_SIGNATURE
154-
) {
155-
return true
200+
const pointedRecordOffset = Number(buffer.readBigUInt64LE(locatorOffset + 8))
201+
const record =
202+
readZip64EocdRecord(buffer, pointedRecordOffset) ??
203+
readZip64EocdRecord(buffer, buffer.lastIndexOf(ZIP64_EOCD_SIGNATURE_BYTES))
204+
if (!record) {
205+
return null
156206
}
157-
centralDirectorySize = Number(buffer.readBigUInt64LE(zip64EocdOffset + 40))
158-
centralDirectoryOffset = Number(buffer.readBigUInt64LE(zip64EocdOffset + 48))
207+
centralDirectoryOffset = record.centralDirectoryOffset
159208
expectedEocdOffset =
160209
centralDirectoryOffset +
161-
centralDirectorySize +
162-
ZIP64_EOCD_LOCATOR_SIZE +
163-
12 +
164-
Number(buffer.readBigUInt64LE(zip64EocdOffset + 4))
210+
record.centralDirectorySize +
211+
record.recordLength +
212+
ZIP64_EOCD_LOCATOR_SIZE
165213
}
166214

167-
const prependedBytes = parserEocdOffset - expectedEocdOffset
168-
if (prependedBytes < 0) {
169-
return false
215+
const baseOffset = eocdOffset - expectedEocdOffset
216+
if (baseOffset < 0) {
217+
return null
170218
}
171-
if (prependedBytes === 0 && parserEocdOffset === findEocdOffset(buffer)) {
219+
return { eocdOffset, baseOffset, centralDirectoryStart: baseOffset + centralDirectoryOffset }
220+
}
221+
222+
/**
223+
* Whether JSZip would read central-directory records from a different place
224+
* than the guard inspects — prepended data, a later EOCD signature, or a
225+
* diverging ZIP64 chain — so a buffer the guard reads as "not a ZIP" (or as a
226+
* different ZIP) would still be inflated in full downstream. A buffer JSZip
227+
* would refuse, or in which it finds no records, is not flagged, so a non-ZIP
228+
* file that merely contains EOCD-like bytes keeps the no-op path.
229+
*/
230+
function zipParserReadsDifferentArchive(buffer: Buffer): boolean {
231+
const parser = locateAsZipParser(buffer)
232+
if (
233+
!parser ||
234+
!hasSignatureAt(buffer, parser.centralDirectoryStart, CENTRAL_DIRECTORY_HEADER_SIGNATURE)
235+
) {
172236
return false
173237
}
174238

175-
const parserCentralDirectoryStart = prependedBytes + centralDirectoryOffset
176-
return (
177-
parserCentralDirectoryStart + 4 <= buffer.length &&
178-
buffer.readUInt32LE(parserCentralDirectoryStart) === CENTRAL_DIRECTORY_HEADER_SIGNATURE
239+
const guardEocdOffset = findEocdOffset(buffer)
240+
const guardLocation = guardEocdOffset < 0 ? null : locateCentralDirectory(buffer, guardEocdOffset)
241+
return !(
242+
parser.baseOffset === 0 &&
243+
parser.eocdOffset === guardEocdOffset &&
244+
guardLocation?.offset === parser.centralDirectoryStart
179245
)
180246
}
181247

@@ -186,7 +252,7 @@ export interface CentralDirectoryLocation {
186252

187253
/**
188254
* Resolve the central directory offset and entry count, following the ZIP64
189-
* end-of-central-directory chain when the 32-bit fields are saturated.
255+
* end-of-central-directory chain when any EOCD field is saturated.
190256
*/
191257
export function locateCentralDirectory(
192258
buffer: Buffer,
@@ -195,24 +261,19 @@ export function locateCentralDirectory(
195261
let entryCount = buffer.readUInt16LE(eocdOffset + 10)
196262
let cdOffset = buffer.readUInt32LE(eocdOffset + 16)
197263

198-
const needsZip64 = entryCount === UINT16_SENTINEL || cdOffset === UINT32_SENTINEL
199-
if (needsZip64) {
264+
if (eocdUsesZip64(buffer, eocdOffset)) {
200265
const locatorOffset = eocdOffset - ZIP64_EOCD_LOCATOR_SIZE
201-
if (locatorOffset < 0 || buffer.readUInt32LE(locatorOffset) !== ZIP64_EOCD_LOCATOR_SIGNATURE) {
266+
if (!hasSignatureAt(buffer, locatorOffset, ZIP64_EOCD_LOCATOR_SIGNATURE)) {
202267
return null
203268
}
204269

205-
const zip64EocdOffset = Number(buffer.readBigUInt64LE(locatorOffset + 8))
206-
if (
207-
zip64EocdOffset < 0 ||
208-
zip64EocdOffset + 56 > buffer.length ||
209-
buffer.readUInt32LE(zip64EocdOffset) !== ZIP64_EOCD_SIGNATURE
210-
) {
270+
const record = readZip64EocdRecord(buffer, Number(buffer.readBigUInt64LE(locatorOffset + 8)))
271+
if (!record) {
211272
return null
212273
}
213274

214-
entryCount = Number(buffer.readBigUInt64LE(zip64EocdOffset + 32))
215-
cdOffset = Number(buffer.readBigUInt64LE(zip64EocdOffset + 48))
275+
entryCount = record.entryCount
276+
cdOffset = record.centralDirectoryOffset
216277
}
217278

218279
if (cdOffset < 0 || cdOffset > buffer.length) {
@@ -514,10 +575,11 @@ export interface ZipCentralDirectoryStats {
514575
* per — a lied count can neither hide records nor inflate the tally. Unlike a
515576
* raw whole-buffer signature scan, STORED entry payloads (e.g. a nested `.zip`
516577
* archived without recompression) are never miscounted as records. Returns
517-
* `null` when the buffer is not a parseable ZIP, so callers can fail closed.
578+
* `null` when the buffer is not a parseable ZIP, or when JSZip would read its
579+
* records from somewhere else, so callers can fail closed.
518580
*/
519581
export function readZipCentralDirectoryStats(buffer: Buffer): ZipCentralDirectoryStats | null {
520-
if (buffer.length < EOCD_MIN_SIZE) {
582+
if (buffer.length < EOCD_MIN_SIZE || zipParserReadsDifferentArchive(buffer)) {
521583
return null
522584
}
523585

0 commit comments

Comments
 (0)