How Command Injection Happens in Node.js child_process and How to Fix It

Command Injection in Node.js: How a Trash Function Became a Remote Code Execution Gateway

What Happened

In a Node.js server application, a seemingly innocent feature—moving files to trash across different operating systems—contained a critical command injection vulnerability. The trashPath() function in server.js was constructing shell commands using template literals with user-supplied file paths, allowing attackers to inject arbitrary commands that would execute with the application's privileges.

The vulnerability existed at line 454 in server.js, where the code was building OS-specific commands like this:

// VULNERABLE CODE (macOS)
cmd = `osascript -e 'on run argv' -e 'tell application "Finder" to delete (POSIX file (item 1 of argv) as alias)' -e 'end run' ${shellQuote(target)}`;
exec(cmd, (err) => { /* ... */ });

This is a textbook command injection vulnerability. Even though shellQuote() was used, it provides only surface-level protection and can be bypassed.

Introduction: The Trap of String-Based Command Construction

The trashPath() function handles file deletion requests—a critical operation that requires user input (the file path to delete). The developers understood the risk enough to use shellQuote() for escaping, but this approach is fundamentally flawed:

1. Shell escaping is fragile: Different shells interpret escape sequences differently. What's safe in bash might not be safe in zsh or other shells.
2. Template literals encourage concatenation: Mixing code and data in a single string makes it easy to accidentally forget escaping somewhere.
3. The shell itself is the attack surface: Even with perfect escaping, passing user input to a shell command interpreter is higher risk than necessary.

The real vulnerability wasn't just the escaping—it was the architectural choice to use exec(), which spawns /bin/sh and interprets the entire string as a shell command.

The Vulnerability Explained: Multiple Paths to Code Execution

Let's examine the vulnerable patterns in detail. The code had three OS-specific implementations, all using exec():

macOS Implementation (Vulnerable):

cmd = `osascript -e 'on run argv' -e 'tell application "Finder" to delete (POSIX file (item 1 of argv) as alias)' -e 'end run' ${shellQuote(target)}`;
exec(cmd, (err) => { /* ... */ });

Windows Implementation (Vulnerable):

