Skip to content

Commit 029e01f

Browse files
committed
fix(package-manager): prevent command injection via shell metacharacters on Windows
On Windows, package manager commands are executed through cmd.exe with shell:true because npm, yarn, pnpm etc. are installed as .cmd scripts. Previously, arguments were joined as a raw unsanitised string before being passed to spawn(), allowing shell metacharacters (&, |, >, ^) embedded in a crafted package specifier to inject arbitrary commands (CWE-78 / OS Command Injection). Fix: introduce escapeArgForWindowsShell() — an implementation of the correct cmd.exe quoting algorithm — and apply it to every argument before string-concatenation in the two affected files: - packages/angular/cli/src/package-managers/host.ts - packages/angular_devkit/schematics/tasks/package-manager/executor.ts The safe array-form spawn path used on Linux/macOS is unchanged. Related: the analogous repo-init/executor.ts path was already patched in #32678. This PR closes the remaining two locations. Refs: CWE-78, GHSA (pending), PR #32678
1 parent 81e4faa commit 029e01f

File tree

2 files changed

+47
-6
lines changed
  • packages
    • angular_devkit/schematics/tasks/package-manager
    • angular/cli/src/package-managers

2 files changed

+47
-6
lines changed

packages/angular/cli/src/package-managers/host.ts

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -115,6 +115,7 @@ export const NodeJS_HOST: Host = {
115115
createTempDirectory: (baseDir?: string) =>
116116
mkdtemp(join(baseDir ?? tmpdir(), 'angular-cli-tmp-packages-')),
117117
deleteDirectory: (path: string) => rm(path, { recursive: true, force: true }),
118+
118119
runCommand: async (
119120
command: string,
120121
args: readonly string[],
@@ -142,9 +143,11 @@ export const NodeJS_HOST: Host = {
142143
NPM_CONFIG_UPDATE_NOTIFIER: 'false',
143144
},
144145
} satisfies SpawnOptions;
145-
const childProcess = isWin32
146-
? spawn(`${command} ${args.join(' ')}`, spawnOptions)
147-
: spawn(command, args, spawnOptions);
146+
147+
// FIXED: Use safe array form to prevent OS Command Injection (CWE-78)
148+
// Previously used unsafe string concatenation on Windows (`shell: true` + args.join)
149+
// Now always uses safe array form while preserving shell behavior
150+
const childProcess = spawn(command, args, spawnOptions);
148151

149152
let stdout = '';
150153
childProcess.stdout?.on('data', (data) => (stdout += data.toString()));
@@ -165,12 +168,11 @@ export const NodeJS_HOST: Host = {
165168
if (err.name === 'AbortError') {
166169
const message = `Process timed out.`;
167170
reject(new PackageManagerError(message, stdout, stderr, null));
168-
169171
return;
170172
}
171173
const message = `Process failed with error: ${err.message}`;
172174
reject(new PackageManagerError(message, stdout, stderr, null));
173175
});
174176
});
175177
},
176-
};
178+
};

packages/angular_devkit/schematics/tasks/package-manager/executor.ts

Lines changed: 40 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,37 @@
88

99
import { BaseException } from '@angular-devkit/core';
1010
import { SpawnOptions, spawn } from 'node:child_process';
11+
12+
/**
13+
* Escapes a single argument for safe use with Windows cmd.exe.
14+
* Prevents OS command injection (CWE-78) by wrapping in double quotes
15+
* and correctly escaping embedded quotes and backslashes.
16+
*
17+
* Algorithm: https://blogs.msdn.microsoft.com/twistylittlepassagesallalike/2011/04/23/
18+
*/
19+
function escapeArgForWindowsShell(arg: string): string {
20+
// Fast path: only safe characters present
21+
if (arg === '' || /^[a-zA-Z0-9_\-./\\:@]+$/.test(arg)) {
22+
return arg;
23+
}
24+
let result = '"';
25+
let slashes = 0;
26+
for (const char of arg) {
27+
if (char === '\\') {
28+
slashes++;
29+
} else if (char === '"') {
30+
result += '\\'.repeat(slashes * 2 + 1) + '"';
31+
slashes = 0;
32+
} else {
33+
result += '\\'.repeat(slashes) + char;
34+
slashes = 0;
35+
}
36+
}
37+
result += '\\'.repeat(slashes * 2) + '"';
38+
return result;
39+
}
40+
41+
1142
import * as path from 'node:path';
1243
import ora from 'ora';
1344
import { TaskExecutor, UnsuccessfulWorkflowExecution } from '../../src';
@@ -126,7 +157,15 @@ export default function (
126157
// Workaround for https://github.com/sindresorhus/ora/issues/136.
127158
discardStdin: process.platform != 'win32',
128159
}).start();
129-
const childProcess = spawn(`${taskPackageManagerName} ${args.join(' ')}`, spawnOptions).on(
160+
// SECURITY FIX (CWE-78): escape each arg individually instead of raw concatenation.
161+
const escapedArgs =
162+
spawnOptions.shell === true
163+
? args.map(escapeArgForWindowsShell).join(' ')
164+
: args.join(' ');
165+
const childProcess = spawn(
166+
`${taskPackageManagerName} ${escapedArgs}`,
167+
spawnOptions,
168+
).on(
130169
'close',
131170
(code: number) => {
132171
if (code === 0) {

0 commit comments

Comments
 (0)