From 92cada2acad80b05e3d2f89913d4a7ed5fd43499 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 7971e9b8f..f7c49b532 100644 --- a/src/infra/exec-approvals-analysis.ts +++ b/src/infra/exec-approvals-analysis.ts @@ -338,12 +338,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; @@ -368,7 +369,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; @@ -388,7 +389,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[] = []; @@ -430,6 +431,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; @@ -530,7 +544,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 b1a3d2022..c2158335a 100644 --- a/src/infra/exec-approvals.test.ts +++ b/src/infra/exec-approvals.test.ts @@ -305,6 +305,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",