Skip to content

Commit dd8808a

Browse files
committed
fix(core): replace unsafe cmd.exe fallback with PowerShell on Windows
When bash is absent, zx silently fell through to child_process with shell:true — which on Windows means cmd.exe. cmd.exe does not interpret bash-style ANSI-C quoting (\$'...''), so every interpolated argument is a straight injection vector: '& calc &' passes through the quoting layer unmodified and the metacharacters are executed by cmd.exe. Fix: make the bash-not-found path Windows-aware. - Probe PATH for pwsh then powershell.exe in order of preference. - If a PS shell is found, use it with the matching quotePowerShell quoting convention and the '; exit \0' postfix. - If no safe shell is found at all on win32, throw a Fail with a clear diagnostic rather than silently executing in an insecure context. - Non-Windows platforms retain the prior (acceptable) behaviour. - Explicit shell/prefix/postfix CLI overrides are still honoured after auto-detection runs. Reported-by: LAKSHMIKANTHAN K (letchupkt) CWE: CWE-78 (OS Command Injection)
1 parent 98531fc commit dd8808a

File tree

1 file changed

+32
-5
lines changed

1 file changed

+32
-5
lines changed

src/core.ts

Lines changed: 32 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1022,13 +1022,40 @@ function setShell(n: string, ps = true) {
10221022
$.quote = ps ? quotePowerShell : quote
10231023
}
10241024

1025-
try {
1025+
{
1026+
// Preserve any shell/prefix/postfix overrides the caller may have set
1027+
// before we attempt shell detection (set -euo pipefail etc.).
10261028
const { shell, prefix, postfix } = $
1027-
useBash()
1028-
if (isString(shell)) $.shell = shell
1029-
if (isString(prefix)) $.prefix = prefix
1029+
try {
1030+
useBash()
1031+
} catch {
1032+
// bash not found — on Windows we must NOT fall through to cmd.exe: its
1033+
// parser does not honour bash-style $'…' quoting, so every interpolated
1034+
// argument is a potential command-injection vector.
1035+
if (process.platform === 'win32') {
1036+
const winShell = which.sync('pwsh', { nothrow: true })
1037+
?? which.sync('powershell.exe', { nothrow: true })
1038+
if (!winShell) {
1039+
throw new Fail(
1040+
`No safe shell found: 'bash', 'pwsh', and 'powershell.exe' are all absent from PATH. ` +
1041+
`Running under cmd.exe is unsafe because bash-style quoting does not apply there.`
1042+
)
1043+
}
1044+
// pwsh / powershell.exe — use the PowerShell quoting convention and
1045+
// the standard ; exit $LastExitCode postfix.
1046+
$.shell = winShell
1047+
$.prefix = ''
1048+
$.postfix = '; exit $LastExitCode'
1049+
$.quote = quotePowerShell
1050+
}
1051+
// On non-Windows platforms the original behaviour (shell:true via
1052+
// execvp) is acceptable; no action needed.
1053+
}
1054+
// Re-apply explicit caller overrides now that defaults are set.
1055+
if (isString(shell)) $.shell = shell
1056+
if (isString(prefix)) $.prefix = prefix
10301057
if (isString(postfix)) $.postfix = postfix
1031-
} catch (err) {}
1058+
}
10321059

10331060
let cwdSyncHook: AsyncHook
10341061

0 commit comments

Comments
 (0)