diff --git a/src/utils/__tests__/shell-escape.test.ts b/src/utils/__tests__/shell-escape.test.ts new file mode 100644 index 00000000..9b72ab82 --- /dev/null +++ b/src/utils/__tests__/shell-escape.test.ts @@ -0,0 +1,70 @@ +import { describe, it, expect } from 'vitest'; +import { shellEscapeArg } from '../shell-escape.ts'; + +describe('shellEscapeArg', () => { + it('wraps a simple string in single quotes', () => { + expect(shellEscapeArg('hello')).toBe("'hello'"); + }); + + it('returns empty single-quoted string for empty input', () => { + expect(shellEscapeArg('')).toBe("''"); + }); + + it('escapes embedded single quotes using the POSIX technique', () => { + expect(shellEscapeArg("it's")).toBe("'it'\\''s'"); + }); + + it('handles multiple single quotes', () => { + expect(shellEscapeArg("a'b'c")).toBe("'a'\\''b'\\''c'"); + }); + + it('passes through double quotes safely inside single quotes', () => { + expect(shellEscapeArg('say "hi"')).toBe('\'say "hi"\''); + }); + + it('neutralises dollar-sign variable expansion', () => { + const escaped = shellEscapeArg('$HOME'); + expect(escaped).toBe("'$HOME'"); + }); + + it('neutralises backtick command substitution', () => { + const escaped = shellEscapeArg('`id`'); + expect(escaped).toBe("'`id`'"); + }); + + it('neutralises $() command substitution', () => { + const escaped = shellEscapeArg('$(whoami)'); + expect(escaped).toBe("'$(whoami)'"); + }); + + it('neutralises semicolon command chaining', () => { + const escaped = shellEscapeArg('foo; rm -rf /'); + expect(escaped).toBe("'foo; rm -rf /'"); + }); + + it('handles newlines (cannot break out of single quotes)', () => { + const escaped = shellEscapeArg('line1\nline2'); + expect(escaped).toBe("'line1\nline2'"); + }); + + it('handles backslashes', () => { + const escaped = shellEscapeArg('path\\to\\file'); + expect(escaped).toBe("'path\\to\\file'"); + }); + + it('handles pipe and redirection metacharacters', () => { + const escaped = shellEscapeArg('a | b > c < d'); + expect(escaped).toBe("'a | b > c < d'"); + }); + + it('handles a realistic malicious scheme name (CWE-78 PoC)', () => { + // An attacker might supply this as a scheme parameter via MCP tool call + const malicious = '$(touch /tmp/pwned)'; + const escaped = shellEscapeArg(malicious); + // The result should be a valid single-quoted string that cannot execute $(touch) + expect(escaped).toBe("'$(touch /tmp/pwned)'"); + // Verify no unquoted regions exist + expect(escaped.startsWith("'")).toBe(true); + expect(escaped.endsWith("'")).toBe(true); + }); +}); diff --git a/src/utils/command.ts b/src/utils/command.ts index 9d85fb3d..7bfc1032 100644 --- a/src/utils/command.ts +++ b/src/utils/command.ts @@ -13,6 +13,7 @@ import { spawn } from 'child_process'; import { createWriteStream, existsSync } from 'fs'; import { tmpdir as osTmpdir } from 'os'; import { log } from './logger.ts'; +import { shellEscapeArg } from './shell-escape.ts'; import type { FileSystemExecutor } from './FileSystemExecutor.ts'; import type { CommandExecutor, CommandResponse, CommandExecOptions } from './CommandExecutor.ts'; @@ -41,16 +42,8 @@ async function defaultExecutor( let escapedCommand = command; if (useShell) { // For shell execution, we need to format as ['/bin/sh', '-c', 'full command string'] - const commandString = command - .map((arg) => { - // Shell metacharacters that require quoting: space, quotes, equals, dollar, backticks, semicolons, pipes, etc. - if (/[\s,"'=$`;&|<>(){}[\]\\*?~]/.test(arg) && !/^".*"$/.test(arg)) { - // Escape all quotes and backslashes, then wrap in double quotes - return `"${arg.replace(/(["\\])/g, '\\$1')}"`; - } - return arg; - }) - .join(' '); + // Use POSIX single-quote escaping for each argument to prevent injection + const commandString = command.map((arg) => shellEscapeArg(arg)).join(' '); escapedCommand = ['/bin/sh', '-c', commandString]; } diff --git a/src/utils/shell-escape.ts b/src/utils/shell-escape.ts new file mode 100644 index 00000000..bb61a0e1 --- /dev/null +++ b/src/utils/shell-escape.ts @@ -0,0 +1,15 @@ +/** + * POSIX-safe shell argument escaping. + * + * Wraps a string in single quotes and escapes any embedded single quotes + * using the standard `'\''` technique. This is the safest way to pass + * arbitrary strings as arguments to `/bin/sh -c` commands. + * + * @param arg The argument to escape for safe shell interpolation + * @returns A single-quoted, safely escaped string + */ +export function shellEscapeArg(arg: string): string { + // Replace each single quote with: end current quote, escaped single quote, start new quote + // Then wrap the whole thing in single quotes + return "'" + arg.replace(/'/g, "'\\''") + "'"; +}