diff --git a/src/vs/workbench/contrib/debug/node/debugAdapter.ts b/src/vs/workbench/contrib/debug/node/debugAdapter.ts index db82c70d89d..ef22dd43528 100644 --- a/src/vs/workbench/contrib/debug/node/debugAdapter.ts +++ b/src/vs/workbench/contrib/debug/node/debugAdapter.ts @@ -17,6 +17,73 @@ import { IDebugAdapterExecutable, IDebugAdapterNamedPipeServer, IDebugAdapterSer import { AbstractDebugAdapter } from '../common/abstractDebugAdapter.js'; import { killTree } from '../../../../base/node/processes.js'; +const windowsBatchUnquotedCharacters = '#$*+-./:?@\\_'; +const windowsBatchInvalidCharacters = /[\0\r\n]/; +const windowsBatchControlCharacter = /\p{Cc}/u; + +function windowsBatchArgumentNeedsQuotes(argument: string): boolean { + if (!argument || argument.endsWith('\\')) { + return true; + } + + for (const character of argument) { + const codePoint = character.codePointAt(0)!; + const isAsciiAlphaNumeric = codePoint >= 0x30 && codePoint <= 0x39 + || codePoint >= 0x41 && codePoint <= 0x5A + || codePoint >= 0x61 && codePoint <= 0x7A; + if (codePoint <= 0x7F && !isAsciiAlphaNumeric && !windowsBatchUnquotedCharacters.includes(character) + || windowsBatchControlCharacter.test(character)) { + return true; + } + } + + return false; +} + +function escapeWindowsBatchArgument(argument: string, forceQuotes = false): string { + const quote = forceQuotes || windowsBatchArgumentNeedsQuotes(argument); + let result = quote ? '"' : ''; + let backslashes = 0; + + for (const character of argument) { + if (character === '\\') { + backslashes++; + } else { + if (character === '"') { + result += '\\'.repeat(backslashes); + result += '"'; + } else if (character === '%') { + result += '%%cd:~,'; + } + backslashes = 0; + } + result += character; + } + + if (quote) { + result += '\\'.repeat(backslashes); + result += '"'; + } + + return result; +} + +/** + * Builds an injection-safe cmd.exe invocation for a Windows batch file. + */ +export function prepareWindowsBatchCommand(command: string, args: readonly string[]): string[] { + if (command.includes('"') || windowsBatchInvalidCharacters.test(command) || args.some(argument => windowsBatchInvalidCharacters.test(argument))) { + throw new Error(nls.localize('invalidWindowsBatchCommand', "Debug adapter commands and arguments contain invalid characters.")); + } + + const shellCommand = [ + escapeWindowsBatchArgument(command, true), + ...args.map(argument => escapeWindowsBatchArgument(argument)) + ].join(' '); + + return ['/e:ON', '/v:OFF', '/d', '/c', `"${shellCommand}"`]; +} + /** * An implementation that communicates via two streams with the debug adapter. */ @@ -236,15 +303,11 @@ export class ExecutableDebugAdapter extends StreamDebugAdapter { if (options.cwd) { spawnOptions.cwd = options.cwd; } - if (platform.isWindows && (command.endsWith('.bat') || command.endsWith('.cmd'))) { + if (platform.isWindows && /\.(bat|cmd)$/i.test(command)) { // https://github.com/microsoft/vscode/issues/224184 - spawnOptions.shell = true; - spawnCommand = `"${command}"`; - spawnArgs = args.map(a => { - a = a.replace(/"/g, '\\"'); // Escape existing double quotes with \ - // Wrap in double quotes - return `"${a}"`; - }); + spawnOptions.windowsVerbatimArguments = true; + spawnCommand = process.env['ComSpec'] || 'cmd.exe'; + spawnArgs = prepareWindowsBatchCommand(command, args); } this.serverProcess = cp.spawn(spawnCommand, spawnArgs, spawnOptions); diff --git a/src/vs/workbench/contrib/debug/test/node/debugAdapter.test.ts b/src/vs/workbench/contrib/debug/test/node/debugAdapter.test.ts new file mode 100644 index 00000000000..958f42c3997 --- /dev/null +++ b/src/vs/workbench/contrib/debug/test/node/debugAdapter.test.ts @@ -0,0 +1,120 @@ +/*--------------------------------------------------------------------------------------------- + * Copyright (c) Microsoft Corporation. All rights reserved. + * Licensed under the MIT License. See License.txt in the project root for license information. + *--------------------------------------------------------------------------------------------*/ + +import assert from 'assert'; +import { spawnSync } from 'child_process'; +import { existsSync } from 'fs'; +import { mkdtemp, readFile, rm, writeFile } from 'fs/promises'; +import { tmpdir } from 'os'; +import { join } from '../../../../../base/common/path.js'; +import { ensureNoDisposablesAreLeakedInTestSuite } from '../../../../../base/test/common/utils.js'; +import { prepareWindowsBatchCommand } from '../../node/debugAdapter.js'; + +suite('Debug - Debug Adapter', () => { + + ensureNoDisposablesAreLeakedInTestSuite(); + + test('escapes Windows batch commands and arguments', () => { + assert.deepStrictEqual( + prepareWindowsBatchCommand( + 'C:\\Program Files\\adapter.cmd', + ['plain', 'with spaces', 'quote" & calc.exe & "', '|<>()^%!', 'C:\\path\\', '%PATH:z=z%'] + ), + [ + '/e:ON', + '/v:OFF', + '/d', + '/c', + '""C:\\Program Files\\adapter.cmd" plain "with spaces" "quote"" & calc.exe & """ "|<>()^%%cd:~,%!" "C:\\path\\\\" "%%cd:~,%PATH:z=z%%cd:~,%""' + ] + ); + }); + + test('escapes backslash runs around quotes', () => { + assert.deepStrictEqual( + prepareWindowsBatchCommand('adapter.cmd', ['two\\\\', 'three\\\\\\', 'two\\\\"quote', 'three\\\\\\"quote']), + [ + '/e:ON', + '/v:OFF', + '/d', + '/c', + '""adapter.cmd" "two\\\\\\\\" "three\\\\\\\\\\\\" "two\\\\\\\\""quote" "three\\\\\\\\\\\\""quote""' + ] + ); + }); + + test('rejects invalid Windows batch command characters', () => { + assert.deepStrictEqual( + [ + () => prepareWindowsBatchCommand('adapter.cmd', ['safe\r\ncalc.exe']), + () => prepareWindowsBatchCommand('adapter.cmd', ['safe\0calc.exe']), + () => prepareWindowsBatchCommand('adapter".cmd', []) + ].map(run => { + try { + run(); + return false; + } catch { + return true; + } + }), + [true, true, true] + ); + }); + + test('round-trips Windows batch arguments without executing metacharacters', async function () { + if (process.platform !== 'win32') { + this.skip(); + } + + const testDirectory = await mkdtemp(join(tmpdir(), 'vscode-debug-adapter-')); + const adapterPath = join(testDirectory, 'adapter.cmd'); + const captureScriptPath = join(testDirectory, 'capture.cjs'); + const outputPath = join(testDirectory, 'arguments.json'); + const sideEffectPath = join(testDirectory, 'side-effect.txt'); + + try { + const roundTripArgs = [ + 'plain', + 'with spaces', + '', + '|<>()^%!', + 'C:\\path\\', + '%PATH:z=z%', + 'two\\\\slashes' + ]; + const args = [...roundTripArgs, `quote" & echo unexpected>"${sideEffectPath}" & "`]; + const forwardedArgs = args.map((_, index) => `"%~${index + 1}"`).join(' '); + await writeFile(adapterPath, `@echo off\r\n"%VSCODE_TEST_NODE%" "%VSCODE_TEST_CAPTURE_SCRIPT%" ${forwardedArgs}\r\n`); + await writeFile(captureScriptPath, 'require("fs").writeFileSync(process.env.VSCODE_TEST_OUTPUT, JSON.stringify(process.argv.slice(2)));'); + + const result = spawnSync(process.env['ComSpec'] || 'cmd.exe', prepareWindowsBatchCommand(adapterPath, args), { + encoding: 'utf8', + env: { + ...process.env, + ELECTRON_RUN_AS_NODE: '1', + VSCODE_TEST_NODE: process.execPath, + VSCODE_TEST_CAPTURE_SCRIPT: captureScriptPath, + VSCODE_TEST_OUTPUT: outputPath + }, + windowsVerbatimArguments: true + }); + const capturedArgs: string[] | undefined = existsSync(outputPath) ? JSON.parse(await readFile(outputPath, 'utf8')) : undefined; + + assert.deepStrictEqual({ + status: result.status, + error: result.error?.message, + capturedArgs: capturedArgs?.slice(0, roundTripArgs.length), + sideEffectCreated: existsSync(sideEffectPath) + }, { + status: 0, + error: undefined, + capturedArgs: roundTripArgs, + sideEffectCreated: false + }); + } finally { + await rm(testDirectory, { recursive: true, force: true }); + } + }); +});