Skip to content

Commit 37962a0

Browse files
authored
fix(file-parsers): guard OOXML parsers against decompression-bomb memory exhaustion (#5239)
* fix(file-parsers): guard OOXML parsers against decompression-bomb memory exhaustion Pre-inspect the ZIP central directory of xlsx/docx/pptx buffers and reject archives whose declared expanded size (>1 GiB) or compression ratio (>150x) exceeds safe bounds, before SheetJS/mammoth/officeparser inflate them. The existing pipeline only capped the compressed input (100 MB), which does not bound decompressed size, so a crafted zip bomb could expand to many GB and OOM the worker. * fix(file-parsers): fail closed on unverifiable ZIP-shaped OOXML archives Address review: the guard previously no-opped (fell through to the decompression library) whenever the central directory could not be parsed, and findEocdOffset accepted the first backward EOCD signature without checking it sat at the buffer tail. A crafted archive with a decoy EOCD or an unsupported directory layout could bypass the size limits. - findEocdOffset now requires the EOCD comment length to place the record exactly at the buffer tail, defeating decoy signatures planted in the trailing region. - assertOoxmlArchiveWithinLimits now fails closed: a ZIP-shaped buffer (local file header / EOCD magic) whose central directory cannot be parsed is rejected rather than passed through. Genuine non-ZIP inputs (legacy OLE .xls/.doc, plaintext) still no-op and defer to the downstream parser.
1 parent b8a197b commit 37962a0

5 files changed

Lines changed: 409 additions & 7 deletions

File tree

‎apps/sim/lib/file-parsers/docx-parser.ts‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ import { createLogger } from '@sim/logger'
33
import mammoth from 'mammoth'
44
import type { FileParseResult, FileParser } from '@/lib/file-parsers/types'
55
import { sanitizeTextForUTF8 } from '@/lib/file-parsers/utils'
6+
import { assertOoxmlArchiveWithinLimits } from '@/lib/file-parsers/zip-guard'
67

78
const logger = createLogger('DocxParser')
89

@@ -37,6 +38,8 @@ export class DocxParser implements FileParser {
3738
throw new Error('Empty buffer provided')
3839
}
3940

41+
assertOoxmlArchiveWithinLimits(buffer)
42+
4043
try {
4144
const result = await mammoth.extractRawText({ buffer })
4245

‎apps/sim/lib/file-parsers/pptx-parser.ts‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ import { readFile } from 'fs/promises'
33
import { createLogger } from '@sim/logger'
44
import type { FileParseResult, FileParser } from '@/lib/file-parsers/types'
55
import { sanitizeTextForUTF8 } from '@/lib/file-parsers/utils'
6+
import { assertOoxmlArchiveWithinLimits } from '@/lib/file-parsers/zip-guard'
67

78
const logger = createLogger('PptxParser')
89

@@ -35,6 +36,8 @@ export class PptxParser implements FileParser {
3536
throw new Error('Empty buffer provided')
3637
}
3738

39+
assertOoxmlArchiveWithinLimits(buffer)
40+
3841
let parseOfficeAsync
3942
try {
4043
const officeParser = await import('officeparser')

‎apps/sim/lib/file-parsers/xlsx-parser.ts‎

Lines changed: 10 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,10 @@
11
import { existsSync } from 'fs'
2+
import { readFile } from 'fs/promises'
23
import { createLogger } from '@sim/logger'
34
import * as XLSX from 'xlsx'
45
import type { FileParseResult, FileParser } from '@/lib/file-parsers/types'
56
import { sanitizeTextForUTF8 } from '@/lib/file-parsers/utils'
7+
import { assertOoxmlArchiveWithinLimits } from '@/lib/file-parsers/zip-guard'
68

79
const logger = createLogger('XlsxParser')
810

@@ -16,6 +18,10 @@ const CONFIG = {
1618
}
1719

1820
export class XlsxParser implements FileParser {
21+
/**
22+
* Read the file into a buffer and delegate to {@link parseBuffer} so the
23+
* decompression-bomb guard runs before SheetJS inflates the workbook.
24+
*/
1925
async parseFile(filePath: string): Promise<FileParseResult> {
2026
try {
2127
if (!filePath) {
@@ -28,13 +34,8 @@ export class XlsxParser implements FileParser {
2834

2935
logger.info(`Parsing XLSX file: ${filePath}`)
3036

31-
// Read with streaming option for large files
32-
const workbook = XLSX.readFile(filePath, {
33-
dense: true, // Use dense mode for better memory efficiency
34-
sheetStubs: false, // Don't create stub cells
35-
})
36-
37-
return this.processWorkbook(workbook)
37+
const buffer = await readFile(filePath)
38+
return this.parseBuffer(buffer)
3839
} catch (error) {
3940
logger.error('XLSX file parsing error:', error)
4041
throw new Error(`Failed to parse XLSX file: ${(error as Error).message}`)
@@ -52,6 +53,8 @@ export class XlsxParser implements FileParser {
5253
throw new Error('Empty buffer provided')
5354
}
5455

56+
assertOoxmlArchiveWithinLimits(buffer)
57+
5558
const workbook = XLSX.read(buffer, {
5659
type: 'buffer',
5760
dense: true, // Use dense mode for better memory efficiency
Lines changed: 123 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,123 @@
1+
/**
2+
* @vitest-environment node
3+
*/
4+
import JSZip from 'jszip'
5+
import { describe, expect, it } from 'vitest'
6+
import {
7+
assertOoxmlArchiveWithinLimits,
8+
type OoxmlSizeLimits,
9+
ZipBombError,
10+
} from '@/lib/file-parsers/zip-guard'
11+
12+
const HIGH_LIMITS: OoxmlSizeLimits = {
13+
maxTotalUncompressedBytes: 1024 * 1024 * 1024,
14+
maxCompressionRatio: 10_000,
15+
ratioCheckFloorBytes: 1024 * 1024 * 1024,
16+
}
17+
18+
async function buildZip(
19+
entries: Record<string, string>,
20+
options: { comment?: string } = {}
21+
): Promise<Buffer> {
22+
const zip = new JSZip()
23+
for (const [name, content] of Object.entries(entries)) {
24+
zip.file(name, content)
25+
}
26+
return zip.generateAsync({
27+
type: 'nodebuffer',
28+
compression: 'DEFLATE',
29+
comment: options.comment,
30+
})
31+
}
32+
33+
describe('assertOoxmlArchiveWithinLimits', () => {
34+
it('accepts a well-formed archive within limits', async () => {
35+
const buffer = await buildZip({ 'word/document.xml': '<xml>hello world</xml>' })
36+
expect(() => assertOoxmlArchiveWithinLimits(buffer, HIGH_LIMITS)).not.toThrow()
37+
})
38+
39+
it('rejects an archive whose declared expanded size exceeds the absolute cap', async () => {
40+
const buffer = await buildZip({ 'xl/worksheets/sheet1.xml': 'A'.repeat(200_000) })
41+
expect(() =>
42+
assertOoxmlArchiveWithinLimits(buffer, {
43+
maxTotalUncompressedBytes: 100_000,
44+
maxCompressionRatio: 10_000,
45+
ratioCheckFloorBytes: 1024 * 1024 * 1024,
46+
})
47+
).toThrow(ZipBombError)
48+
})
49+
50+
it('rejects an archive whose compression ratio exceeds the limit', async () => {
51+
const buffer = await buildZip({ 'xl/worksheets/sheet1.xml': 'A'.repeat(200_000) })
52+
expect(() =>
53+
assertOoxmlArchiveWithinLimits(buffer, {
54+
maxTotalUncompressedBytes: 1024 * 1024 * 1024,
55+
maxCompressionRatio: 5,
56+
ratioCheckFloorBytes: 1000,
57+
})
58+
).toThrow(ZipBombError)
59+
})
60+
61+
it('does not flag a small but highly compressible archive below the ratio floor', async () => {
62+
const buffer = await buildZip({ 'xl/worksheets/sheet1.xml': 'A'.repeat(200_000) })
63+
expect(() =>
64+
assertOoxmlArchiveWithinLimits(buffer, {
65+
maxTotalUncompressedBytes: 1024 * 1024 * 1024,
66+
maxCompressionRatio: 5,
67+
ratioCheckFloorBytes: 1024 * 1024 * 1024,
68+
})
69+
).not.toThrow()
70+
})
71+
72+
it('sums declared sizes across multiple entries', async () => {
73+
const buffer = await buildZip({
74+
'a.xml': 'A'.repeat(60_000),
75+
'b.xml': 'B'.repeat(60_000),
76+
})
77+
expect(() =>
78+
assertOoxmlArchiveWithinLimits(buffer, {
79+
maxTotalUncompressedBytes: 100_000,
80+
maxCompressionRatio: 10_000,
81+
ratioCheckFloorBytes: 1024 * 1024 * 1024,
82+
})
83+
).toThrow(ZipBombError)
84+
})
85+
86+
it('accepts a well-formed archive that carries a trailing comment', async () => {
87+
const buffer = await buildZip(
88+
{ 'word/document.xml': '<xml>hello</xml>' },
89+
{ comment: 'generated by test' }
90+
)
91+
expect(() => assertOoxmlArchiveWithinLimits(buffer, HIGH_LIMITS)).not.toThrow()
92+
})
93+
94+
it('fails closed for a ZIP-shaped buffer whose central directory is unparseable', () => {
95+
const buffer = Buffer.alloc(64)
96+
buffer.writeUInt32LE(0x04034b50, 0) // local file header signature, no valid EOCD
97+
expect(() => assertOoxmlArchiveWithinLimits(buffer)).toThrow(ZipBombError)
98+
})
99+
100+
it('rejects a decoy EOCD signature that does not validate against the buffer tail', async () => {
101+
const realZip = await buildZip({ 'xl/worksheets/sheet1.xml': 'A'.repeat(200_000) })
102+
// A decoy EOCD (zeroed central directory) appended after the genuine archive
103+
// would, without tail validation, redirect the guard to an empty directory
104+
// and undercount the real entries.
105+
const decoy = Buffer.alloc(64)
106+
decoy.writeUInt32LE(0x06054b50, 0)
107+
const tampered = Buffer.concat([realZip, decoy])
108+
expect(() => assertOoxmlArchiveWithinLimits(tampered)).toThrow(ZipBombError)
109+
})
110+
111+
it('no-ops for buffers that are not ZIP archives', () => {
112+
const plaintext = Buffer.from('this is just plain text, not a zip archive at all')
113+
expect(() => assertOoxmlArchiveWithinLimits(plaintext)).not.toThrow()
114+
})
115+
116+
it('no-ops for buffers too small to contain an EOCD record', () => {
117+
expect(() => assertOoxmlArchiveWithinLimits(Buffer.from('PK'))).not.toThrow()
118+
})
119+
120+
it('no-ops for an empty buffer', () => {
121+
expect(() => assertOoxmlArchiveWithinLimits(Buffer.alloc(0))).not.toThrow()
122+
})
123+
})

0 commit comments

Comments
 (0)