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.

Author: waleedlatif1

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

@@ -15,12 +15,19 @@ const HIGH_LIMITS: OoxmlSizeLimits = {
   ratioCheckFloorBytes: 1024 * 1024 * 1024,
 }
 
-async function buildZip(entries: Record<string, string>): Promise<Buffer> {
+async function buildZip(
+  entries: Record<string, string>,
+  options: { comment?: string } = {}
+): Promise<Buffer> {
   const zip = new JSZip()
   for (const [name, content] of Object.entries(entries)) {
     zip.file(name, content)
   }
-  return zip.generateAsync({ type: 'nodebuffer', compression: 'DEFLATE' })
+  return zip.generateAsync({
+    type: 'nodebuffer',
+    compression: 'DEFLATE',
+    comment: options.comment,
+  })
 }
 
 describe('assertOoxmlArchiveWithinLimits', () => {
@@ -76,6 +83,31 @@ describe('assertOoxmlArchiveWithinLimits', () => {
     ).toThrow(ZipBombError)
   })
 
+  it('accepts a well-formed archive that carries a trailing comment', async () => {
+    const buffer = await buildZip(
+      { 'word/document.xml': '<xml>hello</xml>' },
+      { comment: 'generated by test' }
+    )
+    expect(() => assertOoxmlArchiveWithinLimits(buffer, HIGH_LIMITS)).not.toThrow()
+  })
+
+  it('fails closed for a ZIP-shaped buffer whose central directory is unparseable', () => {
+    const buffer = Buffer.alloc(64)
+    buffer.writeUInt32LE(0x04034b50, 0) // local file header signature, no valid EOCD
+    expect(() => assertOoxmlArchiveWithinLimits(buffer)).toThrow(ZipBombError)
+  })
+
+  it('rejects a decoy EOCD signature that does not validate against the buffer tail', async () => {
+    const realZip = await buildZip({ 'xl/worksheets/sheet1.xml': 'A'.repeat(200_000) })
+    // A decoy EOCD (zeroed central directory) appended after the genuine archive
+    // would, without tail validation, redirect the guard to an empty directory
+    // and undercount the real entries.
+    const decoy = Buffer.alloc(64)
+    decoy.writeUInt32LE(0x06054b50, 0)
+    const tampered = Buffer.concat([realZip, decoy])
+    expect(() => assertOoxmlArchiveWithinLimits(tampered)).toThrow(ZipBombError)
+  })
+
   it('no-ops for buffers that are not ZIP archives', () => {
     const plaintext = Buffer.from('this is just plain text, not a zip archive at all')
     expect(() => assertOoxmlArchiveWithinLimits(plaintext)).not.toThrow()

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

@@ -15,6 +15,7 @@ const logger = createLogger('ZipBombGuard')
  * compression ratio exceeds a safe threshold — without decompressing anything.
  */
 
+const LOCAL_FILE_HEADER_SIGNATURE = 0x04034b50
 const EOCD_SIGNATURE = 0x06054b50
 const ZIP64_EOCD_LOCATOR_SIGNATURE = 0x07064b50
 const ZIP64_EOCD_SIGNATURE = 0x06064b50
@@ -58,14 +59,36 @@ export class ZipBombError extends Error {
   }
 }
 
+/**
+ * Whether the buffer is shaped like a ZIP archive — i.e. begins with a local
+ * file header (the leading signature of every non-empty ZIP, and thus every
+ * OOXML document) or with the EOCD signature of an empty archive. Used to fail
+ * closed: a ZIP-shaped buffer the guard cannot parse must be rejected rather
+ * than handed to a decompression library.
+ */
+function isZipShaped(buffer: Buffer): boolean {
+  if (buffer.length < 4) {
+    return false
+  }
+  const signature = buffer.readUInt32LE(0)
+  return signature === LOCAL_FILE_HEADER_SIGNATURE || signature === EOCD_SIGNATURE
+}
+
 /**
  * Locate the End Of Central Directory record by scanning backwards from the end
- * of the buffer (it sits within the trailing 22 + comment bytes).
+ * of the buffer (it sits within the trailing 22 + comment bytes). A candidate
+ * is only accepted when its declared comment length places the record exactly
+ * at the buffer tail, so a decoy EOCD signature planted in the comment region
+ * cannot redirect the guard to a smaller, attacker-chosen central directory.
  */
 function findEocdOffset(buffer: Buffer): number {
   const minStart = Math.max(0, buffer.length - EOCD_MIN_SIZE - MAX_EOCD_COMMENT_SIZE)
   for (let offset = buffer.length - EOCD_MIN_SIZE; offset >= minStart; offset--) {
-    if (buffer.readUInt32LE(offset) === EOCD_SIGNATURE) {
+    if (buffer.readUInt32LE(offset) !== EOCD_SIGNATURE) {
+      continue
+    }
+    const commentLength = buffer.readUInt16LE(offset + 20)
+    if (offset + EOCD_MIN_SIZE + commentLength === buffer.length) {
       return offset
     }
   }
@@ -198,15 +221,26 @@ function sumDeclaredUncompressedSize(buffer: Buffer, abortAboveBytes: number): n
  * Reject an OOXML archive whose declared expanded size or compression ratio
  * exceeds safe bounds, before any decompression library materializes it.
  *
- * No-op for inputs that are not parseable ZIP archives — those are handled by
- * the downstream parser's own validation and fallbacks.
+ * Fails closed: a ZIP-shaped buffer whose central directory cannot be parsed is
+ * rejected rather than passed through, so a malformed archive that a downstream
+ * library still inflates cannot bypass the guard. Genuinely non-ZIP inputs
+ * (legacy OLE `.xls`/`.doc`, misidentified plaintext) no-op and defer to the
+ * downstream parser's own validation and fallbacks.
  */
 export function assertOoxmlArchiveWithinLimits(
   buffer: Buffer,
   limits: OoxmlSizeLimits = DEFAULT_OOXML_SIZE_LIMITS
 ): void {
   const totalUncompressed = sumDeclaredUncompressedSize(buffer, limits.maxTotalUncompressedBytes)
   if (totalUncompressed === null) {
+    if (isZipShaped(buffer)) {
+      logger.warn('Rejected ZIP-shaped archive: central directory could not be parsed', {
+        compressedBytes: buffer.length,
+      })
+      throw new ZipBombError(
+        'Unable to inspect ZIP central directory; refusing to parse an unverifiable ZIP-shaped archive'
+      )
+    }
     return
   }