Skip to content

Commit 44ce671

Browse files
authored
fix(file-parsers): reject ZIP archives JSZip reads from outside the guarded region (#9773)
1 parent 17a0287 commit 44ce671

2 files changed

Lines changed: 188 additions & 9 deletions

File tree

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

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -322,4 +322,51 @@ describe('assertOoxmlArchiveWithinLimits', () => {
322322
const plaintext = Buffer.from('this is just plain text, not a zip archive at all')
323323
expect(() => assertOoxmlArchiveWithinLimits(plaintext)).not.toThrow()
324324
})
325+
326+
describe('archives JSZip reads from a different place than the guard inspects', () => {
327+
const PER_ENTRY_LIMITS: OoxmlSizeLimits = { ...HIGH_LIMITS, maxEntryUncompressedBytes: 100_000 }
328+
const oversizedEntry = () => buildZip({ 'word/document.xml': 'A'.repeat(200_000) })
329+
330+
async function expectJsZipReadsOversizedEntry(buffer: Buffer) {
331+
const zip = await JSZip.loadAsync(buffer)
332+
expect(await zip.file('word/document.xml')?.async('string')).toHaveLength(200_000)
333+
}
334+
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)
341+
342+
expect(() => assertOoxmlArchiveWithinLimits(wrapped, PER_ENTRY_LIMITS)).toThrow(
343+
ArchiveIntegrityError
344+
)
345+
})
346+
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/)
351+
352+
expect(() => assertOoxmlArchiveWithinLimits(buffer, HIGH_LIMITS)).toThrow(
353+
ArchiveIntegrityError
354+
)
355+
})
356+
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)
363+
const buffer = Buffer.concat([
364+
Buffer.from('%PDF-1.7 binary body '),
365+
eocd,
366+
Buffer.from('tail'),
367+
])
368+
369+
expect(() => assertOoxmlArchiveWithinLimits(buffer)).not.toThrow()
370+
})
371+
})
325372
})

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

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

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+
}
129+
130+
/**
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.
134+
*/
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 ZipParserView {
147+
eocdOffset: number
148+
/** Bytes JSZip treats as prepended data and rebases every offset past. */
149+
baseOffset: number
150+
/** Absolute offset JSZip starts reading central-directory records from. */
151+
centralDirectoryStart: number
152+
}
153+
154+
/**
155+
* Where JSZip — which mammoth and the Word document helpers parse through —
156+
* reads the archive from, mirroring `readEndOfCentral` in jszip 3.10. Unlike
157+
* the guard it does not anchor on the buffer tail: it takes the LAST EOCD
158+
* signature anywhere, the last ZIP64 locator signature anywhere (and the last
159+
* ZIP64 record signature if the locator's in-range pointer misses), and treats
160+
* any gap before the central directory as prepended data. Returns `null` when
161+
* JSZip would refuse the buffer.
162+
*
163+
* Fields are read with JSZip's integer semantics, not the spec's: its
164+
* `readInt` folds 4- and 8-byte values through 32-bit shifts, keeping only the
165+
* low dword as a signed int (`readInt32LE`). Reading the full width here would
166+
* let a value with high bits set look out of range while JSZip reads it as an
167+
* ordinary offset or size.
168+
*/
169+
function locateAsZipParser(buffer: Buffer): ZipParserView | null {
170+
const eocdOffset = buffer.lastIndexOf(EOCD_SIGNATURE_BYTES)
171+
if (eocdOffset < 0 || eocdOffset + EOCD_MIN_SIZE > buffer.length) {
172+
return null
173+
}
174+
175+
let centralDirectoryOffset = buffer.readInt32LE(eocdOffset + 16)
176+
let expectedEocdOffset = centralDirectoryOffset + buffer.readInt32LE(eocdOffset + 12)
177+
if (eocdUsesZip64(buffer, eocdOffset)) {
178+
const locatorOffset = buffer.lastIndexOf(ZIP64_EOCD_LOCATOR_SIGNATURE_BYTES)
179+
if (locatorOffset < 0 || locatorOffset + ZIP64_EOCD_LOCATOR_SIZE > buffer.length) {
180+
return null
181+
}
182+
// JSZip's signature probe throws on an out-of-range pointer instead of
183+
// falling back to the signature scan.
184+
const pointedRecordOffset = buffer.readInt32LE(locatorOffset + 8)
185+
if (pointedRecordOffset < 0 || pointedRecordOffset + 4 > buffer.length) {
186+
return null
187+
}
188+
const recordOffset = hasSignatureAt(buffer, pointedRecordOffset, ZIP64_EOCD_SIGNATURE)
189+
? pointedRecordOffset
190+
: buffer.lastIndexOf(ZIP64_EOCD_SIGNATURE_BYTES)
191+
if (recordOffset < 0 || recordOffset + 56 > buffer.length) {
192+
return null
193+
}
194+
centralDirectoryOffset = buffer.readInt32LE(recordOffset + 48)
195+
// The record's size field (+4) excludes its leading signature and itself (12 bytes).
196+
expectedEocdOffset =
197+
centralDirectoryOffset +
198+
buffer.readInt32LE(recordOffset + 40) +
199+
12 +
200+
buffer.readInt32LE(recordOffset + 4) +
201+
ZIP64_EOCD_LOCATOR_SIZE
202+
}
203+
204+
const baseOffset = eocdOffset - expectedEocdOffset
205+
if (baseOffset < 0 || centralDirectoryOffset < 0) {
206+
return null
207+
}
208+
return { eocdOffset, baseOffset, centralDirectoryStart: baseOffset + centralDirectoryOffset }
209+
}
210+
211+
/**
212+
* Whether JSZip would read central-directory records from a different place
213+
* than the guard inspects — prepended data, a later EOCD signature, or a
214+
* diverging ZIP64 chain — so a buffer the guard reads as "not a ZIP" (or as a
215+
* different ZIP) would still be inflated in full downstream. A buffer JSZip
216+
* would refuse, or in which it finds no records, is not flagged, so a non-ZIP
217+
* file that merely contains EOCD-like bytes keeps the no-op path.
218+
*/
219+
function zipParserReadsDifferentArchive(buffer: Buffer): boolean {
220+
const parser = locateAsZipParser(buffer)
221+
if (
222+
!parser ||
223+
!hasSignatureAt(buffer, parser.centralDirectoryStart, CENTRAL_DIRECTORY_HEADER_SIGNATURE)
224+
) {
225+
return false
226+
}
227+
228+
const guardEocdOffset = findEocdOffset(buffer)
229+
const guardLocation = guardEocdOffset < 0 ? null : locateCentralDirectory(buffer, guardEocdOffset)
230+
return !(
231+
parser.baseOffset === 0 &&
232+
parser.eocdOffset === guardEocdOffset &&
233+
guardLocation?.offset === parser.centralDirectoryStart
234+
)
235+
}
236+
116237
export interface CentralDirectoryLocation {
117238
offset: number
118239
entryCount: number
119240
}
120241