const ps = target.replace(/'/g, "''");
cmd = `powershell -NoProfile -Command "Add-Type -AssemblyName Microsoft.VisualBasic; [Microsoft.VisualBasic.FileIO.FileSystem]::${method}('${ps}','OnlyErrorDialogs','SendToRecycleBin')"`;
exec(cmd, (err) => { /* ... */ });

Linux Implementation (Vulnerable):

cmd = `gio trash ${shellQuote(target)} || trash-put ${shellQuote(target)} || trash ${shellQuote(target)}`;
exec(cmd, (err) => { /* ... */ });

Attack Scenario: Real-World Exploitation

Consider an attacker uploading a file with this name:

'; rm -rf / #.txt

When passed through shellQuote(), it might become:

'; rm -rf / #.txt

If there's any bypass in the quoting logic (and these have been discovered in shellQuote() implementations), the final command becomes:

osascript ... ''; rm -rf / #.txt'

The shell would interpret this as two commands:
1. The intended osascript command
2. The injected rm -rf / command

On a Linux system, this could delete the entire filesystem. On any system, it could exfiltrate data, install backdoors, or compromise the application.

Why shellQuote() Isn't Enough

The shellQuote() function attempts to quote arguments for shell safety, but:
- It relies on the shell's quoting behavior being consistent
- Bugs in shellQuote() implementations have led to bypasses (see Bash bug CVE-2014-6271)
- It doesn't prevent all injection vectors, especially in complex command chains with pipes and logical operators

The Fix: From exec() to execFile()

The fix makes a fundamental architectural change: eliminate the shell entirely. Instead of constructing a single command string and passing it to /bin/sh, the code now uses execFile() with separate argument arrays.

Before (Vulnerable):

let cmd;
if (PLATFORM === 'darwin') {
  cmd = `osascript -e 'on run argv' -e 'tell application "Finder" to delete (POSIX file (item 1 of argv) as alias)' -e 'end run' ${shellQuote(target)}`;
} else if (PLATFORM === 'win32') {
  const ps = target.replace(/'/g, "''");
  cmd = `powershell -NoProfile -Command "Add-Type -AssemblyName Microsoft.VisualBasic; [Microsoft.VisualBasic.FileIO.FileSystem]::${method}('${ps}','OnlyErrorDialogs','SendToRecycleBin')"`;
} else {
  cmd = `gio trash ${shellQuote(target)} || trash-put ${shellQuote(target)} || trash ${shellQuote(target)}`;
}
exec(cmd, (err) => {
  if (!err) return resolve({ ok: true });
  // ...
});

After (Secure):

let bin, args;
if (PLATFORM === 'darwin') {
  bin = 'osascript';
  args = ['-e', 'on run argv', '-e', 'tell application "Finder" to delete (POSIX file (item 1 of argv) as alias)', '-e', 'end run', target];
} else if (PLATFORM === 'win32') {
  bin = 'powershell';
  args = ['-NoProfile', '-Command', `Add-Type -AssemblyName Microsoft.VisualBasic; [Microsoft.VisualBasic.FileIO.FileSystem]::${method}('${target.replace(/'/g, "''")}','OnlyErrorDialogs','SendToRecycleBin')`];
} else {
  bin = 'sh';
  args = ['-c', 'gio trash "$1" || trash-put "$1" || trash "$1"', '--', target];
}
execFile(bin, args, (err) => {
  if (!err) return resolve({ ok: true });
  // ...
});

Key Security Improvements

1. No Shell Interpretation: execFile() executes the binary directly without spawning /bin/sh. The file path in the args array is never passed to a shell parser—it's passed directly to the program as a raw argument.

2. Arguments as Data: Each element in the args array is treated as a separate argument to the program. Shell metacharacters like ;, |, &&, and backticks have no special meaning—they're just data.

3. Attack Neutralization: The same malicious filename from our earlier example:
   '; rm -rf / #.txt

Is now passed directly to osascript as a literal argument. It cannot be interpreted as a command because there's no shell processing the string. osascript sees it as the path to delete, not as a command to execute.

1. No Escape Sequence Vulnerabilities: By avoiding string concatenation entirely, we eliminate the possibility of escaping bugs or bypass techniques.

Prevention & Best Practices

1. Always Use execFile() or spawn() Over exec()

The Node.js documentation explicitly warns about exec():

This function spawns a shell and runs a command within that shell, passing any arguments to the command.

For any user-controlled input, use execFile() or spawn():
- execFile(): Execute a file, returns the output
- spawn(): Stream-based execution, better for large outputs

// ✅ GOOD: execFile with arguments array
const { execFile } = require('child_process');
execFile('ls', ['-la', userProvidedPath], callback);

// ✅ GOOD: spawn for streaming
const { spawn } = require('child_process');
const child = spawn('tar', ['-czf', 'archive.tar.gz', userProvidedPath]);

// ❌ BAD: exec with string concatenation
exec(`ls -la ${userPath}`, callback);

// ❌ BAD: exec with template literals
exec(`tar -czf archive.tar.gz ${userPath}`, callback);

2. Separate Code from Data

Use argument arrays to keep commands (code) separate from user input (data):

// ❌ WRONG: Code and data mixed
const cmd = `convert ${imagePath} -resize 100x100 ${outputPath}`;
exec(cmd, callback);

// ✅ RIGHT: Code and data separated
execFile('convert', [imagePath, '-resize', '100x100', outputPath], callback);

3. Validate Input Patterns

Even with execFile(), validate that user input matches expected patterns:

// Whitelist expected path patterns
if (!/^[\w\-./]+$/.test(userPath)) {
  throw new Error('Invalid path format');
}

// For file paths, resolve and check they're in expected directory
const resolvedPath = path.resolve(userPath);
const allowedDir = path.resolve('/allowed/directory');
if (!resolvedPath.startsWith(allowedDir)) {
  throw new Error('Path traversal attempt');
}

execFile('rm', [resolvedPath], callback);

4. Use Security Scanning Tools

Semgrep detected this vulnerability with the rule javascript.lang.security.detect-child-process.detect-child-process. Integrate this into your CI/CD:

# Add to your CI pipeline
semgrep --config=p/security-audit --json src/ > semgrep-results.json

Semgrep rule patterns to enforce:
- Flag any use of exec() with user input
- Flag shell pipeline operators in dynamically constructed commands
- Warn on string concatenation for command construction

Key Takeaways

• Never use exec() with user input: Template literals and string concatenation in shell commands are command injection vulnerabilities waiting to happen. The trashPath() function demonstrates exactly how "small" uses of exec() become critical security holes.

• execFile() eliminates the shell attack surface: By using execFile() with argument arrays, the file path in line 456 can never be interpreted as a shell command, regardless of its contents.

• shellQuote() is not a substitute for architecture: While the original code attempted to use shellQuote() for safety, this only adds a layer of obfuscation. The secure fix removes the need for escaping entirely by avoiding the shell.

• Arguments arrays are semantically secure: In the fixed code, the args array at line 443 passes the file path as a separate argument, not as part of the command string. This is a fundamental security improvement.

• Test your command injection fixes thoroughly: The PR verified that existing tests still pass with the new implementation. When migrating from exec() to execFile(), ensure cross-platform behavior is maintained (macOS, Windows, Linux all worked correctly with the new approach).

How Orbis AppSec Detected This

Source: HTTP request handling or file upload processing that eventually calls trashPath(p) with a user-controlled p parameter representing the file path to delete.

Sink: The exec(cmd, ...) call at line 454 in server.js, where cmd is a dynamically constructed string containing the unsanitized target variable derived from the function argument p.

Missing control: No validation that target is safe for shell interpretation. While shellQuote() was used for macOS and Linux paths, this function doesn't provide guaranteed protection against all injection patterns, and the architecture itself relies on shell escaping rather than eliminating shell interpretation.

CWE: CWE-78: Improper Neutralization of Special Elements used in an OS Command ('OS Command Injection')

Fix: Replace exec() with execFile() and pass file paths as separate elements in an args array instead of concatenating them into the command string. This eliminates shell interpretation of user input.

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 through exec() is one of the most dangerous vulnerabilities in Node.js applications because it grants attackers the full power of the shell with the application's privileges. The trashPath() function in server.js demonstrates how even well-intentioned security measures like shellQuote() can provide false confidence when the underlying architecture uses exec().

The fix—migrating to execFile() with argument arrays—is a best practice that should be applied across any Node.js application handling user input in child process calls. By treating the command (code) separately from user input (data), developers eliminate entire classes of injection vulnerabilities while making their code simpler and more maintainable.

When reviewing your own code, ask: "Could this user input reach exec() or a shell command string?" If the answer is yes, refactor to execFile() or spawn() with argument arrays. Your security posture will improve significantly with this single architectural change.

References

• CWE-78: Improper Neutralization of Special Elements used in an OS Command: https://cwe.mitre.org/data/definitions/78.html
• Node.js child_process Documentation: https://nodejs.org/api/child_process.html#child_process_child_process_execfile_file_args_options_callback
• OWASP Command Injection: https://owasp.org/www-community/attacks/Command_Injection
• Semgrep Rule - detect-child-process: https://semgrep.dev/r?q=detect-child-process
• GitHub PR - fix: sanitize child_process call in server.js: fix: sanitize child_process call in server.js...