Fix potential OS command injection in async.js

[DongZifan]

Command Injection Reproduction Notes for async.js

Summary

Hello,
I am writing to report a potential OS Command Injection vulnerability in the following file:
build/src/utils/async.js

The issue appears when user-controlled input is passed to the utility's spawn and exec wrapper functions. Both functions are configured with { shell: true } enabled by default. Under these conditions, specially crafted input strings containing shell metacharacters can escape the intended command context and execute arbitrary code on the host system. This poses a significant security risk as it allows for Remote Code Execution (RCE).

Root Cause

The issue is caused by the default shell: true option enforced in the following functions:

spawn: async (command, args, opts) => {
    // ...
    opts = opts || { stdio: 'inherit', shell: true }; // Forces shell execution
    const proc = spawnCb(command, args, opts);
    // ...
},

exec: async (command, opts) => {
    // ...
    opts = opts || { stdio: 'inherit', shell: true }; // Forces shell execution
    const proc = execCb(command, opts);
    // ...
}

Reproduction Material

A minimal reproduction script is provided in: poc_async.js
poc_async.js

This Proof of Concept (PoC) is intended to demonstrate that external input can reach dangerous command execution logic and trigger unauthorized OS commands.

What the PoC Does

The PoC performs a minimal end-to-end trigger of the vulnerable code path:

1. It hooks child_process.exec and child_process.spawn to log intercepted commands before they execute.
2. It loads the vulnerable async.js.
3. It invokes utils.exec() with a payload containing & calc # designed to chain an additional command.
4. It invokes utils.spawn() with a similar payload, demonstrating that array-based arguments are still vulnerable when shell: true is active.
5. Each call successfully passes the malicious payload to the system shell for execution.

In the provided example, the injected payload is crafted for Windows systems. Successful command execution will launch the Calculator application.

Example Payload

The PoC uses the following payload for the input parameters:

& calc #

How to Run

Run from the project root:

node poc_async.js

Expected Output

When the PoC runs, you should see output similar to:

(*) Exec: echo "Hello" & calc #

[POC] child_process.exec() would run:
>> COMMAND: echo "Hello" & calc #

(*) Spawn: echo "Testing Spawn" & calc #

[POC] child_process.spawn() would run:
>> echo "Testing Spawn" & calc # 

[POC] The injection instruction has been delivered to the operating system

In the vulnerable version, after the requests are processed, the local machine will launch two instances of the Calculator application as a benign demonstration effect (one for exec and one for spawn).
This shows that the attacker-controlled input can influence command execution behavior through both vulnerable execution paths.

Patch Explanation

This branch also includes a patched version of async.js intended to mitigate the command injection risk described above.

What the patch changes

The patch introduces two key changes to prevent shell metacharacter interpretation:

• Disables default shell execution: The default shell option is changed from true to false in both spawn and exec wrappers. This prevents the OS shell from interpreting metacharacters in command arguments.

• Replaces child_process.exec with child_process.execFile: The exec wrapper now parses the command string using command.trim().split(/\s+/) into a binary path and arguments array, then calls execFile which executes the binary directly without a shell.

Validation introduced by the patch

The patched version makes the following changes:

• spawn wrapper: Defaults to shell: false (explicitly set with opts.shell = false). Arguments are passed literally to the target process without any shell interpretation.

• exec wrapper: Uses command.trim().split(/\s+/) to split the command string into the executable file path and its arguments, then delegates to child_process.execFile:

const parts = command.trim().split(/\s+/);
const file = parts[0];
const args = parts.slice(1);
const proc = execFileCb(file, args, opts);

Because execFile executes the binary directly without invoking cmd.exe or /bin/sh, shell metacharacters such as &, |, ;, $, >, and < are treated as literal argument values rather than command syntax. This prevents command injection regardless of what characters appear in user-supplied input.

[DongZifan]

@DongZifan please read the following Contributor License Agreement(CLA). If you agree with the CLA, please reply with the following information.请仔细阅读以下的《贡献者许可协议》。如果您同意该协议，请回复以下信息。

@microsoft-github-policy-service agree [company="{your company}"]

Options:  选项：

• (default - no company specified) I have sole ownership of intellectual property rights to my Submissions and I am not making Submissions in the course of work for my employer.（默认值：未指定公司）我对自己提交的各项知识产权拥有独家所有权。我提交这些内容并非是在为雇主工作期间所为。

