# Vulnerability Verification Report

## 1. Summary
- **Vulnerability Type**: rce (CWE-78, OS Command Injection / allowlist bypass)
- **Flagged Location**: `src/infra/exec-approvals-analysis.ts` — `splitShellPipeline` (lines 270–503) and `splitCommandChain` (lines 925–1024), consumed by `evaluateShellAllowlist` (lines 1036–1123)
- **Vulnerability Description**: OpenClaw versions prior to 2026.2.22 contain an allowlist bypass in `system.run` whereby attackers can split `$(...)` command substitution across a `\<newline>` line-continuation sequence inside double quotes. The static shell analyzer fails to recognize that `$` immediately followed by `\<newline>` followed by `(` reassembles into `$(` after the shell removes the line continuation, so the analyzer accepts the command as a single allowlisted invocation while the real shell still performs command substitution at execution time.

## 2. Analysis Logic

### Step 1: Examining the analyzer sink in `src/infra/exec-approvals-analysis.ts:391-414`
`splitShellPipeline` is the core shell-tokenizing routine used by `evaluateShellAllowlist` to decide whether a command satisfies the allowlist. Inside double quotes it explicitly rejects `$(`, backticks and bare newlines:

```ts
if (inDouble) {
  if (ch === "\\" && isDoubleQuoteEscape(next)) {
    buf += ch;
    buf += next;
    i += 1;
    emptySegment = false;
    continue;
  }
  if (ch === "$" && next === "(") {
    return { ok: false, reason: "unsupported shell token: $()", segments: [] };
  }
  if (ch === "`") {
    return { ok: false, reason: "unsupported shell token: `", segments: [] };
  }
  if (ch === "\n" || ch === "\r") {
    return { ok: false, reason: "unsupported shell token: newline", segments: [] };
  }
  ...
}
```

`DOUBLE_QUOTE_ESCAPES` (line 251) includes `"\n"` and `"\r"`:
```ts
const DOUBLE_QUOTE_ESCAPES = new Set(["\\", '"', "$", "`", "\n", "\r"]);
```

So a `\<newline>` pair is treated as a valid in-quote escape sequence (line-continuation). The `$(` rejection only triggers when `$` is *immediately* followed by `(`. There is no lookahead that re-combines `$` + `\<newline>` + `(` into `$(`. The same blind spot exists in `splitCommandChain` (lines 965-977) which uses the identical `isDoubleQuoteEscape` escape rule.

### Step 2: Confirming the intended protection via tests in `src/infra/exec-approvals.test.ts:137-159`
The analyzer is *supposed* to reject command substitution inside double quotes:
```ts
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: $()");
});
```
That test passes only because `$` and `(` are adjacent. The bypass payload `$\<newline>(` is *not* covered by any test, and the analyzer logic above does not reject it.

### Step 3: Walking the bypass payload through the analyzer
For `echo "text$\n(whoami)"` (with a literal newline between `\` and `(`), tracing `splitShellPipeline`:
- At the `$` (inside double quotes): `next` is `\\`, so the `ch === "$" && next === "("` check fails; `$` is appended to `buf`.
- At the `\\`: `next` is `\n`. `isDoubleQuoteEscape("\n")` returns `true`, so the escape branch runs, appending both `\\` and `\n` to `buf` and skipping past the newline (`i += 1; continue;`, lines 392–397).
- At the `(`: still inside double quotes. None of the `$(`, backtick, or newline checks fire (the `(` is not `$` and not a newline); `(` is appended to `buf` like any other in-quote character.

Result: `analysisOk === true`, and `tokenizeShellSegment` (lines 585–652) builds `argv = ["echo", "text$\n(whoami)"]` (with the literal newline preserved). If `echo` resolves to a path matching the allowlist, `evaluateExecAllowlist` returns `allowlistSatisfied: true`. The single new test case `echo "text$\n(whoami)"` would pass the analyzer exactly like the existing `echo "output: \$(whoami)"` test at line 156 — but the two have radically different shell semantics.

### Step 4: Confirming a real shell executes the command on both gateway and node paths
The gateway path in `src/agents/bash-tools.exec.ts:555-580` spawns the command via a real shell:
```ts
const { shell, args: shellArgs } = getShellConfig();
const { child: spawned } = await spawnWithFallback({
  argv: [shell, ...shellArgs, opts.command],
  ...
});
```
`getShellConfig()` (`src/agents/shell-utils.ts:22-50`) returns `["sh", ["-c"]]` (or `$SHELL -c`) on Unix, so `opts.command` is interpreted by `/bin/sh -c`.

The node path constructs argv through `buildNodeShellCommand` in `src/infra/node-shell.ts:1-9`:
```ts
export function buildNodeShellCommand(command: string, platform?: string | null) {
  ...
  return ["/bin/sh", "-lc", command];
}
```
That argv is forwarded to `system.run` (`src/agents/bash-tools.exec.ts:1051,1121-1122`) and ultimately spawned by `runCommand` in `src/node-host/invoke.ts:241`:
```ts
const child = spawn(argv[0], argv.slice(1), {
  cwd,
  env,
  stdio: ["ignore", "pipe", "pipe"],
  windowsHide: true,
});
```
So both execution paths feed the raw command string to `/bin/sh -c` (or `-lc`), where line-continuation processing inside double quotes removes the `\<newline>` pair. After reassembly the shell sees `echo "text$(whoami)"` and runs `whoami` via command substitution.

### Step 5: Confirming `rawCommand` is what the analyzer inspects
In `src/node-host/invoke.ts:539-554` the analyzer runs against the user-controlled `rawCommand`, not the argv:
```ts
if (rawCommand) {
  const allowlistEval = evaluateShellAllowlist({
    command: rawCommand,
    ...
```
`rawCommand` is supplied by the caller as `params.command` (`src/agents/bash-tools.exec.ts:1122`), i.e. the attacker-controlled shell string. There is no normalization that would collapse `\<newline>` before analysis.

### Step 6: Checking for any framework-level mitigation
There is no upstream shell-escaping layer between the analyzer and `spawn`. `validateHostEnv` (`src/agents/bash-tools.exec.ts:83-107`) only filters environment variables; it does not touch the command string. `sanitizeEnv` in `src/node-host/invoke.ts:137-175` likewise only filters env. No `shell:false` re-execution, no `bash -p` restriction, and no sandboxing is applied to the gateway or node execution paths when the allowlist is satisfied. The allowlist analyzer is the only line of defense, and Step 3 shows it can be bypassed.

### Step 7: Confirming the codebase is the vulnerable pre-fix version
`package.json:3` declares `"version": "2026.2.13"`, and `CHANGELOG.md:3` marks `## 2026.2.13 (Unreleased)`. Both are earlier than the CVE's fixed version `2026.2.22`, so the unfixed analyzer in this tree is the code the CVE applies to.

### Analysis Process
- Q1 (Is there a dangerous sink reachable with user-controlled input?) → YES — `/bin/sh -c <rawCommand>` on gateway (`src/agents/bash-tools.exec.ts:557`) and `/bin/sh -lc <command>` on node (`src/infra/node-shell.ts:8`, spawned at `src/node-host/invoke.ts:241`).
- Q2 (Does an allowlist/sanitizer gate the input?) → YES — `evaluateShellAllowlist` in `src/infra/exec-approvals-analysis.ts:1036` is the gate.
- Q3 (Is the sanitizer effective for this input shape?) → NO — `splitShellPipeline` (lines 391-414) only rejects the literal two-byte sequence `$(`. It does not account for `$` + `\<newline>` + `(` reassembling into `$(` after shell line-continuation removal, because `\<newline>` is consumed as a valid in-double-quote escape (lines 392-397) and the subsequent `(` is treated as ordinary in-quote text.
- Q4 (Is the bypass payload actually uninterpreted by the analyzer but interpreted by the shell?) → YES — traced in Step 3 for the analyzer and Step 4 for `/bin/sh -c`; shell semantics remove `\<newline>` inside double quotes, producing `$(...)`.
- Q5 (Is this code path reachable from a real caller, not dead/test code?) → YES — `handleInvoke` in `src/node-host/invoke.ts:488-797` invokes the analyzer on every `system.run` request from the gateway; `createExecTool` in `src/agents/bash-tools.exec.ts:810-1687` invokes it for `host === "gateway"` and `host === "node"` for every agent `exec` call.
- → Reached leaf: TRUE_POSITIVE

## 3. Verdict
**TRUE_POSITIVE**

**Key Evidence:**
- `src/infra/exec-approvals-analysis.ts:391-414` — inside double quotes, the only `$(` rejection is the exact two-byte check `ch === "$" && next === "("`; `\<newline>` is silently consumed as an escape (lines 392-397), so `$\<newline>(` is never recognized as `$(`.
- `src/agents/bash-tools.exec.ts:555-580` and `src/infra/node-shell.ts:8` (spawned at `src/node-host/invoke.ts:241`) — both execution paths feed the raw command to `/bin/sh -c`/`-lc`, where `\<newline>` inside double quotes is removed, reassembling the bypass into `$(...)` and triggering command substitution.
- `src/infra/exec-approvals.test.ts:137-159` — the test suite proves `$(whoami)` inside double quotes is *intended* to be rejected, but no test exercises the `$\<newline>(` reassembly, confirming the gap.
- `package.json:3` (`"version": "2026.2.13"`) and `CHANGELOG.md:3` (`2026.2.13 (Unreleased)`) — version is earlier than the CVE's fixed version `2026.2.22`.

## 4. Remediation Suggestions
- In `splitShellPipeline` and `splitCommandChain` (`src/infra/exec-approvals-analysis.ts`), after consuming a `\<newline>` escape inside double quotes, re-check the character that *follows* the escape: if the buffer's last meaningful character is `$` and the next character is `(`, reject with `unsupported shell token: $() (line-continuation)`. Equivalently, before appending `$` to `buf` inside double quotes, look ahead past any `\<newline>` pairs and reject if the next significant character is `(`. Apply the same lookahead for `` ` `` (backtick) and `$(` to cover `\`\<newline>`\` and `$`\<newline>`(`.
- Generalize the existing `$(` rejection to "post-escape-removal" form: when an escape branch consumes `\\` + `\n` (or `\\` + `\r`), peek at the next char and verify it does not turn the preceding `$` into `$(`.
- Add regression tests that exercise `echo "text$\n(whoami)"`, `echo "text$\r(whoami)"`, and `` echo "text`$\n(whoami)`" `` through both `analyzeShellCommand` and `evaluateShellAllowlist`, asserting `ok === false` with the `$()` reason. Also assert that the existing `echo "output: \$(whoami)"` test (escaped `$`, line 156) still passes, so the fix does not over-block legitimate `\$`.
- Consider defense-in-depth: when `host === "gateway"` or `host === "node"` and `security === "allowlist"`, prefer executing the parsed `segments[].argv` directly via `spawn` (no `-c`) instead of re-emitting `rawCommand` to `/bin/sh -c`, so that even if the analyzer misses a metacharacter, the shell never gets a chance to interpret it. The Windows branch at `src/node-host/invoke.ts:778-790` already does this; Unix should mirror it.
- Re-run the full `exec-approvals.test.ts` suite plus the new regression tests after the fix to ensure no false positives on heredoc, pipeline, and chain tests.
