From a54bba6ee0c7f8d2faae07c2c9873f6d018e919b Mon Sep 17 00:00:00 2001 From: orlyjamie <125909656+theonejvo@users.noreply.github.com> Date: Thu, 19 Feb 2026 03:22:03 +1100 Subject: [PATCH] fix(security): block command substitution in unquoted heredoc bodies The shell command analyzer (splitShellPipeline) skipped all token validation while parsing heredoc bodies. When the heredoc delimiter was unquoted, bash performs command substitution on the body content, allowing $(cmd) and backtick expressions to execute arbitrary commands that bypass the exec allowlist. Track whether heredoc delimiters are quoted or unquoted. When unquoted, scan the body for $( , ${ , and backtick tokens and reject the command. Quoted heredocs (<<'EOF' / <<"EOF") are safe - the shell treats their body as literal text. Ref: https://github.com/openclaw/openclaw/security/advisories/GHSA-65rx-fvh6-r4h2 --- src/infra/exec-approvals-analysis.ts | 22 +++++++++-- src/infra/exec-approvals.test.ts | 56 ++++++++++++++++++++++++++++ 2 files changed, 74 insertions(+), 4 deletions(-) diff --git a/src/infra/exec-approvals-analysis.ts b/src/infra/exec-approvals-analysis.ts index 0a8340eef5..6c1251ce13 100644 --- a/src/infra/exec-approvals-analysis.ts +++ b/src/infra/exec-approvals-analysis.ts @@ -279,12 +279,13 @@ function splitShellPipeline(command: string): { ok: boolean; reason?: string; se type HeredocSpec = { delimiter: string; stripTabs: boolean; + quoted: boolean; }; const parseHeredocDelimiter = ( source: string, start: number, - ): { delimiter: string; end: number } | null => { + ): { delimiter: string; end: number; quoted: boolean } | null => { let i = start; while (i < source.length && (source[i] === " " || source[i] === "\t")) { i += 1; @@ -309,7 +310,7 @@ function splitShellPipeline(command: string): { ok: boolean; reason?: string; se continue; } if (ch === quote) { - return { delimiter, end: i + 1 }; + return { delimiter, end: i + 1, quoted: true }; } delimiter += ch; i += 1; @@ -329,7 +330,7 @@ function splitShellPipeline(command: string): { ok: boolean; reason?: string; se if (!delimiter) { return null; } - return { delimiter, end: i }; + return { delimiter, end: i, quoted: false }; }; const segments: string[] = []; @@ -371,6 +372,19 @@ function splitShellPipeline(command: string): { ok: boolean; reason?: string; se i += 1; } } else { + // When the heredoc delimiter is unquoted, the shell performs expansion + // on the body (variable substitution, command substitution, etc.). + // Reject commands that use unquoted heredocs containing expansion tokens + // to prevent allowlist bypass via embedded command substitution. + const currentHeredoc = pendingHeredocs[0]; + if (currentHeredoc && !currentHeredoc.quoted) { + if (ch === "`") { + return { ok: false, reason: "command substitution in unquoted heredoc", segments: [] }; + } + if (ch === "$" && (next === "(" || next === "{")) { + return { ok: false, reason: "command substitution in unquoted heredoc", segments: [] }; + } + } heredocLine += ch; } continue; @@ -471,7 +485,7 @@ function splitShellPipeline(command: string): { ok: boolean; reason?: string; se const parsed = parseHeredocDelimiter(command, scanIndex); if (parsed) { - pendingHeredocs.push({ delimiter: parsed.delimiter, stripTabs }); + pendingHeredocs.push({ delimiter: parsed.delimiter, stripTabs, quoted: parsed.quoted }); buf += command.slice(scanIndex, parsed.end); i = parsed.end - 1; } diff --git a/src/infra/exec-approvals.test.ts b/src/infra/exec-approvals.test.ts index bbb73a4a04..633f38e46c 100644 --- a/src/infra/exec-approvals.test.ts +++ b/src/infra/exec-approvals.test.ts @@ -278,6 +278,62 @@ describe("exec approvals shell parsing", () => { expect(res.segments[0]?.argv[0]).toBe("/usr/bin/cat"); }); + it("rejects command substitution in unquoted heredoc body", () => { + const res = analyzeShellCommand({ + command: "/usr/bin/cat < { + const res = analyzeShellCommand({ + command: "/usr/bin/cat < { + const res = analyzeShellCommand({ + command: "/usr/bin/cat < { + const res = analyzeShellCommand({ + command: "/usr/bin/cat <<'EOF'\n$(id)\nEOF", + }); + expect(res.ok).toBe(true); + expect(res.segments[0]?.argv[0]).toBe("/usr/bin/cat"); + }); + + it("allows command substitution in double-quoted heredoc body (shell ignores it)", () => { + const res = analyzeShellCommand({ + command: '/usr/bin/cat <<"EOF"\n$(id)\nEOF', + }); + expect(res.ok).toBe(true); + expect(res.segments[0]?.argv[0]).toBe("/usr/bin/cat"); + }); + + it("rejects nested command substitution in unquoted heredoc", () => { + const res = analyzeShellCommand({ + command: "/usr/bin/cat < { + const res = analyzeShellCommand({ + command: "/usr/bin/cat < { const res = analyzeShellCommand({ command: "/usr/bin/echo first line\n/usr/bin/echo second line",