# Vulnerability Verification Report

## 1. Summary
- **Vulnerability Type**: OS Command Injection (CWE-78)
- **Flagged Location**: `src/infra/exec-approvals-analysis.ts:399-401` (and surrounding parser logic)
- **Vulnerability Description**: OpenClaw versions prior to 2026.2.22 contain an allowlist bypass in `system.run` that allows attackers to execute non-allowlisted commands by splitting command substitution `$(...)` using shell line-continuation characters `\<newline>` inside double quotes. The `splitShellPipeline` parser fails to detect `$()` when `$` and `(` are separated by an escaped newline, causing the allowlist analysis to pass while the shell folds the line continuation into executable command substitution.

## 2. Analysis Logic

### Step 1: Identifying the command execution sink

The `system.run` command handler in `src/node-host/invoke.ts` (line 488-493) is the entry point for executing arbitrary commands on node hosts. When `system.run` is invoked, the handler at line 792 calls `runCommand()` which uses `spawn()` from Node.js `child_process`:

```typescript
// src/node-host/invoke.ts:227-246
async function runCommand(
  argv: string[],
  ...
): Promise<RunResult> {
  return await new Promise((resolve) => {
    ...
    const child = spawn(argv[0], argv.slice(1), {
      cwd,
      env,
      stdio: ["ignore", "pipe", "pipe"],
      windowsHide: true,
    });
```

The argv passed to `spawn` on non-Windows is `["/bin/sh", "-lc", rawCommand]` (constructed in `src/infra/node-shell.ts:7-8`), meaning the shell IS explicitly invoked and interprets the command string.

### Step 2: Tracing the allowlist analysis path

When `system.run` is invoked with a `rawCommand`, the handler calls `evaluateShellAllowlist()` (`invoke.ts:540-549`):

```typescript
// src/node-host/invoke.ts:539-554
if (rawCommand) {
    const allowlistEval = evaluateShellAllowlist({
      command: rawCommand,
      allowlist: approvals.allowlist,
      safeBins,
      cwd: params.cwd ?? undefined,
      env,
      skillBins: bins,
      autoAllowSkills,
      platform: process.platform,
    });
```

`evaluateShellAllowlist` (`src/infra/exec-approvals-analysis.ts:1036-1123`) calls `analyzeShellCommand` which calls `splitShellPipeline`.

### Step 3: Identifying the parsing vulnerability in `splitShellPipeline`

The `splitShellPipeline` function (`src/infra/exec-approvals-analysis.ts:270-503`) is a state-machine parser that walks the command string character-by-character, tracking quote states, escapes, and dangerous shell tokens.

**The `$(...)` check inside double quotes** (lines 399-401):
```typescript
// src/infra/exec-approvals-analysis.ts:399-401
if (ch === "$" && next === "(") {
    return { ok: false, reason: "unsupported shell token: $()", segments: [] };
}
```

