Skip to content

Fix potential OS command injection in clean.js#3335

Open
DongZifan wants to merge 1 commit into
elastic:masterfrom
DongZifan:master
Open

Fix potential OS command injection in clean.js#3335
DongZifan wants to merge 1 commit into
elastic:masterfrom
DongZifan:master

Conversation

@DongZifan
Copy link
Copy Markdown

Command Injection Reproduction Notes for clean.js

Summary

Hello,
I am writing to report a potential Git Argument Injection vulnerability in the following file:
docs/preview/clean.js

The issue appears when remote branch names are extracted via a loose regular expression and subsequently used to construct Git command arrays that are executed via child_process.execFile(...). Under certain conditions, specially crafted branch names provided in the remote Git repository may allow unintended Git option execution on the host system. This could potentially lead to arbitrary code execution or arbitrary file read/write risks, depending on how the input is parsed by the Git executable.

Root Cause

The issue is caused by unsafe argument construction and missing option terminators in the following functions:

const heads_to_prs = (heads) => {
    return heads
      .split('\n')
      .reduce((acc, line) => {
        // Loose regex that allows special characters and option prefixes like '-'
        const found = line.match(/^.+ refs\/heads\/((.+)_(\d+))$/); 
        // ...
      });
  }

  const prAge = async function (pr) {
    return parseInt(await exec_git(
      [
        "show", "--pretty=%ad", "--no-notes", "--no-patch", "--date=unix",
        pr.branch // No '--' terminator to prevent pr.branch from being parsed as an option
      ],
      { cwd: local_path }
    ));
  }

Reproduction Material

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

This Proof of Concept (PoC) is intended to demonstrate that external input (remote branch names) can reach dangerous command execution logic through the vulnerable code path.

What the PoC Does

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

  1. It hooks child_process.execFile to monitor the arguments passed to Git.
  2. It initializes the Cleaner module and invokes the cleanup process (cleaner.run()).
  3. During the show-ref --heads step, the mocked Git repository returns a maliciously crafted branch name: --upload-pack=calc.exe_123.
  4. The loose regex in heads_to_prs successfully matches and extracts this malicious branch name..
  5. The supplied pr.branch value is passed as an element into the command array for git show (and potentially git push) via exec_git(...).

In the provided example, the injected payload is crafted so that, if executed by a real Git binary, it would trigger the --upload-pack option. In the PoC, it intercepts the malicious argument and simulates opening the Calculator application. This serves as a visible indicator that external input can reach the OS command execution sink without proper sanitization.

Example Payload

The PoC uses the following payload for the malicious branch name:

--upload-pack=calc.exe_123

How to Run

Run from the project root:

node poc_clean.js dummy 1

Expected Output

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

[POC] Initializing Cleaner for node: 1

[POC] execFile would run:
 git [ 'clone', '--bare', '--reference', './cache_1/docs.git', 'https://github.com/elastic/docs.git', './tmp_1/docs.git' ] 

[POC] execFile would run:
 git [ 'show-ref', '--heads' ] 

[POC] execFile would run:
 git [
  'show',
  '--pretty=%ad',
  '--no-notes',
  '--no-patch',
  '--date=unix',
  '--upload-pack=calc.exe_123'
] 

[POC SUCCESS] Argument injection confirmed in git arguments!

In the vulnerable version, after the crafted remote branch is processed, the local machine will launch the Calculator application (simulated in the PoC) as a benign demonstration effect.
This shows that the attacker-controlled branch name value can influence Git command execution behavior through the vulnerable exec_git path.

Patch Explanation

This branch also includes a patched version of clean.js intended to mitigate the Git argument injection risk described above.

What the patch changes

The patch adds strict validation and correct POSIX command argument termination before branch-related values are used by the following functions:

  • heads_to_prs(heads)
  • prAge(pr)
  • deleteBranch(pr)

In the vulnerable version, attacker-controlled values such as:

  • pr.branch

could reach child_process.execFile('git', ...) as unintended options.

The patched version introduces input checks and proper command terminators before these values are passed into command execution logic.

Validation introduced by the patch

For Git branch names, the patch ensures that:

  1. The regex used in heads_to_prs strictly validates the commit hash and the branch characters, allowing only alphanumeric characters, underscores, dots, and hyphens.
  2. A negative lookahead (?!-) is used in the regex to explicitly prevent branch names from starting with a hyphen -.
  3. An explicit programmatic check if (pr.branch.startsWith('-')) is added to deleteBranch as an ultimate safeguard to throw an error if an invalid branch name reaches the deletion phase.
  4. The -- POSIX end-of-options delimiter is injected before pr.branch in prAge ([..., "--date=unix", "--", pr.branch]).

If validation fails or attempts to parse malicious options occur, the branch is safely ignored or securely terminated instead of being parsed as an unintended Git option.

@DongZifan DongZifan requested a review from a team as a code owner May 14, 2026 13:57
@DongZifan DongZifan requested a review from technige May 14, 2026 13:57
@cla-checker-service
Copy link
Copy Markdown

❌ Author of the following commits did not sign a Contributor Agreement:
74f3da2

Please, read and sign the above mentioned agreement if you want to contribute to this project

@github-actions
Copy link
Copy Markdown

A documentation preview will be available soon.

Request a new doc build by commenting
  • Rebuild this PR: run docs-build
  • Rebuild this PR and all Elastic docs: run docs-build rebuild

run docs-build is much faster than run docs-build rebuild. A rebuild should only be needed in rare situations.

If your PR continues to fail for an unknown reason, the doc build pipeline may be broken. Elastic employees can check the pipeline status here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant