fix(files): escape user filenames in the Content-Disposition header

apps/sim/app/api/files/utils.test.ts

@@ -1,5 +1,10 @@
 import { describe, expect, it } from 'vitest'
-import { createFileResponse, extractFilename, findLocalFile } from '@/app/api/files/utils'
+import {
+  createFileResponse,
+  encodeFilenameForHeader,
+  extractFilename,
+  findLocalFile,
+} from '@/app/api/files/utils'
 
 describe('extractFilename', () => {
   describe('legitimate file paths', () => {
@@ -317,6 +322,62 @@ describe('extractFilename', () => {
       })
     })
 
+    /**
+     * `originalName` is attacker-controlled — it only rejects path separators — so a
+     * quote can reach the header and close the quoted parameter early. RFC 6266 tells
+     * clients to prefer `filename*`, so an injected one decides the name the file
+     * lands under on disk regardless of what the product UI displayed.
+     */
+    describe('encodeFilenameForHeader parameter injection', () => {
+      it('neutralizes a quote that would close the quoted filename parameter', () => {
+        expect(encodeFilenameForHeader(`report.pdf"; filename*=UTF-8''invoice.html`)).toBe(
+          `filename="report.pdf__ filename*=UTF-8''invoice.html"; filename*=UTF-8''report.pdf%22%3B%20filename%2A%3DUTF-8%27%27invoice.html`
+        )
+      })
+
+      it('neutralizes the same injection on the non-ascii branch', () => {
+        expect(encodeFilenameForHeader(`repört.pdf"; filename*=UTF-8''invoice.html`)).toBe(
+          `filename="rep_rt.pdf__ filename*=UTF-8''invoice.html"; filename*=UTF-8''rep%C3%B6rt.pdf%22%3B%20filename%2A%3DUTF-8%27%27invoice.html`
+        )
+      })
+
+      it('emits exactly one filename* parameter, holding the real name', () => {
+        const name = `report.pdf"; filename*=UTF-8''invoice.html`
+        const header = encodeFilenameForHeader(name)
+        // Strip the quoted value: text inside it is inert, so only what follows counts.
+        const parameters = `${header.slice(0, header.indexOf('filename="'))}${header.slice(header.lastIndexOf('"') + 1)}`
+        expect(parameters.match(/filename\*=/g)).toHaveLength(1)
+        // The one surviving filename* is the real name, not the injected one.
+        expect(decodeURIComponent(parameters.split(`filename*=UTF-8''`)[1])).toBe(name)
+      })
+
+      it('percent-encodes an apostrophe so it cannot desync the ext-value delimiter', () => {
+        const header = encodeFilenameForHeader("it's a café.pdf")
+        expect(header.split(`filename*=UTF-8''`)[1]).toBe('it%27s%20a%20caf%C3%A9.pdf')
+      })
+
+      it('encodes control characters that would otherwise be an invalid header value', () => {
+        const header = encodeFilenameForHeader('report\r\nX-Injected: 1.pdf')
+        expect(header).toBe(
+          `filename="report__X-Injected: 1.pdf"; filename*=UTF-8''report%0D%0AX-Injected%3A%201.pdf`
+        )
+        expect(
+          () =>
+            new Response('data', { headers: { 'Content-Disposition': `attachment; ${header}` } })
+        ).not.toThrow()
+      })
+
+      it('leaves an ordinary ascii filename byte-identical', () => {
+        expect(encodeFilenameForHeader('quarterly-report (final).pdf')).toBe(
+          'filename="quarterly-report (final).pdf"'
+        )
+      })
+
+      it('strips the directory prefix before encoding', () => {
+        expect(encodeFilenameForHeader('workspace/abc/report.pdf')).toBe('filename="report.pdf"')
+      })
+    })
+
     describe('Content Security Policy', () => {
       it('should include CSP header only for SVG responses', () => {
         const svgResponse = createFileResponse({

apps/sim/app/api/files/utils.ts

@@ -209,18 +209,47 @@ function getSecureFileHeaders(filename: string, originalContentType: string) {
   }
 }
 
+/**
+ * Percent-encode a filename as an RFC 8187 `ext-value`.
+ *
+ * `encodeURIComponent` alone is not enough: it leaves `'`, `(`, `)` and `*` raw, and
+ * none of those are `attr-char`. The apostrophe is the specific hazard — it is the
+ * delimiter in `UTF-8''name`, so a filename like `it's.pdf` would emit a third `'`
+ * and desync the parser.
+ */
+function encodeExtValue(filename: string): string {
+  return encodeURIComponent(filename).replace(
+    /['()*]/g,
+    (char) => `%${char.charCodeAt(0).toString(16).toUpperCase()}`
+  )
+}
+
+/**
+ * Build the `filename` parameters for a Content-Disposition header.
+ *
+ * The name is attacker-controlled (it is the user's `originalName`), so it can never
+ * be interpolated raw: a `"` closes the quoted-string early and everything after it
+ * is parsed as further parameters. An injected `filename*` is the payload that
+ * matters, because RFC 6266 tells clients to prefer `filename*` over `filename` —
+ * so the attacker's value wins and the download lands under a name the product UI
+ * never showed. Both parameters are therefore always emitted from sanitized input:
+ * the quoted form keeps only printable ASCII minus `"` and `\`, and the `filename*`
+ * form is fully percent-encoded.
+ *
+ * `;` is neutralized too, even though a quoted string may legally contain one: the
+ * quoted parameter exists as the fallback for clients that do not implement
+ * `filename*`, and those are the same clients liable to split parameters on a bare
+ * `;` without honouring the quoting. The exact name still survives in `filename*`.
+ */
 export function encodeFilenameForHeader(storageKey: string): string {
   const filename = storageKey.split('/').pop() || storageKey
-
-  const hasNonAscii = /[^\x00-\x7F]/.test(filename)
-
-  if (!hasNonAscii) {
+  const asciiSafe = filename.replace(/[^\x20-\x7E]/g, '_').replace(/["\\;]/g, '_')
+  // Unchanged input proves the name is printable ASCII with no `"` or `\`, so the
+  // quoted form alone is both safe and sufficient — `filename*` buys nothing here.
+  if (asciiSafe === filename) {
     return `filename="${filename}"`
   }
-
-  const encodedFilename = encodeURIComponent(filename)
-  const asciiSafe = filename.replace(/[^\x00-\x7F]/g, '_')
-  return `filename="${asciiSafe}"; filename*=UTF-8''${encodedFilename}`
+  return `filename="${asciiSafe}"; filename*=UTF-8''${encodeExtValue(filename)}`
 }
 
 export function createFileResponse(file: FileResponse): NextResponse {