How Command Injection Happens in Node.js Child Process Calls and How to Fix It
The Vulnerability in Context
In a Node.js library, a high-severity command injection vulnerability was discovered in index.js at line 8. The vulnerable code was calling execSync() with shell string interpolation on a user-controlled file path parameter, creating an exploit primitive that could allow attackers to execute arbitrary commands. This pattern is particularly dangerous in libraries because vulnerabilities affect all downstream consumers who depend on the package.
The specific problematic code looked like this:
const { execSync } = require('child_process')
function run (script) {
try {
const stdout = execSync(`php ${path.join(__dirname, script)}`)
return stdout.toString()
} catch (err) {
throw new Error(`PHP process exited with code ${err.status}`)
}
}
Notice the backtick string interpolation on line 9: execSync(`php ${path.join(__dirname, script)}`). This approach passes the entire command as a shell string, which means the shell will interpret special characters in the script parameter.
The Vulnerability Explained
Why This Code Is Dangerous
When you use execSync() with a template string, Node.js executes the command through a shell by default. This means any shell metacharacters in the script parameter will be interpreted as shell syntax, not as literal filename characters.
Example attack scenario:
If an attacker calls the vulnerable run() function with this input:
php.run('./test/lib/templates/selfContainedTest.php; rm -rf /')
The actual command executed becomes:
php /full/path/to/test/lib/templates/selfContainedTest.php; rm -rf /
The shell sees the semicolon as a command separator and executes the destructive rm -rf / command with the same privileges as the Node.js application.
Other dangerous metacharacters include:
- | (pipe) - redirect output to another command
- && (AND operator) - execute next command if first succeeds
- || (OR operator) - execute next command if first fails
- ` (backticks) - command substitution
- $() - command substitution
- & (background execution)
Real-World Impact
For a PHP templating library like this one, an attacker could:
1. Inject arbitrary shell commands through the script filename
2. Read sensitive files on the system
3. Modify or delete application data
4. Establish reverse shells for persistent access
5. Pivot to other systems on the network
The vulnerability was classified as HIGH severity because it provides direct code execution with the privileges of the Node.js process.
The Fix
The security patch made two critical changes to eliminate the command injection vulnerability:
Change 1: Replace execSync with execFileSync
-const { execSync } = require('child_process')
+const { execFileSync } = require('child_process')
execFileSync() is fundamentally different from execSync():
- execSync(): Executes a command string through a shell interpreter
- execFileSync(): Directly executes a file without invoking a shell
Change 2: Pass Arguments as Array Elements
function run (script) {
+ const scriptPath = path.join(__dirname, script)
try {
- const stdout = execSync(`php ${path.join(__dirname, script)}`)
+ const stdout = execFileSync('php', [scriptPath])
return stdout.toString()
} catch (err) {
throw new Error(`PHP process exited with code ${err.status}`)
}
}
This change is crucial. Instead of:
execSync(`php ${path.join(__dirname, script)}`) // VULNERABLE: shell interprets the string
The fix uses:
execFileSync('php', [scriptPath]) // SAFE: arguments are passed directly to the executable
By passing the PHP script path as an array element in the second argument, Node.js passes it directly to the PHP executable without shell interpretation. Shell metacharacters become harmless literal characters.
Applied to Both Vulnerable Functions
The same pattern was applied to the runWithData() function:
function runWithData (template, model) {
const jsonModel = JSON.stringify(model, circular())
try {
- const stdout = execSync(`php ${path.join(__dirname, '/loader.php')}`, {
+ const stdout = execFileSync('php', [path.join(__dirname, '/loader.php')], {
input: jsonModel
})
return stdout.toString()
Verification: The New Test Case
The fix includes a new security test that validates the vulnerability is resolved:
test.serial('Shell metacharacters in the script path are not executed as shell commands', async t => {
try {
await php.run('./test/lib/templates/selfContainedTest.php; echo pwned')
} catch (e) {
t.true(e.message.includes('PHP process exited with code'))
}
})
This test attempts to inject the command ; echo pwned through the script path. With the vulnerable code, this would execute the echo command. With the fix, the semicolon is treated as a literal filename character, PHP fails to find a file with that name, and an error is thrown as expected.
Prevention & Best Practices
1. Always Use execFileSync() or execFile() Instead of execSync() or exec()
When you need to execute a program with parameters:
// ❌ UNSAFE
execSync(`program ${userInput}`)
// ✅ SAFE
execFileSync('program', [userInput])
2. Never Use String Interpolation for Command Arguments
// ❌ UNSAFE
execSync(`php script.php --name="${userName}"`)
// ✅ SAFE
execFileSync('php', ['script.php', '--name=' + userName])
// or with proper escaping if needed:
execFileSync('php', ['script.php', `--name=${userName}`])
3. Avoid shell: true Option
// ❌ UNSAFE
execSync('php script.php', { shell: true })
// ✅ SAFE
execFileSync('php', ['script.php'])
4. Validate and Sanitize Input When Possible
Even with execFileSync(), validate input:
function run(script) {
// Validate that script is a relative path to a PHP file
if (!script.endsWith('.php') || script.includes('..')) {
throw new Error('Invalid script path')
}
const scriptPath = path.join(__dirname, script)
const stdout = execFileSync('php', [scriptPath])
return stdout.toString()
}
5. Use Static Analysis Tools
Semgrep, ESLint with security plugins, and other static analysis tools can detect these patterns:
semgrep --config=p/security-audit index.js
6. Reference Security Standards
- CWE-78: OS Command Injection - https://cwe.mitre.org/data/definitions/78.html
- OWASP Command Injection: https://owasp.org/www-community/attacks/Command_Injection
Key Takeaways
-
Never use
execSync()with string interpolation on user-controlled input. The vulnerable patternexecSync(`command ${userInput}`)is inherently unsafe because the shell interprets metacharacters. -
execFileSync()with array arguments is the correct pattern for spawning child processes in Node.js. This bypasses the shell and treats all arguments as literal values, eliminating command injection. -
Shell metacharacters (
;,|,&&, etc.) in file paths become harmless when usingexecFileSync()because they're never passed to a shell interpreter. -
The
path.join()function alone is insufficient for security. While it normalizes paths, it doesn't prevent injection attacks when the result is interpolated into a shell command. -
This vulnerability affects all downstream consumers of the library. Security issues in dependencies can compromise applications that use them, making proactive fixes essential for library maintainers.
How Orbis AppSec Detected This
Source: The script parameter passed to the run() function (user-controlled input from function arguments)
Sink: The execSync() call at index.js:9 where the script parameter is interpolated into a shell command string
Missing control: No validation that the script parameter contains only safe characters, and the use of execSync() with shell string interpolation instead of execFileSync() with argument arrays
CWE: CWE-78 - Improper Neutralization of Special Elements used in an OS Command ('OS Command Injection')
Fix: Replaced execSync() with execFileSync(), moved the path joining outside the command string, and passed the script path as an array argument to prevent shell interpretation
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 unsafe child process calls is a critical vulnerability that can lead to complete system compromise. The fix in this case demonstrates the importance of using the right API for the job: execFileSync() with array arguments is the secure way to spawn child processes in Node.js when you need to pass dynamic parameters.
By understanding how shell metacharacters can be exploited through string interpolation, developers can write more secure code. This vulnerability also highlights the value of static analysis tools like Semgrep in catching dangerous patterns before they reach production.
If you maintain a Node.js library or application that spawns child processes, audit your codebase now for similar patterns. Replace execSync() and exec() calls with their safer counterparts, and always pass dynamic arguments as array elements rather than interpolating them into command strings.