From 9e0d925f9563bf12d8b3a95e1a238b6eaac1819f Mon Sep 17 00:00:00 2001 From: John Richmond <5629+jr@users.noreply.github.com> Date: Mon, 29 Sep 2025 13:09:26 -0700 Subject: [PATCH] Check for a few more esoteric subshell patterns --- .../__tests__/command-validation.spec.ts | 36 +++++++++++++++++++ webview-ui/src/utils/command-validation.ts | 9 ++++- 2 files changed, 44 insertions(+), 1 deletion(-) diff --git a/webview-ui/src/utils/__tests__/command-validation.spec.ts b/webview-ui/src/utils/__tests__/command-validation.spec.ts index e87bae07e5..58edc86723 100644 --- a/webview-ui/src/utils/__tests__/command-validation.spec.ts +++ b/webview-ui/src/utils/__tests__/command-validation.spec.ts @@ -313,6 +313,30 @@ ls -la || echo "Failed"` expect(containsDangerousSubstitution("rm *(e:sudo apt install malware:)")).toBe(true) }) + it('detects bash $"..." string interpolation with command substitution', () => { + // The exact example from the issue + expect(containsDangerousSubstitution('echo $"test$(whoami)"')).toBe(true) + + // Various forms of command substitution inside $"..." + expect(containsDangerousSubstitution('echo $"Hello $(date)"')).toBe(true) + expect(containsDangerousSubstitution('echo $"User: `whoami`"')).toBe(true) + expect(containsDangerousSubstitution('echo $"test`pwd`"')).toBe(true) + + // Multiple command substitutions + expect(containsDangerousSubstitution('echo $"$(date) - $(whoami)"')).toBe(true) + expect(containsDangerousSubstitution('echo $"User: `whoami` in `pwd`"')).toBe(true) + + // Command substitution at different positions + expect(containsDangerousSubstitution('echo $"$(whoami) is the user"')).toBe(true) + expect(containsDangerousSubstitution('echo $"The user is $(whoami)"')).toBe(true) + expect(containsDangerousSubstitution('echo $"Current $(date) time"')).toBe(true) + + // Complex command substitutions + expect(containsDangerousSubstitution('echo $"Files: $(ls -la)"')).toBe(true) + expect(containsDangerousSubstitution('echo $"Process: $(ps aux | grep node)"')).toBe(true) + expect(containsDangerousSubstitution('echo $"Result: `rm -rf /`"')).toBe(true) + }) + it("does NOT flag safe parameter expansions", () => { // Regular parameter expansions without dangerous operators expect(containsDangerousSubstitution("echo ${var}")).toBe(false) @@ -351,6 +375,15 @@ ls -la || echo "Failed"` expect(containsDangerousSubstitution("rm *.txt")).toBe(false) expect(containsDangerousSubstitution("cat ?(foo|bar)")).toBe(false) expect(containsDangerousSubstitution("echo *(^/)")).toBe(false) // Safe glob qualifier (not e:) + + // Safe $"..." strings without command substitution + expect(containsDangerousSubstitution('echo $"Hello World"')).toBe(false) + expect(containsDangerousSubstitution('echo $"This is a test"')).toBe(false) + expect(containsDangerousSubstitution('echo $"User: $USER"')).toBe(false) // Variable expansion is safe + expect(containsDangerousSubstitution('echo $"Path: ${PATH}"')).toBe(false) // Variable expansion is safe + expect(containsDangerousSubstitution('echo $"Count: $count"')).toBe(false) + expect(containsDangerousSubstitution('echo $"Translated string with no substitution"')).toBe(false) + expect(containsDangerousSubstitution('echo $""')).toBe(false) // Empty translated string }) it("handles complex combinations of dangerous patterns", () => { @@ -379,6 +412,9 @@ ls -la || echo "Failed"` // The zsh glob qualifier exploit expect(containsDangerousSubstitution("ls *(e:whoami:)")).toBe(true) + + // The bash $"..." string interpolation exploit + expect(containsDangerousSubstitution('echo $"test$(whoami)"')).toBe(true) }) }) }) diff --git a/webview-ui/src/utils/command-validation.ts b/webview-ui/src/utils/command-validation.ts index 572ca32bad..3a35b02849 100644 --- a/webview-ui/src/utils/command-validation.ts +++ b/webview-ui/src/utils/command-validation.ts @@ -73,6 +73,7 @@ type ShellToken = string | { op: string } | { command: string } * - <<<$(...) or <<<`...` - Here-strings with command substitution * - =(...) - Zsh process substitution that executes commands * - *(e:...:) or similar - Zsh glob qualifiers with code execution + * - $"..." with command substitution - Bash translated strings with embedded command execution * * @param source - The command string to analyze * @returns true if dangerous substitution patterns are detected, false otherwise @@ -111,6 +112,11 @@ export function containsDangerousSubstitution(source: string): boolean { // This regex matches patterns like *(e:...:), ?(e:...:), +(e:...:), @(e:...:), !(e:...:) const zshGlobQualifier = /[*?+@!]\(e:[^:]+:\)/.test(source) + // Check for $"..." string interpolation with command substitution + // $"..." is a bash feature for translated strings that allows command substitution inside + // e.g., echo $"test$(whoami)" or echo $"test`pwd`" + const bashTranslatedStringWithSubstitution = /\$"[^"]*(\$\(|`)[^"]*"/.test(source) + // Return true if any dangerous pattern is detected return ( dangerousParameterExpansion || @@ -118,7 +124,8 @@ export function containsDangerousSubstitution(source: string): boolean { indirectExpansion || hereStringWithSubstitution || zshProcessSubstitution || - zshGlobQualifier + zshGlobQualifier || + bashTranslatedStringWithSubstitution ) }