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

The Setup: A Shortcut Function with a Hidden Flaw

The bin/index.js file in this Node.js application handles several platform-specific tasks, including creating a Windows desktop shortcut after installation. The createWindowsShortcut(exePath, destDir) function, defined around line 219, is responsible for generating a .lnk file on the user's Desktop. It does this by constructing a PowerShell one-liner and running it with Node's execSync.

On the surface, this looks like a reasonable approach — PowerShell is the natural tool for creating Windows shortcuts via the WScript.Shell COM object. But the implementation contained a critical flaw: the destDir argument (and its siblings exePath and shortcutPath) were interpolated directly into the command string, with only a naïve single-quote escape standing between the application and arbitrary command execution.

Semgrep flagged this pattern with rule javascript.lang.security.detect-child-process.detect-child-process at line 232, and the fix that followed is a masterclass in how to properly eliminate this class of vulnerability.

The Vulnerability Explained

Here is the exact vulnerable code that was removed:

// Normalize paths for Windows shells
const normShortcutPath = shortcutPath.replace(/\//g, '\\');
const normExePath = exePath.replace(/\//g, '\\');
const normDestDir = destDir.replace(/\//g, '\\');

const script = `$WshShell = New-Object -ComObject WScript.Shell; $Shortcut = $WshShell.CreateShortcut('${normShortcutPath}'); $Shortcut.TargetPath = '${normExePath}'; $Shortcut.WorkingDirectory = '${normDestDir}'; $Shortcut.Save();`;
// Escape single quotes for PowerShell
const escapedScript = script.replace(/'/g, "''");

execSync(`powershell -Command "${escapedScript}"`, { stdio: 'ignore' });

There are two compounding problems here:

Problem 1: String Interpolation into a Shell Command

The values of normShortcutPath, normExePath, and normDestDir are embedded directly into a PowerShell script string using template literals. This script is then passed as a single string argument to execSync, which invokes a shell to interpret it. Any shell metacharacter — a semicolon, a backtick, a $(...) expression — present in destDir would be interpreted by PowerShell as part of the command, not as a literal path.

Problem 2: The Escape Is Insufficient

The code attempts to mitigate this by escaping single quotes (' → ''), which is the PowerShell string-doubling convention. But this protection is incomplete for several reasons:

• The outer command is wrapped in double quotes at the execSync level: powershell -Command "${escapedScript}". Double-quote contexts in PowerShell have different escaping rules than single-quote contexts.
• Backtick (`) is PowerShell's escape character and is not neutralized.
• Subexpression operators like $(...) and &{...} are active inside double-quoted strings.
• On some Windows configurations, the command passes through cmd.exe first before reaching PowerShell, adding another layer of shell interpretation with its own metacharacters (&, |, %, ^).

A Concrete Attack Scenario

Suppose this library is used as a dependency in a larger tool that accepts a user-supplied installation directory. If an attacker can influence the value of destDir — for example, by passing a crafted path like:

C:\legit\path'; Start-Process calc.exe; #

After the single-quote escape, this becomes:

C:\legit\path''; Start-Process calc.exe; #

The doubled quote closes the PowerShell string, the semicolon ends the statement, and Start-Process calc.exe (or any other command) executes with the privileges of the Node.js process. On a developer's machine or a CI/CD runner, that's often elevated privilege.

Because this is a library, the attack surface is every downstream consumer who passes any externally influenced path into this function.

The Fix

The fix is elegant and thorough. It makes two coordinated changes across two files.

Before (bin/index.js, ~line 219–232)

const normShortcutPath = shortcutPath.replace(/\//g, '\\');
const normExePath = exePath.replace(/\//g, '\\');
const normDestDir = destDir.replace(/\//g, '\\');

const script = `$WshShell = New-Object -ComObject WScript.Shell; $Shortcut = $WshShell.CreateShortcut('${normShortcutPath}'); $Shortcut.TargetPath = '${normExePath}'; $Shortcut.WorkingDirectory = '${normDestDir}'; $Shortcut.Save();`;
const escapedScript = script.replace(/'/g, "''");

execSync(`powershell -Command "${escapedScript}"`, { stdio: 'ignore' });

After (bin/index.js)

import { buildShortcutInvocation } from './windows-shortcut.cjs';

// ...

const { args, env } = buildShortcutInvocation(exePath, destDir, shortcutPath);
execFileSync('powershell', args, { stdio: 'ignore', env });

The New Helper: bin/windows-shortcut.cjs

const STATIC_SCRIPT = `
$WshShell = New-Object -ComObject WScript.Shell
$Shortcut = $WshShell.CreateShortcut($env:CODEX_SHORTCUT_PATH)
$Shortcut.TargetPath = $env:CODEX_TARGET_PATH
$Shortcut.WorkingDirectory = $env:CODEX_WORKING_DIR
$Shortcut.Save()
`.trim();

function normWin(p) {
    return p // [normalizes path separators]
}

Why This Fix Works

1. execFileSync instead of execSync

execSync spawns a shell (cmd.exe on Windows) and passes the entire command as a string for the shell to parse. execFileSync bypasses the shell entirely — it invokes the named executable (powershell) directly with an array of arguments. There is no shell metacharacter interpretation because there is no shell.

2. Static script, no interpolation

The PowerShell script stored in STATIC_SCRIPT is a compile-time constant. It contains no template literals, no string concatenation, and no user data whatsoever. PowerShell reads the path values from environment variables ($env:CODEX_SHORTCUT_PATH, $env:CODEX_TARGET_PATH, $env:CODEX_WORKING_DIR), not from the script text itself.

3. Data travels via environment variables

The buildShortcutInvocation function constructs an env object containing the path values. These are passed to execFileSync as process environment variables — a channel that is completely separate from the command/argument channel. No matter what characters destDir contains, they cannot affect the structure of the PowerShell script being executed.

4. Separation of code and data

This is the fundamental principle behind all injection-prevention: code and data must travel through separate channels. The old code mixed them (data embedded in code string). The new code keeps them strictly separated (static code + data via environment).

Prevention & Best Practices

1. Prefer execFileSync / execFile over execSync / exec

When you must invoke an external process, always prefer the execFile family. These functions accept the executable path and arguments as separate parameters and never invoke a shell:

// Dangerous — shell interprets the entire string
execSync(`convert ${userFile} output.png`);

// Safe — no shell, arguments are passed directly to the process
execFileSync('convert', [userFile, 'output.png']);

2. Never interpolate untrusted data into command strings

If you find yourself using template literals or string concatenation to build a command, stop. That's a signal to redesign the data flow. Use argument arrays, environment variables, stdin, or temporary files instead.

3. Use environment variables to pass data to subprocesses

As demonstrated in this fix, environment variables are an excellent out-of-band channel for passing data to child processes. They are not subject to shell parsing and can contain arbitrary strings (including newlines and special characters) safely.

4. Validate and normalize paths before use

Even with the new fix, it's good practice to validate that destDir is an absolute path pointing to an expected location before using it. A path validation step (e.g., checking that the resolved path starts with an expected prefix) adds defense in depth.

5. Use static analysis in CI

The Semgrep rule javascript.lang.security.detect-child-process.detect-child-process caught this issue automatically. Add Semgrep or a similar SAST tool to your CI pipeline to catch these patterns before they reach production.

Relevant standards:
- OWASP: OS Command Injection Defense Cheat Sheet
- CWE-78: Improper Neutralization of Special Elements used in an OS Command ('OS Command Injection')

Key Takeaways

• The createWindowsShortcut function in bin/index.js was the specific risk site — not the entire codebase. Targeted, scoped fixes like this one are easier to review and less likely to introduce regressions.
• Escaping single quotes in a PowerShell string is not a complete defense — the outer execSync shell layer, double-quote context, and backtick metacharacter all create bypass opportunities.
• execFileSync('powershell', args, ...) is categorically safer than execSync('powershell -Command "..."') because it eliminates the shell parsing layer entirely.
• Moving path data into env variables (CODEX_SHORTCUT_PATH, CODEX_TARGET_PATH, CODEX_WORKING_DIR) means the PowerShell script is always static — there is nothing for an attacker to inject into.
• Library code has a larger attack surface than application code — a vulnerability in a shared package affects every downstream consumer, making proactive hardening especially important.

How Orbis AppSec Detected This

• Source: The destDir parameter of createWindowsShortcut(exePath, destDir) in bin/index.js — a value that downstream consumers of this library pass in, potentially from user-controlled input.
• Sink: execSync(\powershell -Command "${escapedScript}"`, { stdio: 'ignore' })at line 232 ofbin/index.js, where the tainteddestDirvalue (vianormDestDir`) is embedded into the executed command string.
• Missing control: No validation of destDir against an allowlist or expected path prefix; no use of argument arrays or environment variable channels to isolate data from code; reliance on incomplete single-quote escaping.
• CWE: CWE-78 — Improper Neutralization of Special Elements used in an OS Command ('OS Command Injection')
• Fix: Replaced dynamic command string interpolation with a static PowerShell script and execFileSync, passing all path values through environment variables to eliminate the injection surface.

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 createWindowsShortcut vulnerability is a precise illustration of why "sanitize the input" is often the wrong mental model for command injection. The original code tried to sanitize — it normalized path separators and escaped single quotes — but the sanitization was incomplete because the root cause was never addressed: user data was being embedded into a command string that a shell would interpret.

The correct mental model is never mix code and data. The fix achieves this perfectly: a static, compile-time PowerShell script reads its inputs from environment variables at runtime, and execFileSync ensures no shell ever sees the path values as text to parse. The vulnerability surface is reduced to zero, not reduced to "probably small enough."

For Node.js developers building tools that invoke subprocesses: audit every exec and execSync call in your codebase. If any argument to those functions contains a variable, ask whether that variable could ever be influenced by external input — directly or through a chain of function calls. If the answer is "maybe," replace the shell-based call with execFileSync and 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 OS Command Injection Defense Cheat Sheet
• Node.js child_process.execFileSync documentation
• Semgrep rule: detect-child-process
• harden: sanitize child_process call in index.js...