improvement(files): harden untrusted document preview and parsing (#6420)

* improvement(files): harden the docx preview renderer

* fix(files): tighten the OOXML archive size guard against parser OOM

* fix(files): apply the per-entry OOXML cap to every part, not just XML names

Author: waleedlatif1

apps/sim/app/workspace/[workspaceId]/files/components/file-viewer/docx-preview.tsx

@@ -4,7 +4,7 @@ import { memo, useCallback, useEffect, useRef, useState } from 'react'
 import { cn } from '@sim/emcn'
 import { createLogger } from '@sim/logger'
 import { toError } from '@sim/utils/errors'
-import { sanitizeRenderedHyperlinks } from '@/lib/core/security/url-safety'
+import { sanitizeRenderedHyperlinks, stripEmbeddedFrames } from '@/lib/core/security/url-safety'
 import type { WorkspaceFileRecord } from '@/lib/uploads/contexts/workspace'
 import { PREVIEW_LOADING_OVERLAY, PreviewError, resolvePreviewError } from './preview-shared'
 import { PreviewToolbar } from './preview-toolbar'
@@ -208,9 +208,11 @@ export const DocxPreview = memo(function DocxPreview({
           inWrapper: true,
           ignoreWidth: false,
           ignoreHeight: false,
+          renderAltChunks: false,
         })
         if (!cancelled && containerRef.current) {
           sanitizeRenderedHyperlinks(containerRef.current)
+          stripEmbeddedFrames(containerRef.current)
           applyPostRenderStyling()
           setHasRenderedPreview(true)
           setDocumentRenderVersion((version) => version + 1)

apps/sim/lib/core/security/url-safety.test.ts

@@ -2,7 +2,11 @@
  * @vitest-environment jsdom
  */
 import { describe, expect, it } from 'vitest'
-import { isAllowedExternalUrl, sanitizeRenderedHyperlinks } from '@/lib/core/security/url-safety'
+import {
+  isAllowedExternalUrl,
+  sanitizeRenderedHyperlinks,
+  stripEmbeddedFrames,
+} from '@/lib/core/security/url-safety'
 
 describe('isAllowedExternalUrl', () => {
   it('allows http, https, and mailto URLs', () => {
@@ -59,3 +63,21 @@ describe('sanitizeRenderedHyperlinks', () => {
     expect(anchor?.getAttribute('rel')).toBe('noopener noreferrer')
   })
 })
+
+describe('stripEmbeddedFrames', () => {
+  it('removes a srcdoc iframe and leaves sibling content intact', () => {
+    const container = document.createElement('div')
+    container.innerHTML =
+      '<p>page</p><iframe srcdoc="&lt;script&gt;fetch(1)&lt;/script&gt;"></iframe>'
+    stripEmbeddedFrames(container)
+    expect(container.querySelector('iframe')).toBeNull()
+    expect(container.querySelector('p')?.textContent).toBe('page')
+  })
+
+  it('removes object and embed plugin elements', () => {
+    const container = document.createElement('div')
+    container.innerHTML = '<object data="x"></object><embed src="x" />'
+    stripEmbeddedFrames(container)
+    expect(container.querySelectorAll('object, embed').length).toBe(0)
+  })
+})

apps/sim/lib/core/security/url-safety.ts

@@ -1,6 +1,6 @@
 /**
- * URL safety utilities for external hyperlinks/media in untrusted document content
- * (PPTX, DOCX, and other previews rendered into the app origin).
+ * Safety utilities for untrusted document content rendered into the app origin
+ * (PPTX, DOCX, and other previews).
  */
 
 const ALLOWED_PROTOCOLS = new Set(['http:', 'https:', 'mailto:'])
@@ -35,3 +35,14 @@ export function sanitizeRenderedHyperlinks(root: ParentNode): void {
     anchor.removeAttribute('href')
   }
 }
+
+/**
+ * Removes embedded browsing contexts from rendered document content. A renderer-injected
+ * `<iframe srcdoc>` inherits the app origin, so any script it carries runs with the
+ * victim's session. Defense in depth — renderers are also configured not to emit frames.
+ */
+export function stripEmbeddedFrames(root: ParentNode): void {
+  for (const element of root.querySelectorAll('iframe, object, embed')) {
+    element.remove()
+  }
+}

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

@@ -11,6 +11,7 @@ import {
 
 const HIGH_LIMITS: OoxmlSizeLimits = {
   maxTotalUncompressedBytes: 1024 * 1024 * 1024,
+  maxEntryUncompressedBytes: 1024 * 1024 * 1024,
   maxCompressionRatio: 10_000,
   ratioCheckFloorBytes: 1024 * 1024 * 1024,
 }
@@ -98,6 +99,7 @@ describe('assertOoxmlArchiveWithinLimits', () => {
     expect(() =>
       assertOoxmlArchiveWithinLimits(buffer, {
         maxTotalUncompressedBytes: 100_000,
+        maxEntryUncompressedBytes: 1024 * 1024 * 1024,
         maxCompressionRatio: 10_000,
         ratioCheckFloorBytes: 1024 * 1024 * 1024,
       })
@@ -109,6 +111,7 @@ describe('assertOoxmlArchiveWithinLimits', () => {
     expect(() =>
       assertOoxmlArchiveWithinLimits(buffer, {
         maxTotalUncompressedBytes: 1024 * 1024 * 1024,
+        maxEntryUncompressedBytes: 1024 * 1024 * 1024,
         maxCompressionRatio: 5,
         ratioCheckFloorBytes: 1000,
       })
@@ -120,6 +123,7 @@ describe('assertOoxmlArchiveWithinLimits', () => {
     expect(() =>
       assertOoxmlArchiveWithinLimits(buffer, {
         maxTotalUncompressedBytes: 1024 * 1024 * 1024,
+        maxEntryUncompressedBytes: 1024 * 1024 * 1024,
         maxCompressionRatio: 5,
         ratioCheckFloorBytes: 1024 * 1024 * 1024,
       })
@@ -131,13 +135,63 @@ describe('assertOoxmlArchiveWithinLimits', () => {
       'a.xml': 'A'.repeat(60_000),
       'b.xml': 'B'.repeat(60_000),
     })
+    // Each entry (60 KB) is under the per-entry cap; only the summed total trips
+    // the limit, so this must fail on the total branch, not the per-entry one.
     expect(() =>
       assertOoxmlArchiveWithinLimits(buffer, {
         maxTotalUncompressedBytes: 100_000,
+        maxEntryUncompressedBytes: 1024 * 1024 * 1024,
         maxCompressionRatio: 10_000,
         ratioCheckFloorBytes: 1024 * 1024 * 1024,
       })
-    ).toThrow(ZipBombError)
+    ).toThrow(/Decompressed size .* exceeds the maximum allowed/)
+  })
+
+  it('rejects an archive whose largest single entry exceeds the per-entry cap', async () => {
+    const buffer = await buildZip({ 'word/document.xml': 'A'.repeat(200_000) })
+    expect(() =>
+      assertOoxmlArchiveWithinLimits(buffer, {
+        maxTotalUncompressedBytes: 1024 * 1024 * 1024,
+        maxEntryUncompressedBytes: 100_000,
+        maxCompressionRatio: 10_000,
+        ratioCheckFloorBytes: 1024 * 1024 * 1024,
+      })
+    ).toThrow(/single entry's decompressed size .* exceeds the maximum allowed/)
+  })
+
+  it('applies the per-entry cap to a non-.xml part resolved through OPC relationships', async () => {
+    // The main document part is resolved via relationship target, not a fixed
+    // path, so a bomb under a `.bin` name is still DOM-parsed — the cap must not
+    // exempt it on filename.
+    const buffer = await buildZip({ 'word/document.bin': 'A'.repeat(200_000) })
+    expect(() =>
+      assertOoxmlArchiveWithinLimits(buffer, {
+        maxTotalUncompressedBytes: 1024 * 1024 * 1024,
+        maxEntryUncompressedBytes: 100_000,
+        maxCompressionRatio: 10_000,
+        ratioCheckFloorBytes: 1024 * 1024 * 1024,
+      })
+    ).toThrow(/single entry's decompressed size .* exceeds the maximum allowed/)
+  })
+
+  it('rejects a single part larger than the 64 MiB per-entry default before any parser sees it', async () => {
+    // A part declaring 70 MiB expanded passes the old 1 GiB ceiling but drives
+    // the parser's DOM past a modest heap. `underDeclareSizes` is reused in the
+    // over-declaring direction to set the declared size without allocating it.
+    const honest = await buildZip({ 'word/document.xml': 'A'.repeat(200_000) })
+    const oversized = underDeclareSizes(honest, 70 * 1024 * 1024)
+    expect(() => assertOoxmlArchiveWithinLimits(oversized)).toThrow(
+      /single entry's decompressed size .* exceeds the maximum allowed 67108864 bytes/
+    )
+  })
+
+  it('accepts an ordinary document under the default limits', async () => {
+    const buffer = await buildZip({
+      '[Content_Types].xml': '<?xml version="1.0"?><Types/>',
+      '_rels/.rels': '<?xml version="1.0"?><Relationships/>',
+      'word/document.xml': `<w:document>${'text '.repeat(5000)}</w:document>`,
+    })
+    expect(() => assertOoxmlArchiveWithinLimits(buffer)).not.toThrow()
   })
 
   it('accepts a well-formed archive that carries a trailing comment', async () => {
@@ -248,6 +302,7 @@ describe('assertOoxmlArchiveWithinLimits', () => {
     expect(() =>
       assertOoxmlArchiveWithinLimits(buffer, {
         maxTotalUncompressedBytes: 100_000,
+        maxEntryUncompressedBytes: 1024 * 1024 * 1024,
         maxCompressionRatio: 10_000,
         ratioCheckFloorBytes: 1024 * 1024 * 1024,
       })

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

@@ -12,8 +12,9 @@ const logger = createLogger('ZipBombGuard')
  * exhausting the worker and crashing the process with an OOM.
  *
  * This guard inspects the ZIP central directory (which records each entry's
- * declared uncompressed size) and rejects archives whose total expanded size or
- * compression ratio exceeds a safe threshold — without decompressing anything.
+ * declared uncompressed size) and rejects archives whose total expanded size,
+ * largest single entry, or compression ratio exceeds a safe threshold — without
+ * decompressing anything.
  */
 
 const LOCAL_FILE_HEADER_SIGNATURE = 0x04034b50
@@ -40,22 +41,30 @@ const DATA_DESCRIPTOR_FLAG = 0x0008
 export interface OoxmlSizeLimits {
   /** Hard ceiling on the summed declared uncompressed size of all entries. */
   maxTotalUncompressedBytes: number
+  /** Hard ceiling on any single entry's declared uncompressed size — the parser materializes an individual part (e.g. `document.xml`) into a DOM, or base64-embeds media, at a footprint many times the part size. */
+  maxEntryUncompressedBytes: number
   /** Maximum allowed expanded:compressed ratio across the whole archive. */
   maxCompressionRatio: number
   /** The ratio check only applies once the expanded size exceeds this floor, so small files are never flagged. */
   ratioCheckFloorBytes: number
 }
 
-const ONE_GIBIBYTE = 1024 * 1024 * 1024
 const ONE_HUNDRED_MEBIBYTES = 100 * 1024 * 1024
+const ONE_HUNDRED_FIFTY_MEBIBYTES = 150 * 1024 * 1024
+const SIXTY_FOUR_MEBIBYTES = 64 * 1024 * 1024
 
 /**
- * Defaults sized against the 100 MB compressed-input cap of the parse pipeline.
- * A legitimate Office document stays well under 1 GiB expanded; the bombs
- * described in the threat model expand to multiple gigabytes.
+ * The downstream parsers (mammoth, SheetJS, officeparser) build a full in-memory
+ * DOM/object graph whose peak heap is many times the XML size — measured at
+ * ~900 MB resident for 32 MB of expanded WordprocessingML, and mammoth then
+ * parses a second time for HTML. The old 1 GiB ceiling let a ~3.5 MB archive
+ * expand past what the process could hold and OOM it. The total and per-entry
+ * caps here keep a single parse's peak within a modest container's budget while
+ * still admitting all but pathologically large documents.
  */
 export const DEFAULT_OOXML_SIZE_LIMITS: OoxmlSizeLimits = {
-  maxTotalUncompressedBytes: ONE_GIBIBYTE,
+  maxTotalUncompressedBytes: ONE_HUNDRED_FIFTY_MEBIBYTES,
+  maxEntryUncompressedBytes: SIXTY_FOUR_MEBIBYTES,
   maxCompressionRatio: 150,
   ratioCheckFloorBytes: ONE_HUNDRED_MEBIBYTES,
 }
@@ -212,11 +221,25 @@ function readCentralDirectoryEntry(
   return { compressionMethod, compressedSize, uncompressedSize, localHeaderOffset }
 }
 
+interface DeclaredSizeStats {
+  /** Summed declared uncompressed size across the contiguous run of records. */
+  total: number
+  /** Largest single entry's declared uncompressed size. */
+  largestEntry: number
+}
+
 /**
- * Sum the declared uncompressed size of every central-directory entry. Returns
- * `null` when the buffer is not a parseable ZIP archive (e.g. legacy binary
- * `.xls`/`.doc`, or a misidentified plaintext file) so the caller can defer to
- * the downstream parser. Stops early once the running total exceeds the limit.
+ * Sum the declared uncompressed size of every central-directory entry and track
+ * the largest single entry. Returns `null` when the buffer is not a parseable
+ * ZIP archive (e.g. legacy binary `.xls`/`.doc`, or a misidentified plaintext
+ * file) so the caller can defer to the downstream parser. Stops early once the
+ * running total, or a single entry, exceeds the corresponding limit.
+ *
+ * The per-entry cap covers every entry, not just `.xml` parts: an OOXML part is
+ * resolved through OPC relationship targets (mammoth's `officeDocument`
+ * relationship, with `word/document.xml` only a fallback), so an arbitrarily
+ * named part is still deserialized into a DOM — a name-based exemption would let
+ * a bomb sail through under a `.bin` target.
  *
  * Like {@link readZipCentralDirectoryStats}, this charges the CONTIGUOUS run of
  * records rather than the EOCD's declared count. JSZip's `readCentralDir` loops
@@ -225,7 +248,10 @@ function readCentralDirectoryEntry(
  * would otherwise hide honestly-large entries from this cap while the parser
  * still expanded them.
  */
-function sumDeclaredUncompressedSize(buffer: Buffer, abortAboveBytes: number): number | null {
+function sumDeclaredUncompressedSize(
+  buffer: Buffer,
+  limits: OoxmlSizeLimits
+): DeclaredSizeStats | null {
   if (buffer.length < EOCD_MIN_SIZE) {
     return null
   }
@@ -241,6 +267,7 @@ function sumDeclaredUncompressedSize(buffer: Buffer, abortAboveBytes: number): n
   }
 
   let total = 0
+  let largestEntry = 0
   let counted = 0
   let cursor = location.offset
   while (
@@ -251,14 +278,18 @@ function sumDeclaredUncompressedSize(buffer: Buffer, abortAboveBytes: number): n
     const extraFieldLength = buffer.readUInt16LE(cursor + 30)
     const commentLength = buffer.readUInt16LE(cursor + 32)
 
-    total += readCentralDirectoryEntry(
+    const entryBytes = readCentralDirectoryEntry(
       buffer,
       cursor,
       fileNameLength,
       extraFieldLength
     ).uncompressedSize
-    if (total > abortAboveBytes) {
-      return total
+    total += entryBytes
+    if (entryBytes > largestEntry) {
+      largestEntry = entryBytes
+    }
+    if (total > limits.maxTotalUncompressedBytes || entryBytes > limits.maxEntryUncompressedBytes) {
+      return { total, largestEntry }
     }
 
     counted += 1
@@ -271,7 +302,7 @@ function sumDeclaredUncompressedSize(buffer: Buffer, abortAboveBytes: number): n
     return null
   }
 
-  return total
+  return { total, largestEntry }
 }
 
 /**
@@ -471,8 +502,8 @@ export function assertOoxmlArchiveWithinLimits(
   buffer: Buffer,
   limits: OoxmlSizeLimits = DEFAULT_OOXML_SIZE_LIMITS
 ): void {
-  const totalUncompressed = sumDeclaredUncompressedSize(buffer, limits.maxTotalUncompressedBytes)
-  if (totalUncompressed === null) {
+  const declared = sumDeclaredUncompressedSize(buffer, limits)
+  if (declared === null) {
     if (isZipShaped(buffer)) {
       logger.warn('Rejected ZIP-shaped archive: central directory could not be parsed', {
         compressedBytes: buffer.length,
@@ -484,6 +515,19 @@ export function assertOoxmlArchiveWithinLimits(
     return
   }
 
+  const { total: totalUncompressed, largestEntry } = declared
+
+  if (largestEntry > limits.maxEntryUncompressedBytes) {
+    logger.warn('Rejected OOXML archive: a single entry exceeds the per-entry limit', {
+      largestEntry,
+      maxEntryUncompressedBytes: limits.maxEntryUncompressedBytes,
+      compressedBytes: buffer.length,
+    })
+    throw new ZipBombError(
+      `A single entry's decompressed size (${largestEntry} bytes) exceeds the maximum allowed ${limits.maxEntryUncompressedBytes} bytes`
+    )
+  }
+
   if (totalUncompressed > limits.maxTotalUncompressedBytes) {
     logger.warn('Rejected OOXML archive: declared expanded size exceeds limit', {
       totalUncompressed,