How command injection happens in Node.js subprocess and how to fix it

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

In the development tools repository, security researchers discovered a critical command injection vulnerability in tools/dev/src/index.ts that could have allowed attackers to execute arbitrary shell commands on systems running the package. The vulnerability existed in the runLoggedCommand() function, which spawns subprocesses to execute build and development commands. By passing specially crafted arguments containing shell metacharacters, an attacker could break out of the intended command and execute malicious code.

This is a real-world example of why subprocess handling in Node.js requires explicit security controls—and how a single line of code can make the difference between a vulnerable and secure implementation.

The Vulnerability Explained

What Happened

The vulnerable code in tools/dev/src/index.ts:322 looked like this:

const child = spawn(request.command, request.args, {
  cwd: request.cwd,
  env: request.env,
  // shell: false is NOT explicitly set here
  stdio: ["ignore", request.logFd, request.logFd],
  windowsHide: process.platform === "win32",
  windowsVerbatimArguments: request.windowsVerbatimArguments,
});

The critical issue: the spawn() call doesn't explicitly set shell: false.

While Node.js defaults to shell: false, this is a dangerous assumption to rely on. More importantly, without explicit security controls, future maintainers might not realize the security implications of this code, and the vulnerability could be reintroduced through refactoring.

The Attack

Imagine an attacker controls the request.args array passed to runLoggedCommand(). They could inject shell metacharacters:

// Attacker-controlled input
const maliciousRequest = {
  command: "echo",
  args: ["hello; cat /etc/passwd"],  // Shell injection payload
  cwd: "/some/path",
  env: process.env,
  logFd: 1
};

// Without shell: false, the semicolon is interpreted as a command separator
// Instead of echoing "hello; cat /etc/passwd", the shell executes:
// 1. echo hello
// 2. cat /etc/passwd

Other dangerous payloads could include:

• Command substitution: ["$(whoami > /tmp/pwned)"] — executes a command and uses its output
• Pipe injection: ["|", "rm", "-rf", "/"] — chains commands together
• Environment variable injection: ["$(id > /tmp/exploit)"] — executes commands via variable expansion
• Background execution: ["&", "nohup", "malware.sh"] — runs commands in the background

Why This Matters

The tools/dev/src/index.ts file is part of a Node.js library distributed to downstream consumers. Every project that uses this package inherits the vulnerability. If the library is used to:

• Run build commands with user-supplied arguments
• Execute scripts based on configuration files
• Process command-line input from developers or CI/CD systems

...then attackers could exploit this to gain code execution in the build pipeline, compromise dependencies, or attack developer machines.

The Fix

The fix is elegantly simple: explicitly set shell: false in the spawn options.

Code Change

const child = spawn(request.command, request.args, {
  cwd: request.cwd,
  env: request.env,
+ shell: false,
  stdio: ["ignore", request.logFd, request.logFd],
  windowsHide: process.platform === "win32",
  windowsVerbatimArguments: request.windowsVerbatimArguments,
});

Why This Works

When shell: false is set:

1. No shell interpreter is invoked: The command is executed directly via execve() on Unix or CreateProcess() on Windows
2. Arguments are passed as-is: The array elements in request.args are passed directly to the executable, not parsed by a shell
3. Shell metacharacters are literal: A semicolon, pipe, or dollar sign in an argument is treated as a literal character, not a special shell operator

With this fix, the attack payload ["hello; cat /etc/passwd"] is passed to the echo command as a single literal string, resulting in:

$ echo "hello; cat /etc/passwd"
hello; cat /etc/passwd

No command injection occurs.

Regression Testing

The PR also added a comprehensive test to prevent future regressions:

describe("runLoggedCommand security: shell injection prevention (V-001)", () => {
  it("spawn is invoked with shell: false to prevent user-controlled argument injection", async () => {
    const src = await readFile(path.join(toolsDevRoot, "src/index.ts"), "utf8");

    // Security invariant: runLoggedCommand must pass shell: false to spawn.
    assert.match(
      src,
      /shell:\s*false/,
      "spawn() in runLoggedCommand must explicitly set shell: false",
    );

    // Guard against regressions that re-introduce shell: true
    assert.doesNotMatch(
      src,
      /shell:\s*true/,
      "No spawn() call in index.ts should set shell: true",
    );
  });
});