@microsoft-github-policy-service agree

• (when company given) I am making Submissions in the course of work for my employer (or my employer has intellectual property rights in my Submissions by contract or applicable law). I have permission from my employer to make Submissions and enter into this Agreement on behalf of my employer. By signing below, the defined term “You” includes me and my employer.(如公司已明确说明)我是在为雇主履行工作职责而提交相关内容/成果的（或者，根据合同或相关法律，我的雇主拥有我所提交内容/成果的知识产权）。我已获得雇主的授权，可以代表雇主来提交这些内容/成果并签署本协议。在下方签字时，术语“您”既指我，也指我的雇主。

@microsoft-github-policy-service agree company="Microsoft"

Contributor License Agreement
投稿人许可协议

@microsoft-github-policy-service agree [company="{Beijing University Of Technology}"]

[DongZifan]

W

@microsoft-github-policy-service agree [company="{Beijing University Of Technology}"]

@microsoft-github-policy-service agree [company="Beijing University Of Technology"]

[Copilot]

🟡 Not ready to approve

As written, it breaks existing call sites that explicitly rely on shell: true and shell features (e.g. pipelines), and the new exec() parsing/option handling changes behavior in ways that will cause failures.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

This review doesn't count toward merge requirements. Sign up for the private preview to control whether Copilot approvals count.

Pull request overview

This PR aims to mitigate a potential OS command-injection vector in the build utilities by avoiding implicit shell execution in build/src/utils/async.js and moving exec() away from child_process.exec.

Changes:

• Switched the exec() wrapper from child_process.exec to child_process.execFile.
• Changed spawn()/exec() option handling to disable shell execution by default.
• Added command-string splitting to derive file + args for execFile.

File summaries

File	Description
build/src/utils/async.js	Updates process-execution wrappers to reduce shell usage (security hardening), with behavioral changes to spawn()/exec() defaults and exec() implementation.

Review details

Comments suppressed due to low confidence (2)

build/src/utils/async.js:30

• spawn() forces opts.shell = false, overriding callers that explicitly pass { shell: true } (for example build/src/push.js:207 and multiple uses in build/src/utils/image-content-extractor.js). This is a breaking change and will cause commands that depend on shell expansion/quoting to fail. Prefer a secure default (shell: false) while still honoring an explicit caller opt-in.

        opts = Object.assign({}, opts || { stdio: 'inherit' });
        opts.shell = false;
    

build/src/utils/async.js:87

• exec() currently (1) forces opts.shell = false even when the caller explicitly requests shell execution and (2) passes a stdio default that can interfere with execFile stdout/stderr capture. This breaks existing uses that rely on shell: true for pipelines (e.g. docker export ... | docker import ...) and can also cause asyncUtils.exec() to return an empty string by default. Use a secure default (shell: false), drop stdio for exec/execFile, and switch between exec() and execFile() based on whether shell execution was explicitly requested.

        opts = Object.assign({}, opts || { stdio: 'inherit' });
        opts.shell = false;

        const parts = command.trim().split(/\s+/);
        const file = parts[0];
        const args = parts.slice(1);

        return new Promise((resolve, reject) => {
            let result = '';
            const proc = execFileCb(file, args, opts);

• Files reviewed: 1/1 changed files
• Comments generated: 2
• Review effort level: Lite

We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.

[DongZifan]

exec is now imported alongside execFile.
spawn and exec now only default shell to false when the caller does not specify it (if (opts.shell === undefined) opts.shell = false). Callers that explicitly pass { shell: true } (e.g. build/src/push.js:215 for pipelines) are respected.
When shell: true is requested, exec uses child_process.exec as before. When shell: false (the default), it uses child_process.execFile.

[DongZifan]

Changed exec from exec(command, opts) to exec(file, args, opts). Arguments are now passed directly to execFile as an array, eliminating the need for command.trim().split(/\s+/) entirely.
Paths with spaces (C:\Program Files...) are preserved because the caller passes the full path as file.
Arguments with embedded spaces or quotes are preserved because each argument is a separate array element.
When shell: true is explicitly requested, the argument array is joined back into a command string for exec.

[DongZifan]

@microsoft-github-policy-service agree 乌衣巷. ***@***.***