From 3072c9c62d8f5f798b4eb67de6e9a0f5eb00762b Mon Sep 17 00:00:00 2001 From: Connor Peet Date: Sat, 13 Jul 2024 12:00:47 -0700 Subject: [PATCH] fix: adopt changes required for CVE-2024-27980 patch Generalizes the formatSubprocessArguments function to all cases. Fixes EINVAL error in selfhost debugging. --- CHANGELOG.md | 1 + src/common/processUtils.ts | 50 ++++++++++++++++++- src/targets/browser/launcher.ts | 12 ++++- src/targets/node/subprocessProgramLauncher.ts | 42 +--------------- 4 files changed, 62 insertions(+), 43 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 6ccd15987..12f5a2079 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,7 @@ This changelog records changes to stable releases since 1.50.2. "TBA" changes he - fix: automatically guess outFiles in extension development ([#2032](https://github.com/microsoft/vscode-js-debug/issues/2032)) - fix: breakpoints at unmapped locations setting in wrong locations ([vscode#219031](https://github.com/microsoft/vscode/issues/219031)) - fix: debug targets orphaned when a detached child starts after a parent exits ([vscode#219673](https://github.com/microsoft/vscode/issues/219673)) +- fix: adopt changes required for CVE-2024-27980 patch ## v1.91 (June 2024) diff --git a/src/common/processUtils.ts b/src/common/processUtils.ts index b5d7de0b6..bd66993fd 100644 --- a/src/common/processUtils.ts +++ b/src/common/processUtils.ts @@ -4,6 +4,7 @@ import { spawn, SpawnOptionsWithoutStdio } from 'child_process'; import { ExecaReturnValue } from 'execa'; +import { platformPathToPreferredCase } from './urlUtils'; /** * Thrown from {@link spawnAsync} if an error or non-zero exit code occurs. @@ -39,7 +40,13 @@ export function spawnAsync( options?: SpawnOptionsWithoutStdio, ): Promise<{ stdout: string; stderr: string }> { return new Promise((resolve, reject) => { - const process = spawn(command, args, { ...options, stdio: 'pipe' }); + const fmt = formatSubprocessArguments(command, args, options?.cwd); + const process = spawn(fmt.executable, fmt.args, { + ...options, + cwd: fmt.cwd, + shell: fmt.shell, + stdio: 'pipe', + }); const stderr: Buffer[] = []; const stdout: Buffer[] = []; @@ -68,3 +75,44 @@ export function spawnAsync( ); }); } + +const windowsShellScriptRe = /\.(bat|cmd)$/i; + +/** + * Formats arguments to avoid issues on Windows: + * + * - CVE-2024-27980 mitigation https://github.com/nodejs/node/issues/52554 + * - https://github.com/microsoft/vscode/issues/45832 + * Note that the handling for CVE-2024-27980 applies the behavior from this + * issue to a superset of cases for which it originally applied. + * + * Originally from https://github.com/microsoft/vscode-node-debug/blob/47747454bc6e8c9e48d8091eddbb7ffb54a19bbe/src/node/nodeDebug.ts#L1120 + */ +export const formatSubprocessArguments = ( + executable: string, + args: ReadonlyArray, + cwd?: string | URL, +) => { + if (process.platform === 'win32') { + executable = platformPathToPreferredCase(executable); + if (cwd) { + cwd = platformPathToPreferredCase(cwd.toString()); + } + + if (executable.endsWith('.ps1')) { + args = ['-File', executable, ...args]; + executable = 'powershell.exe'; + } + } + + if (process.platform !== 'win32' || !windowsShellScriptRe.test(executable)) { + return { executable, args, shell: false, cwd }; + } + + return { + executable: `"${executable}"`, + args: args.map(a => (a.includes(' ') ? `"${a}"` : a)), + shell: true, + cwd, + }; +}; diff --git a/src/targets/browser/launcher.ts b/src/targets/browser/launcher.ts index 8e2bf09cc..4b9493726 100644 --- a/src/targets/browser/launcher.ts +++ b/src/targets/browser/launcher.ts @@ -11,6 +11,7 @@ import { IDisposable } from '../../common/disposable'; import { EnvironmentVars } from '../../common/environmentVars'; import { canAccess } from '../../common/fsUtils'; import { ILogger, LogTag } from '../../common/logging'; +import { formatSubprocessArguments } from '../../common/processUtils'; import { delay } from '../../common/promiseUtil'; import Dap from '../../dap/api'; import { browserProcessExitedBeforePort } from '../../dap/errors'; @@ -110,10 +111,17 @@ export async function launch( } else { logger.info(LogTag.RuntimeLaunch, `Launching Chrome from ${executablePath}`); - const cp = childProcess.spawn(executablePath, browserArguments.toArray(), { + const formatted = formatSubprocessArguments( + executablePath, + browserArguments.toArray(), + (await canAccess(fsPromises, cwd)) ? cwd : process.cwd(), + ); + + const cp = childProcess.spawn(formatted.executable, formatted.args, { detached: true, env: env.defined(), - cwd: (await canAccess(fsPromises, cwd)) ? cwd : process.cwd(), + shell: formatted.shell, + cwd: formatted.cwd, stdio, }) as childProcess.ChildProcessWithoutNullStreams; diff --git a/src/targets/node/subprocessProgramLauncher.ts b/src/targets/node/subprocessProgramLauncher.ts index fe27a4fee..b5cbff63d 100644 --- a/src/targets/node/subprocessProgramLauncher.ts +++ b/src/targets/node/subprocessProgramLauncher.ts @@ -6,8 +6,8 @@ import { ChildProcessWithoutNullStreams, spawn } from 'child_process'; import { inject, injectable } from 'inversify'; import { EnvironmentVars } from '../../common/environmentVars'; import { ILogger } from '../../common/logging'; +import { formatSubprocessArguments } from '../../common/processUtils'; import { StreamSplitter } from '../../common/streamSplitter'; -import * as urlUtils from '../../common/urlUtils'; import { INodeLaunchConfiguration, OutputSource } from '../../configuration'; import Dap from '../../dap/api'; import { ILaunchContext } from '../targets'; @@ -30,7 +30,7 @@ export class SubprocessProgramLauncher implements IProgramLauncher { config: INodeLaunchConfiguration, context: ILaunchContext, ) { - const { executable, args, shell, cwd } = formatArguments( + const { executable, args, shell, cwd } = formatSubprocessArguments( binary, getNodeLaunchArgs(config), config.cwd, @@ -118,44 +118,6 @@ export class SubprocessProgramLauncher implements IProgramLauncher { } } -// Fix for: https://github.com/microsoft/vscode/issues/45832, -// which still seems to be a thing according to the issue tracker. -// From: https://github.com/microsoft/vscode-node-debug/blob/47747454bc6e8c9e48d8091eddbb7ffb54a19bbe/src/node/nodeDebug.ts#L1120 -const formatArguments = (executable: string, args: ReadonlyArray, cwd: string) => { - if (process.platform === 'win32') { - executable = urlUtils.platformPathToPreferredCase(executable); - cwd = urlUtils.platformPathToPreferredCase(cwd); - - if (executable.endsWith('.ps1')) { - args = ['-File', executable, ...args]; - executable = 'powershell.exe'; - } - } - - if (process.platform !== 'win32' || !executable.includes(' ')) { - return { executable, args, shell: false, cwd }; - } - - let foundArgWithSpace = false; - - // check whether there is one arg with a space - const output: string[] = []; - for (const a of args) { - if (a.includes(' ')) { - output.push(`"${a}"`); - foundArgWithSpace = true; - } else { - output.push(a); - } - } - - if (foundArgWithSpace) { - return { executable: `"${executable}"`, args: output, shell: true, cwd: cwd }; - } - - return { executable, args, shell: false, cwd }; -}; - const enum Char { ETX = 3, }