Skip to content

Commit 07e2eea

Browse files
committed
fix(file-parsers): verify actual inflation, not just declared ZIP sizes
The declared uncompressed sizes in a ZIP central directory are attacker- controlled, so a bomb can under-report them and pass the size and ratio checks untouched. officeparser and mammoth only detect the mismatch after inflating the entry in full: a 498 KB archive declaring 1000 bytes per entry drove 559 MB resident through the .doc parser and 538 MB through .docx, then failed. SheetJS and officeparser reject the container earlier, so xlsx/pptx were not affected, but doc and docx both were. Each entry is now inflated during verification under a maxOutputLength bound equal to the size it declared. Node's zlib aborts the moment output would exceed that bound, so a lying entry costs only its declared size and the inflated bytes are discarded immediately; both bomb variants now reject at +0 MB across every extension. Stored entries are checked against their own compressed size, and unsupported compression methods fail closed. Verification walks the contiguous run of central-directory records rather than the EOCD's declared entry count, since that run is what a decompression library allocates per entry — a lied-down count must not hide an entry from verification. Cost is ~0.45 ms per MB of uncompressed content (22 ms for a 50 MB archive), against parse times an order of magnitude larger. All 17 real Word-produced .docx fixtures in mammoth's test data are still accepted.
1 parent 6a006c4 commit 07e2eea

3 files changed

Lines changed: 301 additions & 29 deletions

File tree

apps/sim/lib/file-parsers/doc-parser.test.ts

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,32 @@ describe('DocParser.parseBuffer', () => {
6464
expect(mockExtractRawText).not.toHaveBeenCalled()
6565
})
6666

67+
it('rejects a .doc that under-declares its uncompressed size', async () => {
68+
// Declared sizes alone put this under every limit; officeparser and mammoth
69+
// only notice the mismatch after inflating the entry in full, so the guard
70+
// has to catch it before either library sees the buffer.
71+
const zip = new JSZip()
72+
zip.file('word/document.xml', 'A'.repeat(4 * 1024 * 1024))
73+
const honest = (await zip.generateAsync({
74+
type: 'nodebuffer',
75+
compression: 'DEFLATE',
76+
})) as Buffer
77+
78+
const lying = Buffer.from(honest)
79+
for (let offset = 0; offset + 30 <= lying.length; offset++) {
80+
const signature = lying.readUInt32LE(offset)
81+
if (signature === CENTRAL_DIRECTORY_HEADER_SIGNATURE) {
82+
lying.writeUInt32LE(1000, offset + 24)
83+
} else if (signature === 0x04034b50) {
84+
lying.writeUInt32LE(1000, offset + 22)
85+
}
86+
}
87+
88+
await expect(new DocParser().parseBuffer(lying)).rejects.toThrow(/do not match declared sizes/)
89+
expect(mockParseOfficeAsync).not.toHaveBeenCalled()
90+
expect(mockExtractRawText).not.toHaveBeenCalled()
91+
})
92+
6793
it('rejects a ZIP-shaped .doc whose central directory cannot be parsed', async () => {
6894
const buffer = Buffer.alloc(64)
6995
buffer.writeUInt32LE(0x04034b50, 0)

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

Lines changed: 88 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,47 @@ async function buildZip(
3030
})
3131
}
3232