This test reads the source file and verifies that:
- shell: false is explicitly present
- shell: true never appears in the file

This ensures the security property is maintained through code reviews and automated testing.

Prevention & Best Practices

1. Always Use shell: false Explicitly

Even though Node.js defaults to shell: false, explicitly setting it makes the security intent clear:

// ✅ Good: explicit security control
const child = spawn('command', ['arg1', 'arg2'], { shell: false });

// ⚠️ Risky: relying on defaults
const child = spawn('command', ['arg1', 'arg2']);

// ❌ Dangerous: enabling shell interpretation
const child = spawn('command', ['arg1', 'arg2'], { shell: true });

2. Pass Arguments as Arrays, Never Concatenate Strings

// ✅ Good: arguments passed as array elements
spawn('echo', ['hello', 'world']);

// ❌ Dangerous: concatenating strings with shell interpretation
exec('echo hello world');  // Uses shell by default

3. Use execFile() for Simple Cases

For executing a single file without shell interpretation, execFile() is a good alternative:

import { execFile } from 'child_process';

execFile('ls', ['-la', '/tmp'], (error, stdout, stderr) => {
  if (error) throw error;
  console.log(stdout);
});

4. Validate and Sanitize User Input

While shell: false is the primary defense, defense-in-depth suggests validating input:

function validateCommand(cmd) {
  // Allowlist: only permit known, safe commands
  const allowedCommands = ['build', 'test', 'lint'];
  if (!allowedCommands.includes(cmd)) {
    throw new Error('Invalid command');
  }
  return cmd;
}

function validateArgs(args) {
  // Ensure args is an array of strings
  if (!Array.isArray(args)) throw new Error('Args must be array');
  if (!args.every(arg => typeof arg === 'string')) {
    throw new Error('All args must be strings');
  }
  return args;
}

const command = validateCommand(request.command);
const args = validateArgs(request.args);
spawn(command, args, { shell: false });

5. Use Security Scanners and Linters

Tools can automatically detect dangerous patterns:

• Semgrep: Detects spawn/exec calls without proper controls
• ESLint with security plugins: Flags shell: true usage
• Orbis AppSec: Identifies command injection risks through taint analysis

6. Reference OWASP and CWE 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
• Node.js Child Process Security: https://nodejs.org/en/docs/guides/security/#command-injection

Key Takeaways

• Never rely on shell defaults: Explicitly set shell: false in every spawn() call to make security intent clear and prevent accidental regressions
• Arguments must be arrays, not strings: Passing arguments as array elements ensures they're not interpreted by a shell, even if shell: false is somehow removed
• The runLoggedCommand() function now safely handles untrusted input: User-controlled arguments in request.args are passed directly to the executable without shell parsing
• Regression tests protect against future vulnerabilities: The new test ensures shell: false remains in place and prevents shell: true from being reintroduced
• Command injection is preventable through API design: Choosing the right Node.js APIs (spawn with shell: false) eliminates entire classes of vulnerabilities

How Orbis AppSec Detected This

Source: User-controlled arguments passed to the runLoggedCommand() function via the request.args parameter

Sink: The spawn(request.command, request.args, ...) call in tools/dev/src/index.ts:322 without explicit shell: false

Missing control: No explicit shell: false option and no validation that the spawn options disable shell interpretation

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

Fix: Added shell: false to the spawn options object to ensure arguments are passed directly to the executable without shell metacharacter 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 in Node.js subprocess calls is a critical vulnerability that can lead to remote code execution—but it's entirely preventable through explicit security controls. The fix in tools/dev/src/index.ts demonstrates that sometimes the most important security improvements are the simplest: a single line of code (shell: false) that makes the security intent unmistakable.

For developers maintaining libraries or tools that execute subprocesses, this is a powerful reminder: always explicitly set shell: false, pass arguments as arrays, and test your security assumptions. Use tools like Orbis AppSec to automatically catch these issues before they reach production.

Secure subprocess handling isn't just about preventing attacks—it's about writing code that's safe by default and easy for future maintainers to understand and maintain securely.

References

• CWE-78: OS Command Injection
• OWASP Command Injection
• Node.js Child Process Documentation
• Node.js Security Best Practices: Command Injection
• Semgrep Rule: Dangerous spawn/exec with shell enabled
• GitHub PR: fix: multiple subprocess calls accept user-controlle... in index.ts