This check requires `$` to be IMMEDIATELY followed by `(`. When `$\<newline>(` is used instead, `$` is followed by `\`, not `(`, so the check is bypassed.

**The double-quote escape handling** (lines 266-268, 392-398):
```typescript
// src/infra/exec-approvals-analysis.ts:251
const DOUBLE_QUOTE_ESCAPES = new Set(["\\", '"', "$", "`", "\n", "\r"]);

// src/infra/exec-approvals-analysis.ts:392-398
if (ch === "\\" && isDoubleQuoteEscape(next)) {
    buf += ch;
    buf += next;
    i += 1;
    emptySegment = false;
    continue;
}
```

Since `\n` (newline) IS in `DOUBLE_QUOTE_ESCAPES`, the `\<newline>` sequence inside double quotes is consumed as a valid double-quote escape. Both characters (`\` and `\n`) are added to the buffer, and `i` advances past the newline.

**The `(` inside double quotes bypasses the DISALLOWED check** (lines 250, 472-474):
```typescript
// src/infra/exec-approvals-analysis.ts:250
const DISALLOWED_PIPELINE_TOKENS = new Set([">", "<", "`", "\n", "\r", "(", ")"]);

// src/infra/exec-approvals-analysis.ts:472-474
if (DISALLOWED_PIPELINE_TOKENS.has(ch)) {
    return { ok: false, reason: `unsupported shell token: ${ch}`, segments: [] };
}
```

Lines 472-474 are OUTSIDE the `inDouble` block (lines 391-414), so when `(` is encountered while still inside double quotes, it is NOT rejected by `DISALLOWED_PIPELINE_TOKENS`.

### Step 4: Tracing the complete bypass

Tracing the input `"$\<LF>(cmd)"` through `splitShellPipeline` with the parser inside double quotes:

1. `ch='$'`, `next='\'` → `$(` check fails (next is `\`, not `(`) → `$` is added to buffer
2. `ch='\'`, `next='<LF>'` → `\<LF>` matched as double-quote escape (LF ∈ DOUBLE_QUOTE_ESCAPES) → both chars consumed and added to buffer, `i` advanced past LF
3. `ch='('` → still inside double quotes → `(` not checked against DISALLOWED → `(` added to buffer

Buffer result: `$\<LF>(cmd)` — the parser passes.

**Shell interpretation**: In bash, `\<newline>` inside double quotes is a line continuation (both chars are REMOVED). So the shell interprets the buffer as `$(cmd)` — command substitution executes.

### Step 5: Version confirmation

The `package.json` (`/package.json:3`) shows the version is `2026.2.13`:

```json
"version": "2026.2.13",
```

The CVE states the fix is in version `2026.2.22`. Since `2026.2.13 < 2026.2.22`, this codebase IS vulnerable.

### Step 6: Checking for existing tests or mitigations

The test file `src/infra/exec-approvals.test.ts` contains a test for `$()` inside double quotes (line 137-141):

```typescript
it("rejects command substitution inside double quotes", () => {
    const res = analyzeShellCommand({ command: 'echo "output: $(whoami)"' });
    expect(res.ok).toBe(false);
    expect(res.reason).toBe("unsupported shell token: $()");
});
```

However, there is NO test for the line-continuation bypass variant (`$\<LF>(cmd)`). This confirms the bypass is not accounted for.

### Analysis Process
- Q1: Does user-controllable input reach a command execution sink? → **YES**. The `rawCommand` parameter in `system.run` flows from the gateway/agent request (`src/agents/bash-tools.exec.ts:1122`) through `node.invoke` to `handleInvoke` in `src/node-host/invoke.ts:539`, and finally to `spawn(["/bin/sh", "-lc", rawCommand])` at `src/node-host/invoke.ts:792` → `src/infra/node-shell.ts:8`.
- Q2: Is it code evaluation (eval/exec)? → **No**. It's process execution via `spawn` with shell.
- Q3: Is the command executed in array form without shell? → **No**. The command is executed as `spawn("/bin/sh", ["-lc", command])` — the shell IS invoked (`src/infra/node-shell.ts:8`).
- Q4: Is the user input effectively sanitized? → **No**. The `evaluateShellAllowlist`/`splitShellPipeline` parser fails to detect `$()` when `$` and `(` are separated by `\<newline>` inside double quotes, because: (a) the `$(...)` check at line 399-401 requires `$` immediately followed by `(` — bypassed by `$\<newline>(`; (b) the `DISALLOWED_PIPELINE_TOKENS` check at line 472-474 for `(` is outside the `inDouble` block — bypassed when inside double quotes.
- Q5: Is the code in test/demo/dead/generated context? → **No**. The vulnerable code is in production paths: `src/infra/exec-approvals-analysis.ts` (parser), `src/node-host/invoke.ts` (node host handler), and `src/agents/bash-tools.exec.ts` (agent exec tool).
- → Reached leaf: **TRUE_POSITIVE**

## 3. Verdict
**TRUE_POSITIVE**

**Key Evidence:**
- The `splitShellPipeline` parser at `src/infra/exec-approvals-analysis.ts:399-401` only checks for `$(` immediately adjacent, allowing `$\<newline>(` to bypass detection.
- The `DOUBLE_QUOTE_ESCAPES` set at `src/infra/exec-approvals-analysis.ts:251` includes `\n`, causing `\<newline>` inside double quotes to be consumed as a valid escape rather than rejected.
- The `DISALLOWED_PIPELINE_TOKENS` check for `(` at `src/infra/exec-approvals-analysis.ts:472-474` is outside the `inDouble` block, so `(` inside double quotes after the escaped newline is not rejected.
- The shell is explicitly invoked as `/bin/sh -lc <command>` (`src/infra/node-shell.ts:8`), so bash interprets `\<newline>` as line continuation and folds `$` and `(` together into executable `$(cmd)`.
- Version `2026.2.13` (`package.json:3`) precedes the fix version `2026.2.22` stated in CVE-2026-28460.

## 4. Remediation Suggestions
**Since this is a TRUE_POSITIVE**, the following remediation is recommended:

1. **Patch `splitShellPipeline`** in `src/infra/exec-approvals-analysis.ts` to detect the line-continuation bypass. Inside double quotes, when `\<newline>` is consumed as a double-quote escape (line 392-397), the parser should check whether the character AFTER the consumed newline is `(` and the character BEFORE the `\` was `$`. If so, this should be treated as a `$()` command substitution and rejected.

2. **Add additional pre-scanning** for `$` anywhere in the command followed (possibly with line continuations) by `(`. A simpler approach: before the character-by-character parser runs, strip all `\<newline>` sequences from the command (since bash treats them as line continuations) and then re-check for `$()` and other dangerous tokens.

3. **Add test coverage** in `src/infra/exec-approvals.test.ts` for the line-continuation bypass variant (`echo "$\<LF>(whoami)"`) to ensure the fix is effective and regressions are caught.

4. **Consider hardening the node execution** so that the allowlist analysis is performed on the command string AFTER line continuations are resolved, ensuring consistency between what is analyzed and what the shell executes.
