Skip to content

Commit 0ecfabc

Browse files
committed
fix(file-parsers): reject ZIP archives JSZip reads from outside the guarded region
The OOXML zip-bomb guard anchored on the tail EOCD and assumed the archive starts at offset 0, and it skipped anything not starting with a ZIP signature. JSZip takes the last EOCD signature anywhere in the buffer and rebases past prepended data, so prefixing an archive with a few bytes disabled every size and inflation check while mammoth and the Word helpers still inflated it in full. The guard now rejects any buffer JSZip would read from a different EOCD or base offset than the one it inspected.
1 parent 472399b commit 0ecfabc

2 files changed

Lines changed: 124 additions & 1 deletion

File tree

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

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -322,4 +322,49 @@ 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('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)
338+
339+
expect(() => assertOoxmlArchiveWithinLimits(prefixed, PER_ENTRY_LIMITS)).toThrow(
340+
ArchiveIntegrityError
341+
)
342+
})
343+
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)
351+
352+
expect(() => assertOoxmlArchiveWithinLimits(wrapped, PER_ENTRY_LIMITS)).toThrow(
353+
ArchiveIntegrityError
354+
)
355+
})
356+
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)
360+
const buffer = Buffer.concat([
361+
Buffer.from('%PDF-1.7 binary body '),
362+
eocd,
363+
Buffer.from('tail'),
364+
])
365+
expect(Object.keys((await JSZip.loadAsync(buffer)).files)).toHaveLength(0)
366+
367+
expect(() => assertOoxmlArchiveWithinLimits(buffer)).not.toThrow()
368+
})
369+
})
325370
})

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

Lines changed: 79 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -113,6 +113,72 @@ export function findEocdOffset(buffer: Buffer): number {
113113
return -1
114114
}
115115

116+
const EOCD_SIGNATURE_BYTES = Buffer.from([0x50, 0x4b, 0x05, 0x06])
117+
118+
/**
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.
129+
*/
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
134+
}
135+
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
149+
}
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
156+
}
157+
centralDirectorySize = Number(buffer.readBigUInt64LE(zip64EocdOffset + 40))
158+
centralDirectoryOffset = Number(buffer.readBigUInt64LE(zip64EocdOffset + 48))
159+
expectedEocdOffset =
160+
centralDirectoryOffset +
161+
centralDirectorySize +
162+
ZIP64_EOCD_LOCATOR_SIZE +
163+
12 +
164+
Number(buffer.readBigUInt64LE(zip64EocdOffset + 4))
165+
}
166+
167+
const prependedBytes = parserEocdOffset - expectedEocdOffset
168+
if (prependedBytes < 0) {
169+
return false
170+
}
171+
if (prependedBytes === 0 && parserEocdOffset === findEocdOffset(buffer)) {
172+
return false
173+
}
174+
175+
const parserCentralDirectoryStart = prependedBytes + centralDirectoryOffset
176+
return (
177+
parserCentralDirectoryStart + 4 <= buffer.length &&
178+
buffer.readUInt32LE(parserCentralDirectoryStart) === CENTRAL_DIRECTORY_HEADER_SIGNATURE
179+
)
180+
}
181+
116182
export interface CentralDirectoryLocation {
117183
offset: number
118184
entryCount: number
@@ -503,12 +569,24 @@ export function readZipCentralDirectoryStats(buffer: Buffer): ZipCentralDirector
503569
* or whose entries cannot be verified, is rejected rather than passed through,
504570
* so a malformed archive that a downstream library still inflates cannot bypass
505571
* the guard. Genuinely non-ZIP inputs (legacy OLE `.xls`/`.doc`, misidentified
506-
* plaintext) no-op and defer to the downstream parser's own validation.
572+
* plaintext) no-op and defer to the downstream parser's own validation. "Genuinely
573+
* non-ZIP" is judged by what JSZip would open, not by the leading bytes: an
574+
* archive with prepended data or a relocated EOCD is rejected, because JSZip
575+
* reads it from a place this guard never inspected.
507576
*/
508577
export function assertOoxmlArchiveWithinLimits(
509578
buffer: Buffer,
510579
limits: OoxmlSizeLimits = DEFAULT_OOXML_SIZE_LIMITS
511580
): void {
581+
if (zipParserReadsDifferentArchive(buffer)) {
582+
logger.warn('Rejected ZIP archive: parser would read records the guard cannot inspect', {
583+
compressedBytes: buffer.length,
584+
})
585+
throw new ArchiveIntegrityError(
586+
'Archive has prepended data or a relocated end-of-central-directory record; refusing to parse an unverifiable ZIP archive'
587+
)
588+
}
589+
512590
const declared = sumDeclaredUncompressedSize(buffer, limits)
513591
if (declared === null) {
514592
if (isZipShaped(buffer)) {

0 commit comments

Comments
 (0)