33+
const CENTRAL_DIRECTORY_HEADER_SIGNATURE = 0x02014b50
34+
const LOCAL_FILE_HEADER_SIGNATURE = 0x04034b50
35+
36+
/**
37+
* Rewrite every declared uncompressed size — in both the central directory and
38+
* the local file headers — so the archive under-reports how much it expands to.
39+
* This is the bypass a declared-size-only check cannot see. Zero-length records
40+
* (JSZip emits a stored directory entry per folder) are left alone so the
41+
* archive stays well-formed apart from the lie under test.
42+
*/
43+
function underDeclareSizes(source: Buffer, declared: number): Buffer {
44+
const buffer = Buffer.from(source)
45+
for (let offset = 0; offset + 30 <= buffer.length; offset++) {
46+
const signature = buffer.readUInt32LE(offset)
47+
if (signature === CENTRAL_DIRECTORY_HEADER_SIGNATURE) {
48+
if (buffer.readUInt32LE(offset + 24) !== 0) {
49+
buffer.writeUInt32LE(declared, offset + 24)
50+
}
51+
} else if (signature === LOCAL_FILE_HEADER_SIGNATURE) {
52+
if (buffer.readUInt32LE(offset + 22) !== 0) {
53+
buffer.writeUInt32LE(declared, offset + 22)
54+
}
55+
}
56+
}
57+
return buffer
58+
}
59+
60+
/** Overwrite the compression method on every non-empty central-directory record. */
61+
function setCompressionMethod(source: Buffer, method: number): Buffer {
62+
const buffer = Buffer.from(source)
63+
for (let offset = 0; offset + 46 <= buffer.length; offset++) {
64+
if (
65+
buffer.readUInt32LE(offset) === CENTRAL_DIRECTORY_HEADER_SIGNATURE &&
66+
buffer.readUInt32LE(offset + 24) !== 0
67+
) {
68+
buffer.writeUInt16LE(method, offset + 10)
69+
}
70+
}
71+
return buffer
72+
}
73+
3374
describe('assertOoxmlArchiveWithinLimits', () => {
3475
it('accepts a well-formed archive within limits', async () => {
3576
const buffer = await buildZip({ 'word/document.xml': '<xml>hello world</xml>' })
@@ -108,6 +149,53 @@ describe('assertOoxmlArchiveWithinLimits', () => {
108149
expect(() => assertOoxmlArchiveWithinLimits(tampered)).toThrow(ZipBombError)
109150
})
110151

152+
it('rejects an archive that under-declares its uncompressed size', async () => {
153+
// The declared sizes put this archive far under both limits, so only
154+
// inflating it reveals that it actually expands ~200x further.
155+
const honest = await buildZip({ 'word/document.xml': 'A'.repeat(200_000) })
156+
const lying = underDeclareSizes(honest, 1000)
157+
158+
expect(() => assertOoxmlArchiveWithinLimits(lying, HIGH_LIMITS)).toThrow(ZipBombError)
159+
expect(() => assertOoxmlArchiveWithinLimits(lying, HIGH_LIMITS)).toThrow(
160+
/inflates beyond the 1000 bytes it declares/
161+
)
162+
})
163+
164+
it('still accepts the same archive when its declared sizes are honest', async () => {
165+
const honest = await buildZip({ 'word/document.xml': 'A'.repeat(200_000) })
166+
expect(() => assertOoxmlArchiveWithinLimits(honest, HIGH_LIMITS)).not.toThrow()
167+
})
168+
169+
it('rejects a stored entry whose declared size does not match its payload', async () => {
170+
const zip = new JSZip()
171+
zip.file('document.xml', 'A'.repeat(50_000))
172+
const stored = (await zip.generateAsync({
173+
type: 'nodebuffer',
174+
compression: 'STORE',
175+
})) as Buffer
176+
177+
expect(() =>
178+
assertOoxmlArchiveWithinLimits(underDeclareSizes(stored, 10), HIGH_LIMITS)
179+
).toThrow(/stored entry declares 10 bytes but holds 50000/)
180+
})
181+
182+
it('rejects an entry using a compression method the parsers cannot read', async () => {
183+
const buffer = await buildZip({ 'word/document.xml': '<xml>hello</xml>' })
184+
expect(() =>
185+
assertOoxmlArchiveWithinLimits(setCompressionMethod(buffer, 12), HIGH_LIMITS)
186+
).toThrow(/unsupported compression method 12/)
187+
})
188+
189+
it('accepts a multi-entry archive whose entries all inflate to what they declare', async () => {
190+
const buffer = await buildZip({
191+
'[Content_Types].xml': '<?xml version="1.0"?><Types/>',
192+
'_rels/.rels': '<?xml version="1.0"?><Relationships/>',
193+
'word/document.xml': `<w:document>${'text '.repeat(5000)}</w:document>`,
194+
'word/styles.xml': `<w:styles>${'style '.repeat(2000)}</w:styles>`,
195+
})
196+
expect(() => assertOoxmlArchiveWithinLimits(buffer, HIGH_LIMITS)).not.toThrow()
197+
})
198+
111199
it('no-ops for buffers that are not ZIP archives', () => {
112200
const plaintext = Buffer.from('this is just plain text, not a zip archive at all')
113201
expect(() => assertOoxmlArchiveWithinLimits(plaintext)).not.toThrow()

0 commit comments

Comments
 (0)