Skip to content

fix(media): prevent drawtext filtergraph injection in add_text - #6734

Open
waleedlatif1 wants to merge 1 commit into
stagingfrom
fix/ffmpeg-drawtext-filtergraph-injection
Open

fix(media): prevent drawtext filtergraph injection in add_text#6734
waleedlatif1 wants to merge 1 commit into
stagingfrom
fix/ffmpeg-drawtext-filtergraph-injection

Conversation

@waleedlatif1

@waleedlatif1 waleedlatif1 commented Aug 15, 2026

Copy link
Copy Markdown
Collaborator

Summary

  • 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 rest of the caption was parsed as filtergraph syntax.
  • This let an authenticated caller inject filters — drawtext=textfile=<path> (arbitrary local-file read) or movie=filename=<url>:f=tty (read-SSRF) — rendering the target file's bytes or an HTTP response body into the returned video.
  • Fix: route the caption out-of-band. Write it to a file the operation owns and reference it via drawtext=textfile='<temp>' with expansion=none, so caption bytes never re-enter the filtergraph parser. Only the app-controlled temp path is escaped (for FFmpeg's two-level option parsing); expansion=none also disables %{...} expansion so the caption renders fully literally.
  • Added apps/sim/lib/media/ffmpeg.test.ts covering the two breakout classes (textfile file-read and movie= SSRF) plus a normal caption.

Type of Change

  • Bug fix (security)

Testing

  • Reproduced the injection against a real FFmpeg 8.0 binary and confirmed the fix neutralizes it end-to-end (payload stored as literal caption, never opened).
  • New unit tests assert the caption is never inlined into the filtergraph; verified they fail against the pre-fix code and pass after.
  • Existing ffmpeg tool tests, lint, and type-check all pass (no new errors).

Checklist

  • Code follows project style guidelines
  • Self-reviewed my changes
  • Tests added/updated and passing
  • No new warnings introduced
  • I confirm that I have read and agree to the terms outlined in the Contributor License Agreement (CLA)

@vercel

vercel Bot commented Aug 15, 2026 •
edited
Loading

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment
Project Deployment Actions Updated (UTC)
docs Skipped Skipped Aug 15, 2026 9:14pm

Request Review

@cursor

cursor Bot commented Aug 15, 2026 •
edited
Loading

Copy link
Copy Markdown

PR Summary

Medium Risk
Changes how untrusted text is passed into FFmpeg filtergraphs—a security-sensitive boundary; the fix is localized to add_text and covered by injection-focused tests.

Overview
Fixes a filtergraph injection vulnerability in the add_text FFmpeg path where caller caption text was inlined as drawtext=text='...'. A quote in the caption could break out of the quoted token and inject extra filters (e.g. textfile= local file read or movie= read-SSRF).

add_text now writes the caption to an operation-owned caption.txt in the temp dir and references it via textfile=caption.txt with expansion=none and reload=0, so caption bytes never enter the filtergraph parser. FFmpeg is started with cwd set to that temp dir so the path stays a simple relative filename (avoids quoting issues on absolute paths with apostrophes). The old escapeDrawtext helper is removed.

Adds ffmpeg.test.ts with mocked fluent-ffmpeg: normal captions use textfile=, and breakout payloads (textfile=/proc/self/environ, movie= SSRF) stay literal in the caption file and do not appear in the filter string.

Reviewed by Cursor Bugbot for commit e77c308. Configure here.

@greptile-apps

greptile-apps Bot commented Aug 15, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Greptile Summary

The PR prevents user-controlled captions from entering FFmpeg’s filtergraph parser by storing them in an operation-owned text file.

  • Uses a fixed relative caption.txt path resolved from the temporary working directory.
  • Disables drawtext expansion and adds regression coverage for local-file-read and read-SSRF payloads.

Confidence Score: 5/5

The PR appears safe to merge.

No blocking failure remains.

Important Files Changed
Filename Overview
apps/sim/lib/media/ffmpeg.ts Replaces unsafe inline caption interpolation with an operation-owned text file and disables drawtext expansion.
apps/sim/lib/media/ffmpeg.test.ts Adds focused regression tests confirming normal and malicious captions remain outside the generated filtergraph.

Sequence Diagram

 
sequenceDiagram
  participant Caller
  participant AddText as addText
  participant Temp as Temporary directory
  participant FFmpeg
  Caller->>AddText: Caption and video
  AddText->>Temp: Write caption.txt
  AddText->>FFmpeg: "drawtext=textfile=caption.txt, cwd=temp dir"
  FFmpeg->>Temp: Read literal caption bytes
  FFmpeg-->>Caller: Rendered video
Loading

Reviews (3): Last reviewed commit: "fix(media): prevent drawtext filtergraph..." | Re-trigger Greptile

@waleedlatif1
waleedlatif1 force-pushed the fix/ffmpeg-drawtext-filtergraph-injection branch from 5676385 to 97732f5 Compare August 15, 2026 19:01
@waleedlatif1

waleedlatif1 commented Aug 15, 2026

Copy link
Copy Markdown
Collaborator Author

@greptile

@waleedlatif1

waleedlatif1 commented Aug 15, 2026

Copy link
Copy Markdown
Collaborator Author

@cursor review

cursor[bot]
cursor Bot reviewed Aug 15, 2026
View reviewed changes
Comment thread apps/sim/lib/media/ffmpeg.ts Outdated
@waleedlatif1
fix(media): prevent drawtext filtergraph injection in add_text
e77c308 
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=<path>` (arbitrary
local-file read) or `movie=filename=<url>: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.
@waleedlatif1
waleedlatif1 force-pushed the fix/ffmpeg-drawtext-filtergraph-injection branch from 97732f5 to e77c308 Compare August 15, 2026 21:14
@waleedlatif1

waleedlatif1 commented Aug 15, 2026

Copy link
Copy Markdown
Collaborator Author

@greptile

@waleedlatif1

waleedlatif1 commented Aug 15, 2026

Copy link
Copy Markdown
Collaborator Author

@cursor review

cursor[bot]
cursor Bot reviewed Aug 15, 2026
View reviewed changes

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Bugbot reviewed your changes and found no new issues!

Comment @cursor review or bugbot run to trigger another review on this PR

Reviewed by Cursor Bugbot for commit e77c308. Configure here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@cursor cursor[bot] cursor[bot] left review comments

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@waleedlatif1