-
Notifications
You must be signed in to change notification settings - Fork 29.8k
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
src: disallow direct .bat and .cmd file spawning
An undocumented feature of the Win32 CreateProcess API allows spawning batch files directly but is potentially insecure because arguments are not escaped (and sometimes cannot be unambiguously escaped), hence why they are refused starting today. PR-URL: nodejs-private/node-private#560 Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: Michael Dawson <[email protected]> Reviewed-By: Rafael Gonzaga <[email protected]> CVE-ID: CVE-2024-27980
- Loading branch information
1 parent
423ad47
commit 64b6777
Showing
7 changed files
with
162 additions
and
8 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
108 changes: 108 additions & 0 deletions
108
test/parallel/test-child-process-spawn-windows-batch-file.js
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,108 @@ | ||
| 'use strict'; | ||
|
|
||
| // Node.js on Windows should not be able to spawn batch files directly, | ||
| // only when the 'shell' option is set. An undocumented feature of the | ||
| // Win32 CreateProcess API allows spawning .bat and .cmd files directly | ||
| // but it does not sanitize arguments. We cannot do that automatically | ||
| // because it's sometimes impossible to escape arguments unambiguously. | ||
| // | ||
| // Expectation: spawn() and spawnSync() raise EINVAL if and only if: | ||
| // | ||
| // 1. 'shell' option is unset | ||
| // 2. Platform is Windows | ||
| // 3. Filename ends in .bat or .cmd, case-insensitive | ||
| // | ||
| // exec() and execSync() are unchanged. | ||
|
|
||
| const common = require('../common'); | ||
| const cp = require('child_process'); | ||
| const assert = require('assert'); | ||
| const { isWindows } = common; | ||
|
|
||
| const arg = '--security-revert=CVE-2024-27980'; | ||
| const isRevert = process.execArgv.includes(arg); | ||
|
|
||
| const expectedCode = isWindows && !isRevert ? 'EINVAL' : 'ENOENT'; | ||
| const expectedStatus = isWindows ? 1 : 127; | ||
|
|
||
| const suffixes = | ||
| 'BAT bAT BaT baT BAt bAt Bat bat CMD cMD CmD cmD CMd cMd Cmd cmd' | ||
| .split(' '); | ||
|
|
||
| if (process.argv[2] === undefined) { | ||
| const a = cp.spawnSync(process.execPath, [__filename, 'child']); | ||
| const b = cp.spawnSync(process.execPath, [arg, __filename, 'child']); | ||
| assert.strictEqual(a.status, 0); | ||
| assert.strictEqual(b.status, 0); | ||
| return; | ||
| } | ||
|
|
||
| function testExec(filename) { | ||
| return new Promise((resolve) => { | ||
| cp.exec(filename).once('exit', common.mustCall(function(status) { | ||
| assert.strictEqual(status, expectedStatus); | ||
| resolve(); | ||
| })); | ||
| }); | ||
| } | ||
|
|
||
| function testExecSync(filename) { | ||
| let e; | ||
| try { | ||
| cp.execSync(filename); | ||
| } catch (_e) { | ||
| e = _e; | ||
| } | ||
| if (!e) throw new Error(`Exception expected for ${filename}`); | ||
| assert.strictEqual(e.status, expectedStatus); | ||
| } | ||
|
|
||
| function testSpawn(filename, code) { | ||
| // Batch file case is a synchronous error, file-not-found is asynchronous. | ||
| if (code === 'EINVAL') { | ||
| let e; | ||
| try { | ||
| cp.spawn(filename); | ||
| } catch (_e) { | ||
| e = _e; | ||
| } | ||
| if (!e) throw new Error(`Exception expected for ${filename}`); | ||
| assert.strictEqual(e.code, code); | ||
| } else { | ||
| return new Promise((resolve) => { | ||
| cp.spawn(filename).once('error', common.mustCall(function(e) { | ||
| assert.strictEqual(e.code, code); | ||
| resolve(); | ||
| })); | ||
| }); | ||
| } | ||
| } | ||
|
|
||
| function testSpawnSync(filename, code) { | ||
| { | ||
| const r = cp.spawnSync(filename); | ||
| assert.strictEqual(r.error.code, code); | ||
| } | ||
| { | ||
| const r = cp.spawnSync(filename, { shell: true }); | ||
| assert.strictEqual(r.status, expectedStatus); | ||
| } | ||
| } | ||
|
|
||
| testExecSync('./nosuchdir/nosuchfile'); | ||
| testSpawnSync('./nosuchdir/nosuchfile', 'ENOENT'); | ||
| for (const suffix of suffixes) { | ||
| testExecSync(`./nosuchdir/nosuchfile.${suffix}`); | ||
| testSpawnSync(`./nosuchdir/nosuchfile.${suffix}`, expectedCode); | ||
| } | ||
|
|
||
| go().catch((ex) => { throw ex; }); | ||
|
|
||
| async function go() { | ||
| await testExec('./nosuchdir/nosuchfile'); | ||
| await testSpawn('./nosuchdir/nosuchfile', 'ENOENT'); | ||
| for (const suffix of suffixes) { | ||
| await testExec(`./nosuchdir/nosuchfile.${suffix}`); | ||
| await testSpawn(`./nosuchdir/nosuchfile.${suffix}`, expectedCode); | ||
| } | ||
| } |