fix(subtitle-mcp): validate mediaUrl scheme before FFmpeg to prevent protocol/argument injection

[andesyteoss]

Closes #579

Vulnerability

The generateSubtitle MCP tool in project/aitoearn-backend/apps/aitoearn-ai/src/core/agent/mcp/subtitle.mcp.ts forwards a caller-controlled mediaUrl straight into ffmpeg -i without checking the URL scheme. FFmpeg speaks many protocols beyond http(s) — file:, concat:, subfile:, data:, pipe:, rtmp:, gopher:, srtp:, … — so an authenticated MCP caller (or a prompt-injectable LLM/agent connected to the MCP server) can turn the tool into an arbitrary local file reader and an SSRF gateway, and can also inject FFmpeg CLI flags via a leading -.

• CWE: CWE-78 (argument injection into an external program), CWE-918 (SSRF), CWE-22 (arbitrary file read)
• Sink: execa('ffmpeg', ['-i', mediaUrl, ...]) in extractAudioWithFFmpeg
• Source: mediaUrl from the generateSubtitle MCP tool. generateSubtitleSchema declares mediaUrl: z.string(), which does not constrain the scheme.

The value flows unchanged from the MCP tool handler → extractAudioWithFFmpeg(mediaUrl) → execa('ffmpeg', ['-i', mediaUrl, ...]). execa avoids a shell, so this is not classic shell injection — but FFmpeg itself dispatches on the URL scheme and treats a leading - as a flag.

Fix

Two commits' worth of change squashed into one small diff (+39 lines, single file):

1. New helper assertSafeMediaUrl(mediaUrl):
   • Parses via new URL(...) and requires protocol === 'http:' || 'https:'.
   • Rejects values starting with - (defence against argument injection).
2. Called both from the tool handler (so the caller gets a clean error) and from extractAudioWithFFmpeg (so the check cannot be bypassed by any future caller of that private method).
3. Adds -protocol_whitelist https,http,tls,tcp to the FFmpeg argv as defence-in-depth against protocol chaining inside playlists/HLS segments that FFmpeg would otherwise follow into file: or concat:.

Rationale: the tool is documented as taking a media URL, so restricting to http(s) matches its intended contract. -protocol_whitelist is FFmpeg's own supported mechanism for exactly this class of issue, and layering both gives us protection even if a future refactor loses the JS-level check.

Proof of concept

Against the pre-fix code, any caller that can reach the MCP tool can invoke:

{
  "name": "generateSubtitle",
  "arguments": {
    "mediaUrl": "file:///etc/passwd",
    "language": "en"
  }
}

FFmpeg opens /etc/passwd as input. Other payloads that reproduce the same class of bug pre-fix:

• concat:/etc/hosts|/etc/passwd — concatenate local files as the audio input
• subfile,,start,0,end,4096,,:/etc/shadow — byte-range read of a local file
• http://169.254.169.254/latest/meta-data/ — SSRF to cloud metadata
• gopher://internal-service:6379/_... — SSRF against internal Redis/etc.
• -y or -loglevel debug as the value of mediaUrl — argument injection into the FFmpeg command line

After the fix, every one of these is rejected before FFmpeg is spawned with Invalid mediaUrl protocol "…" / Invalid mediaUrl: must not start with "-", and even if a future code path skipped assertSafeMediaUrl, -protocol_whitelist https,http,tls,tcp would still block file:/concat:/subfile:/gopher:/data:/pipe:.

Testing

• Verified statically that both call sites (extractAudioWithFFmpeg and the tool handler at the generateSubtitleSchema.shape registration) now pass through assertSafeMediaUrl.
• Manually walked the URL parser: new URL('file:///etc/passwd').protocol === 'file:', new URL('concat:a|b').protocol === 'concat:', new URL('http://x/').protocol === 'http:' — the check correctly accepts only http:/https:.
• Confirmed leading-dash guard by checking that new URL('-y') throws (so it's rejected at the parse step too, but the explicit dash guard is retained for clarity and to defend against any downstream code that ever accepts non-URL strings).
• Diff scope confirmed minimal: git diff origin/main..HEAD --stat → 1 file changed, 39 insertions(+).

Adversarial review

Before submitting, we tried to disprove this finding. Considered:

• Is the endpoint already gated in a way that makes it a non-issue? The MCP server is designed to be attached to LLM agents / Cursor / Claude Desktop per the project README. Any user who wires it up is a valid caller, and prompt injection against those agents is a realistic threat model — this isn't an "attacker already has RCE" situation.
• Does execa (no-shell) already prevent this? No. execa prevents shell metacharacter injection, but the vulnerability is inside FFmpeg's own URL-protocol dispatcher, which runs regardless of how the argv was constructed.
• Does the Zod schema constrain the URL? No — z.string() accepts any string, including file:///… and -y.
• Is -protocol_whitelist alone sufficient? It closes the FFmpeg-side attack surface but not the leading-- argument-injection case, and it also allows attackers to still pass e.g. http://internal/ SSRF. The URL-parse check is what enforces http(s) semantics; -protocol_whitelist is the belt to that suspenders.

We couldn't find a mitigation already in the codebase that would prevent exploitation, so the fix is warranted.

[gemini-code-assist]

Caution

The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased.