From 3926183650bcd3ea04adf4ad0ecc79d2533af305 Mon Sep 17 00:00:00 2001 From: tufstraka Date: Mon, 15 Jun 2026 05:42:48 +0000 Subject: [PATCH] fix: escape Windows cmd.exe metacharacters in libnpmexec bin names When npx runs a package on Windows, the bin name from package.json is passed to cmd.exe with no escaping. Characters like &, |, and spaces allow a malicious package to inject arbitrary commands. This patch escapes cmd.exe metacharacters (&, |, <, >, ^, (, ), @, !, ", \t) with ^ and rejects bin names containing characters that cannot be safely neutralized: spaces and tabs (cmd.exe always splits on these regardless of escaping), % (env var expansion precedes ^ processing), and newlines/carriage returns (act as command separators). --- workspaces/libnpmexec/lib/run-script.js | 18 ++++ workspaces/libnpmexec/test/run-script.js | 105 +++++++++++++++++++++++ 2 files changed, 123 insertions(+) diff --git a/workspaces/libnpmexec/lib/run-script.js b/workspaces/libnpmexec/lib/run-script.js index 2de40454f9dcf..f143bfbb5f12c 100644 --- a/workspaces/libnpmexec/lib/run-script.js +++ b/workspaces/libnpmexec/lib/run-script.js @@ -23,6 +23,24 @@ const run = async ({ // literally; double quotes still expand $(), backticks, $var and " args[0] = `'${args[0].replace(/'/g, `'\\''`)}'` } + } else { + if (args.length > 0) { + // On Windows, cmd.exe interprets metacharacters (&, |, <, >, ^, (, ), @, !) + // and uses spaces/tabs as token delimiters. Unlike on POSIX where single-quoting + // the entire name makes it an opaque filename, on Windows there is no escaping + // strategy that reliably prevents cmd.exe from splitting on spaces and executing + // the first token as a command. Therefore, reject any bin name containing + // characters that cannot be safely neutralized: + // - spaces/tabs: always split into multiple tokens regardless of ^ or quoting + // - % : cmd.exe expands environment variables (%VAR%) before ^ processing + // - newlines/carriage returns: act as command separators + if (/[%\r\n\t ]/.test(args[0])) { + throw new Error( + `Cannot execute bin name containing unsafe characters on Windows: ${args[0]}` + ) + } + args[0] = args[0].replace(/([&|<>^()@!"])/g, '^$1') + } } // turn list of args into command string diff --git a/workspaces/libnpmexec/test/run-script.js b/workspaces/libnpmexec/test/run-script.js index 5c92ce19397f6..06a665ded5afb 100644 --- a/workspaces/libnpmexec/test/run-script.js +++ b/workspaces/libnpmexec/test/run-script.js @@ -144,6 +144,111 @@ t.test('escapes executable name to neutralize shell metacharacters', async t => t.equal(pkg.scripts.npx, `'evil'\\''; touch pwned #'`) }) +t.test('escapes Windows cmd.exe metacharacters in executable name', async t => { + let pkg + const { runScript } = await mockRunScript(t, { + 'ci-info': { isCI: true }, + '@npmcli/run-script': async (opts) => { + pkg = opts.pkg + }, + '../lib/is-windows.js': true, + }) + + await runScript({ args: ['hello&calc&echo'] }) + t.equal(pkg.scripts.npx, 'hello^&calc^&echo', + 'ampersands escaped with caret to prevent cmd.exe command splitting') +}) + +t.test('escapes all Windows cmd.exe metacharacters', async t => { + let pkg + const { runScript } = await mockRunScript(t, { + 'ci-info': { isCI: true }, + '@npmcli/run-script': async (opts) => { + pkg = opts.pkg + }, + '../lib/is-windows.js': true, + }) + + await runScript({ args: ['a&b|ce^f(g)h@i!j'] }) + t.equal(pkg.scripts.npx, 'a^&b^|c^e^^f^(g^)h^@i^!j', + 'all cmd.exe metacharacters are escaped with caret') +}) + +t.test('escapes spaces in Windows bin names to prevent multi-token injection', async t => { + const { runScript } = await mockRunScript(t, { + 'ci-info': { isCI: true }, + '@npmcli/run-script': async () => { + throw new Error('should not call run-script') + }, + '../lib/is-windows.js': true, + }) + + await t.rejects(runScript({ args: ['powershell -c calc'] }), { + message: /Cannot execute bin name containing unsafe characters on Windows/, + }, 'rejects bin names with spaces to prevent cmd.exe token splitting') +}) + +t.test('escapes double quotes in Windows bin names', async t => { + let pkg + const { runScript } = await mockRunScript(t, { + 'ci-info': { isCI: true }, + '@npmcli/run-script': async (opts) => { + pkg = opts.pkg + }, + '../lib/is-windows.js': true, + }) + + await runScript({ args: ['evil"&calc"'] }) + t.equal(pkg.scripts.npx, 'evil^"^&calc^"', + 'double quotes escaped to prevent opening quoted regions in cmd.exe') +}) + +t.test('escapes tabs in Windows bin names', async t => { + const { runScript } = await mockRunScript(t, { + 'ci-info': { isCI: true }, + '@npmcli/run-script': async () => { + throw new Error('should not call run-script') + }, + '../lib/is-windows.js': true, + }) + + await t.rejects(runScript({ args: ['evil\tcalc'] }), { + message: /Cannot execute bin name containing unsafe characters on Windows/, + }, 'rejects bin names with tabs to prevent cmd.exe token splitting') +}) + +t.test('rejects Windows bin names containing percent sign', async t => { + const { runScript } = await mockRunScript(t, { + 'ci-info': { isCI: true }, + '@npmcli/run-script': async () => { + throw new Error('should not call run-script') + }, + '../lib/is-windows.js': true, + }) + + await t.rejects(runScript({ args: ['%COMSPEC%'] }), { + message: /Cannot execute bin name containing unsafe characters on Windows/, + }, 'rejects bin names with % to prevent env var expansion in cmd.exe') +}) + +t.test('rejects Windows bin names containing newlines', async t => { + const { runScript } = await mockRunScript(t, { + 'ci-info': { isCI: true }, + '@npmcli/run-script': async () => { + throw new Error('should not call run-script') + }, + '../lib/is-windows.js': true, + }) + + await t.rejects(runScript({ args: ['evil\ncalc'] }), { + message: /Cannot execute bin name containing unsafe characters on Windows/, + }, 'rejects bin names with newline to prevent command splitting in cmd.exe') + + await t.rejects(runScript({ args: ['evil\rcalc'] }), { + message: /Cannot execute bin name containing unsafe characters on Windows/, + }, 'rejects bin names with carriage return') +}) + t.test('isNotWindows', async t => { const { runScript } = await mockRunScript(t, { 'ci-info': { isCI: true },