From e77c3081bec6d4dc74ae00f94f89f2cf0f1418db Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Sat, 15 Aug 2026 11:52:53 -0700 Subject: [PATCH] fix(media): prevent drawtext filtergraph injection in add_text MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The add_text FFmpeg operation inlined the caller's caption into a single-quoted `drawtext=text='...'` filter option. FFmpeg's av_get_token copies bytes verbatim inside a single-quoted run, so a literal quote in the caption closed the quote and the remainder was parsed as filtergraph syntax. An attacker could inject `drawtext=textfile=` (arbitrary local-file read) or `movie=filename=:f=tty` (read-SSRF), rendering the target file bytes or HTTP response body into the returned video. Route the caption out-of-band: write it to a file the operation owns and reference it via `drawtext=textfile=caption.txt` with `expansion=none`, so the caption bytes never re-enter the filtergraph parser. The caption is referenced by a bare relative filename with FFmpeg's working directory set to the temp dir, because FFmpeg's tokenizer cannot round-trip a single quote inside a textfile= value — an absolute temp path would break add_text whenever os.tmpdir() contains a quote (e.g. a Windows profile). The working directory is passed via execve and never parsed as graph syntax, so any character in it is safe. --- apps/sim/lib/media/ffmpeg.test.ts | 116 ++++++++++++++++++++++++++++++ apps/sim/lib/media/ffmpeg.ts | 26 +++++-- 2 files changed, 136 insertions(+), 6 deletions(-) create mode 100644 apps/sim/lib/media/ffmpeg.test.ts diff --git a/apps/sim/lib/media/ffmpeg.test.ts b/apps/sim/lib/media/ffmpeg.test.ts new file mode 100644 index 00000000000..c004f06081b --- /dev/null +++ b/apps/sim/lib/media/ffmpeg.test.ts @@ -0,0 +1,116 @@ +/** + * @vitest-environment node + */ +import fs from 'node:fs' +import path from 'node:path' +import { beforeEach, describe, expect, it, vi } from 'vitest' + +const { capturedVideoFilters, capturedCaptions } = vi.hoisted(() => ({ + capturedVideoFilters: [] as string[], + capturedCaptions: [] as string[], +})) + +vi.mock('node:child_process', () => ({ + execSync: () => '/usr/bin/ffmpeg\n', +})) + +vi.mock('fluent-ffmpeg', () => { + const makeCommand = (cwd?: string) => { + const handlers: Record void> = {} + const cmd: Record = {} + const chain = (fn?: (arg: unknown) => void) => (arg?: unknown) => { + fn?.(arg) + return cmd + } + cmd.input = chain() + cmd.inputOptions = chain() + cmd.outputOptions = chain() + cmd.complexFilter = chain() + cmd.audioFilters = chain() + cmd.noVideo = chain() + cmd.setStartTime = chain() + cmd.setDuration = chain() + cmd.seekInput = chain() + cmd.frames = chain() + cmd.videoFilters = chain((arg) => { + const filter = String(arg) + capturedVideoFilters.push(filter) + // The caption is a bare relative filename resolved against the command's cwd (the + // temp dir). Read it back while it still exists to prove the raw caption never + // reached the filtergraph string. + const match = filter.match(/textfile=([^:]+)/) + if (match && cwd) { + capturedCaptions.push(fs.readFileSync(path.join(cwd, match[1]), 'utf-8')) + } + }) + cmd.on = (event: string, handler: (...args: unknown[]) => void) => { + handlers[event] = handler + return cmd + } + cmd.save = (outputPath: string) => { + fs.writeFileSync(outputPath, Buffer.from('stub-output')) + handlers.end?.() + return cmd + } + return cmd + } + const ffmpeg = ((_input?: unknown, options?: { cwd?: string }) => + makeCommand(options?.cwd)) as unknown as Record & (() => unknown) + ;(ffmpeg as Record).setFfmpegPath = () => {} + ;(ffmpeg as Record).ffprobe = ( + _path: string, + cb: (err: unknown, data: unknown) => void + ) => cb(null, { streams: [], format: {} }) + return { default: ffmpeg } +}) + +import { runFfmpegOperation } from '@/lib/media/ffmpeg' + +const videoInput = { + buffer: Buffer.from('fake-video-bytes'), + mimeType: 'video/mp4', + name: 'clip.mp4', +} + +describe('runFfmpegOperation add_text filtergraph injection', () => { + beforeEach(() => { + capturedVideoFilters.length = 0 + capturedCaptions.length = 0 + }) + + it('routes the caption through textfile= so it never becomes filtergraph syntax', async () => { + await runFfmpegOperation('add_text', [videoInput], { text: 'Hello World :) 100% done' }) + + expect(capturedVideoFilters).toHaveLength(1) + const filter = capturedVideoFilters[0] + expect(filter.startsWith('drawtext=textfile=')).toBe(true) + expect(filter).toContain(':expansion=none:') + // The caption text must not be inlined into the filter string. + expect(filter).not.toContain('Hello World') + }) + + it('neutralizes a breakout payload that would inject textfile=/proc/self/environ', async () => { + const payload = "x',drawtext=textfile=/proc/self/environ:x=10:y=10,drawtext=text=hi" + await runFfmpegOperation('add_text', [videoInput], { text: payload }) + + const filter = capturedVideoFilters[0] + // The whole graph is a single, app-authored drawtext reading our temp caption file. + expect(filter.startsWith('drawtext=textfile=')).toBe(true) + // None of the attacker's injected syntax leaks into the filtergraph string. + expect(filter).not.toContain('/proc/self/environ') + expect(filter).not.toContain('drawtext=text=hi') + // ...and the payload is stored verbatim as literal caption bytes instead. + expect(capturedCaptions).toEqual([payload]) + }) + + it('neutralizes a movie= read-SSRF breakout payload', async () => { + const payload = + "x'[d];movie=filename=http\\://169.254.169.254/latest:f=tty[m];[d][m]overlay=0:0" + await runFfmpegOperation('add_text', [videoInput], { text: payload }) + + const filter = capturedVideoFilters[0] + expect(filter).not.toContain('movie=') + expect(filter).not.toContain('169.254.169.254') + expect(capturedCaptions).toEqual([payload]) + }) +}) diff --git a/apps/sim/lib/media/ffmpeg.ts b/apps/sim/lib/media/ffmpeg.ts index bcfa0b6adf0..9cc94c84a19 100644 --- a/apps/sim/lib/media/ffmpeg.ts +++ b/apps/sim/lib/media/ffmpeg.ts @@ -169,10 +169,6 @@ const TEXT_POSITION: Record = { 'bottom-right': { x: 'w*0.95-text_w', y: 'h*0.86' }, } -function escapeDrawtext(text: string): string { - return text.replace(/\\/g, '\\\\').replace(/:/g, '\\:').replace(/'/g, "\\'").replace(/%/g, '\\%') -} - async function withTempDir(fn: (dir: string) => Promise): Promise { ensureFfmpeg() const dir = await fs.mkdtemp(path.join(os.tmpdir(), 'media-ffmpeg-')) @@ -480,8 +476,26 @@ async function addText( ): Promise { if (!options.text) throw new Error('add_text requires text') const pos = TEXT_POSITION[options.position || 'bottom'] || TEXT_POSITION.bottom + // Route the caption out-of-band through a file the operation owns; never inline it into + // the filtergraph. Inline escaping is not safe — FFmpeg's av_get_token copies bytes + // verbatim inside a single-quoted run, so a literal quote closes the quote and the rest + // of the caption is parsed as filtergraph syntax, injecting filters like + // `textfile=/proc/self/environ` or `movie=http\://...` (arbitrary local-file read + + // read-SSRF, CWE-88). `textfile=` renders the bytes literally and `expansion=none` + // disables drawtext's `%{...}` functions, so the caption can never re-enter the parser. + // + // Reference the caption by a bare relative filename and run FFmpeg with its working + // directory set to the temp dir. FFmpeg's filtergraph tokenizer cannot round-trip a + // single quote inside a `textfile=` value — it drops or mis-parses it — so embedding the + // absolute temp path would break add_text whenever `os.tmpdir()` contains a quote (e.g. a + // Windows profile like `C:\Users\O'Brien\...`). The working directory is handed to the + // process via execve, never parsed as filtergraph syntax, so any character in it is safe. + const captionFileName = 'caption.txt' + await fs.writeFile(path.join(dir, captionFileName), options.text, 'utf-8') const drawtext = [ - `text='${escapeDrawtext(options.text)}'`, + `textfile=${captionFileName}`, + 'expansion=none', + 'reload=0', 'fontcolor=white', 'fontsize=h/18', 'box=1', @@ -491,7 +505,7 @@ async function addText( `y=${pos.y}`, ].join(':') const outputPath = path.join(dir, 'out.mp4') - const command = ffmpeg(inputPath) + const command = ffmpeg(inputPath, { cwd: dir }) .videoFilters(`drawtext=${drawtext}`) .outputOptions(['-c:a', 'copy']) await runCommand(command, outputPath)