Prior pull request (#48726) hardened against prototype pollution
vulnerabilities but effectively missed some use-cases which
opened a window for prototype pollution for some child_process
functions such as spawn(), spawnSync(), and execFileSync().
PR-URL: https://github.com/nodejs/node/pull/53781
Reviewed-By: Vinícius Lourenço Claro Cardoso <contact@viniciusl.com.br>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Rafael Gonzaga <rafael.nunu@hotmail.com>
PR-URL: https://github.com/nodejs/node/pull/52625
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Chemi Atlow <chemi@atlow.co.il>
PR-URL: https://github.com/nodejs/node/pull/48726
Reviewed-By: Matthew Aitken <maitken033380023@gmail.com>
Reviewed-By: Mohammed Keyvanzadeh <mohammadkeyvanzade94@gmail.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
This change adds validation to reject an edge case where the
child_process API argument strings might contain null bytes
somewhere in between. Such strings were being silently truncated
before, so throwing an error should prevent misuses of this API.
Fixes: https://github.com/nodejs/node/issues/44768
Signed-off-by: Darshan Sen <raisinten@gmail.com>
PR-URL: https://github.com/nodejs/node/pull/44782
Reviewed-By: James M Snell <jasnell@gmail.com>
execSync() passes opts.args to checkExecSyncError(). There are
two problems:
1. opts.args appears to always be undefined.
2. Even if it was defined, checkExecSyncError() would not use
it since the command input is guaranteed to be defined.
This commit updates the code to pass undefined instead.
PR-URL: https://github.com/nodejs/node/pull/44766
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Fix issue that will be flagged when we update ESLint and dependencies.
PR-URL: https://github.com/nodejs/node/pull/42489
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Mestery <mestery@protonmail.com>
Reviewed-By: Darshan Sen <raisinten@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
There are a few places in lib where `new Error()` is called and then
additional properties are attached in various ways. This creates a
utility function to generate the errors.
PR-URL: https://github.com/nodejs/node/pull/41879
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
For execFile() and fork(), use INVALID_ARG_TYPE as appropriate instead
of INVALID_ARG_VALUE. Use validator functions where sensible.
PR-URL: https://github.com/nodejs/node/pull/41305
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Minwoo Jung <nodecorelab@gmail.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
execFile() and fork() have complicated argument processing. Clarify code
and avoid using `arguments`.
PR-URL: https://github.com/nodejs/node/pull/41280
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Added JSDoc typings for the `child_process` lib module.
PR-URL: https://github.com/nodejs/node/pull/38222
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Michael Dawson <midawson@redhat.com>
PR-URL: https://github.com/nodejs/node/pull/38927
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Darshan Sen <raisinten@gmail.com>
Reviewed-By: Khaidi Chu <i@2333.moe>
Reviewed-By: Zijian Liu <lxxyxzj@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
The internal validateTimeout() takes a single parameter, so do not pass
a second value.
PR-URL: https://github.com/nodejs/node/pull/37923
Reviewed-By: Zijian Liu <lxxyxzj@gmail.com>
Reviewed-By: Darshan Sen <raisinten@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
PR-URL: https://github.com/nodejs/node/pull/37367
Reviewed-By: Zijian Liu <lxxyxzj@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Move abort signal validation to before spawn is executed
so that the file is not leaked.
PR-URL: https://github.com/nodejs/node/pull/37257
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
The pattern of resolving/rejecting a Promise from outside of its
executor happens numerous times throughout the codebase (more than
what is updated here in fact). This commit abstracts that logic
into an internal utility function.
PR-URL: https://github.com/nodejs/node/pull/37095
Reviewed-By: Gus Caplan <me@gus.host>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Zijian Liu <lxxyxzj@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Co-authored-by: ExE Boss <3889017+ExE-Boss@users.noreply.github.com>
Co-authored-by: Antoine du Hamel <duhamelantoine1995@gmail.com>
PR-URL: https://github.com/nodejs/node/pull/37028
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Darshan Sen <raisinten@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Using new AbortController, add support for promisified
exec to be cancelled.
PR-URL: https://github.com/nodejs/node/pull/34249
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
PR-URL: https://github.com/nodejs/node/pull/37006
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Juan José Arboleda <soyjuanarbol@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Move duplicate abort handler logic into a separate function.
PR-URL: https://github.com/nodejs/node/pull/36644
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Daijiro Wachi <daijiro.wachi@gmail.com>
If an AbortController passed to execfile() is already aborted, use the
same behavior as if the controller was aborted after calling execfile().
This mimics the behavior of fetch in the browser.
PR-URL: https://github.com/nodejs/node/pull/36644
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Daijiro Wachi <daijiro.wachi@gmail.com>
PR-URL: https://github.com/nodejs/node/pull/36603
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
I was working on AbortSignal for spawn and noticed there is a leak in
the current code for AbortSignal support in child_process since it
removes the wrong listener. I used the new signal as argument feature
to make removing the listener easier and added a test.
PR-URL: https://github.com/nodejs/node/pull/36424
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Rich Trott <rtrott@gmail.com>
On Windows environment variables are case-insensitive. When spawning
child process certain apps can get confused if some of the variables are
duplicated.
This adds a step on Windows to normalizeSpawnArguments that removes such
duplicates, keeping only the first (in lexicographic order) entry in the
env key of options. This is partly already done for the PATH entry.
Fixes: https://github.com/nodejs/node/issues/35129
PR-URL: https://github.com/nodejs/node/pull/35210
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Denys Otrishko <shishugi@gmail.com>
Yagiz Nizipli
Aviv Keller
Michaël Zasso
Antoine du Hamel
Liran Tal
atlowChemi
LiviaMedeiros
atlowChemi
Debadree Chatterjee
Darshan Sen
cjihrig
alexcfyung
theanarkh
木杉
Rich Trott
Voltrex
Qingyu Deng
XadillaX
Nitzan Uziely
Nitzan Uziely
ZiJian Liu
Carlos Fuentes
Benjamin Gruenbaum
Bartosz Sosnowski