121242
/**
122243
* Resolve the central directory offset and entry count, following the ZIP64
123-
* end-of-central-directory chain when the 32-bit fields are saturated.
244+
* end-of-central-directory chain when any EOCD field is saturated.
124245
*/
125246
export function locateCentralDirectory(
126247
buffer: Buffer,
@@ -129,18 +250,16 @@ export function locateCentralDirectory(
129250
let entryCount = buffer.readUInt16LE(eocdOffset + 10)
130251
let cdOffset = buffer.readUInt32LE(eocdOffset + 16)
131252

132-
const needsZip64 = entryCount === UINT16_SENTINEL || cdOffset === UINT32_SENTINEL
133-
if (needsZip64) {
253+
if (eocdUsesZip64(buffer, eocdOffset)) {
134254
const locatorOffset = eocdOffset - ZIP64_EOCD_LOCATOR_SIZE
135-
if (locatorOffset < 0 || buffer.readUInt32LE(locatorOffset) !== ZIP64_EOCD_LOCATOR_SIGNATURE) {
255+
if (!hasSignatureAt(buffer, locatorOffset, ZIP64_EOCD_LOCATOR_SIGNATURE)) {
136256
return null
137257
}
138258

139259
const zip64EocdOffset = Number(buffer.readBigUInt64LE(locatorOffset + 8))
140260
if (
141-
zip64EocdOffset < 0 ||
142261
zip64EocdOffset + 56 > buffer.length ||
143-
buffer.readUInt32LE(zip64EocdOffset) !== ZIP64_EOCD_SIGNATURE
262+
!hasSignatureAt(buffer, zip64EocdOffset, ZIP64_EOCD_SIGNATURE)
144263
) {
145264
return null
146265
}
@@ -448,10 +567,11 @@ export interface ZipCentralDirectoryStats {
448567
* per — a lied count can neither hide records nor inflate the tally. Unlike a
449568
* raw whole-buffer signature scan, STORED entry payloads (e.g. a nested `.zip`
450569
* archived without recompression) are never miscounted as records. Returns
451-
* `null` when the buffer is not a parseable ZIP, so callers can fail closed.
570+
* `null` when the buffer is not a parseable ZIP, or when JSZip would read its
571+
* records from somewhere else, so callers can fail closed.
452572
*/
453573
export function readZipCentralDirectoryStats(buffer: Buffer): ZipCentralDirectoryStats | null {
454-
if (buffer.length < EOCD_MIN_SIZE) {
574+
if (buffer.length < EOCD_MIN_SIZE || zipParserReadsDifferentArchive(buffer)) {
455575
return null
456576
}
457577

@@ -503,12 +623,24 @@ export function readZipCentralDirectoryStats(buffer: Buffer): ZipCentralDirector
503623
* or whose entries cannot be verified, is rejected rather than passed through,
504624
* so a malformed archive that a downstream library still inflates cannot bypass
505625
* the guard. Genuinely non-ZIP inputs (legacy OLE `.xls`/`.doc`, misidentified
506-
* plaintext) no-op and defer to the downstream parser's own validation.
626+
* plaintext) no-op and defer to the downstream parser's own validation. "Genuinely
627+
* non-ZIP" is judged by what JSZip would open, not by the leading bytes: an
628+
* archive with prepended data or a relocated EOCD is rejected, because JSZip
629+
* reads it from a place this guard never inspected.
507630
*/
508631
export function assertOoxmlArchiveWithinLimits(
509632
buffer: Buffer,
510633
limits: OoxmlSizeLimits = DEFAULT_OOXML_SIZE_LIMITS
511634
): void {
635+
if (zipParserReadsDifferentArchive(buffer)) {
636+
logger.warn('Rejected ZIP archive: parser would read records the guard cannot inspect', {
637+
compressedBytes: buffer.length,
638+
})
639+
throw new ArchiveIntegrityError(
640+
'Archive has prepended data or a relocated end-of-central-directory record; refusing to parse an unverifiable ZIP archive'
641+
)
642+
}
643+
512644
const declared = sumDeclaredUncompressedSize(buffer, limits)
513645
if (declared === null) {
514646
if (isZipShaped(buffer)) {

0 commit comments

Comments
 (0)