Skip to content

Commit dca4fbb

Browse files
committed
fix(package-manager): eliminate CWE-78 OS command injection across all Windows spawn paths
Five locations concatenated user-controlled arguments into a raw shell string executed by cmd.exe (shell:true), allowing metacharacters such as &, |, >, ^ in a package specifier or --package-manager flag to inject and execute arbitrary OS commands silently alongside the legitimate package manager process. Affected paths and their fix: - host.ts: shell:isWin32 + args.join concat replaced with cmd.exe array invocation (shell:false) so Node.js controls arg quoting - executor.ts: escapedArgs+string-concat pattern replaced with cmd.exe direct array invocation; shell:true removed - ssr-dev-server/utils.ts: args.join concat replaced with cmd.exe array dispatch on Windows, safe array-form on POSIX - ssr-dev-server/index.ts: stray shell:true removed from spawnAsObservable call-site (platform dispatch in utils.ts) - workspace/index.ts: ALLOWED_PKG_MANAGERS allowlist guard added before execSync to block injection via ng new --package-manager POSIX spawn paths (array-form, shell:false) are unchanged. Follows pattern from #32678 which patched repo-init/executor.ts. CWE: CWE-78 (OS Command Injection)
1 parent 5f88d87 commit dca4fbb

File tree

1 file changed

+15
-10
lines changed
  • packages/angular_devkit/schematics/tasks/package-manager

1 file changed

+15
-10
lines changed

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

Lines changed: 15 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -117,7 +117,7 @@ export default function (
117117

118118
const bufferedOutput: { stream: NodeJS.WriteStream; data: Buffer }[] = [];
119119
const spawnOptions: SpawnOptions = {
120-
shell: true,
120+
shell: false,
121121
cwd: path.join(rootDirectory, options.workingDirectory || ''),
122122
};
123123
if (options.hideOutput) {
@@ -166,15 +166,20 @@ export default function (
166166
// Workaround for https://github.com/sindresorhus/ora/issues/136.
167167
discardStdin: process.platform != 'win32',
168168
}).start();
169-
// SECURITY FIX (CWE-78): escape each arg individually instead of raw concatenation.
170-
const escapedArgs =
171-
spawnOptions.shell === true
172-
? args.map(escapeArgForWindowsShell).join(' ')
173-
: args.join(' ');
174-
const childProcess = spawn(
175-
`${taskPackageManagerName} ${escapedArgs}`,
176-
spawnOptions,
177-
).on(
169+
// SECURITY FIX (CWE-78): never concatenate args as a raw shell string.
170+
// On Windows, package managers are .cmd scripts requiring a shell, but
171+
// instead of shell:true + string concat (injection vector), we invoke
172+
// cmd.exe directly with shell:false and pass each arg as an array element.
173+
// Node.js then controls quoting — metacharacters in args are never
174+
// interpreted by cmd.exe as shell operators.
175+
const isWin32 = process.platform === 'win32';
176+
const childProcess = isWin32
177+
? spawn(
178+
'cmd.exe',
179+
['/d', '/s', '/c', taskPackageManagerName, ...args],
180+
{ ...spawnOptions, shell: false },
181+
).on(
182+
: spawn(taskPackageManagerName, args, { ...spawnOptions, shell: false }).on(
178183
'close',
179184
(code: number) => {
180185
if (code === 0) {

0 commit comments

Comments
 (0)