From c14d1c2ed548dd9d56c371fe9c803f611d9743b6 Mon Sep 17 00:00:00 2001 From: Darshan Sen Date: Fri, 14 Oct 2022 18:32:57 +0530 Subject: [PATCH] child_process: validate arguments for null bytes 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 PR-URL: https://github.com/nodejs/node/pull/44782 Reviewed-By: James M Snell --- lib/child_process.js | 28 ++ .../test-child-process-reject-null-bytes.js | 294 ++++++++++++++++++ 2 files changed, 322 insertions(+) create mode 100644 test/parallel/test-child-process-reject-null-bytes.js diff --git a/lib/child_process.js b/lib/child_process.js index 335de95ec06a99..55e6b781cd0069 100644 --- a/lib/child_process.js +++ b/lib/child_process.js @@ -39,6 +39,7 @@ const { ObjectPrototypeHasOwnProperty, RegExpPrototypeExec, SafeSet, + StringPrototypeIncludes, StringPrototypeSlice, StringPrototypeToUpperCase, } = primordials; @@ -137,9 +138,11 @@ function fork(modulePath, args = [], options) { } options = { ...options, shell: false }; options.execPath = options.execPath || process.execPath; + validateArgumentNullCheck(options.execPath, 'options.execPath'); // Prepare arguments for fork: execArgv = options.execArgv || process.execArgv; + validateArgumentsNullCheck(execArgv, 'options.execArgv'); if (execArgv === process.execArgv && process._eval != null) { const index = ArrayPrototypeLastIndexOf(execArgv, process._eval); @@ -182,6 +185,9 @@ function _forkChild(fd, serializationMode) { } function normalizeExecArgs(command, options, callback) { + validateString(command, 'command'); + validateArgumentNullCheck(command, 'command'); + if (typeof options === 'function') { callback = options; options = undefined; @@ -286,6 +292,7 @@ function normalizeExecFileArgs(file, args, options, callback) { // Validate argv0, if present. if (options.argv0 != null) { validateString(options.argv0, 'options.argv0'); + validateArgumentNullCheck(options.argv0, 'options.argv0'); } return { file, args, options, callback }; @@ -536,6 +543,7 @@ function copyProcessEnvToEnv(env, name, optionEnv) { function normalizeSpawnArguments(file, args, options) { validateString(file, 'file'); + validateArgumentNullCheck(file, 'file'); if (file.length === 0) throw new ERR_INVALID_ARG_VALUE('file', file, 'cannot be empty'); @@ -551,6 +559,8 @@ function normalizeSpawnArguments(file, args, options) { args = []; } + validateArgumentsNullCheck(args, 'args'); + if (options === undefined) options = kEmptyObject; else @@ -589,6 +599,7 @@ function normalizeSpawnArguments(file, args, options) { // Validate argv0, if present. if (options.argv0 != null) { validateString(options.argv0, 'options.argv0'); + validateArgumentNullCheck(options.argv0, 'options.argv0'); } // Validate windowsHide, if present. @@ -604,6 +615,7 @@ function normalizeSpawnArguments(file, args, options) { } if (options.shell) { + validateArgumentNullCheck(options.shell, 'options.shell'); const command = ArrayPrototypeJoin([file, ...args], ' '); // Set the shell, switches, and commands. if (process.platform === 'win32') { @@ -681,6 +693,8 @@ function normalizeSpawnArguments(file, args, options) { for (const key of envKeys) { const value = env[key]; if (value !== undefined) { + validateArgumentNullCheck(key, `options.env['${key}']`); + validateArgumentNullCheck(value, `options.env['${key}']`); ArrayPrototypePush(envPairs, `${key}=${value}`); } } @@ -949,6 +963,20 @@ function execSync(command, options) { } +function validateArgumentNullCheck(arg, propName) { + if (typeof arg === 'string' && StringPrototypeIncludes(arg, '\u0000')) { + throw new ERR_INVALID_ARG_VALUE(propName, arg, 'must be a string without null bytes'); + } +} + + +function validateArgumentsNullCheck(args, propName) { + for (let i = 0; i < args.length; ++i) { + validateArgumentNullCheck(args[i], `${propName}[${i}]`); + } +} + + function validateTimeout(timeout) { if (timeout != null && !(NumberIsInteger(timeout) && timeout >= 0)) { throw new ERR_OUT_OF_RANGE('timeout', 'an unsigned integer', timeout); diff --git a/test/parallel/test-child-process-reject-null-bytes.js b/test/parallel/test-child-process-reject-null-bytes.js new file mode 100644 index 00000000000000..7d84534f80c75f --- /dev/null +++ b/test/parallel/test-child-process-reject-null-bytes.js @@ -0,0 +1,294 @@ +'use strict'; +const { mustNotCall } = require('../common'); + +// Regression test for https://github.com/nodejs/node/issues/44768 + +const { throws } = require('assert'); +const { + exec, + execFile, + execFileSync, + execSync, + fork, + spawn, + spawnSync, +} = require('child_process'); + +// Tests for the 'command' argument + +throws(() => exec(`${process.execPath} ${__filename} AAA BBB\0XXX CCC`, mustNotCall()), { + code: 'ERR_INVALID_ARG_VALUE', + message: /The argument 'command' must be a string without null bytes/ +}); + +throws(() => exec('BBB\0XXX AAA CCC', mustNotCall()), { + code: 'ERR_INVALID_ARG_VALUE', + message: /The argument 'command' must be a string without null bytes/ +}); + +throws(() => execSync(`${process.execPath} ${__filename} AAA BBB\0XXX CCC`), { + code: 'ERR_INVALID_ARG_VALUE', + message: /The argument 'command' must be a string without null bytes/ +}); + +throws(() => execSync('BBB\0XXX AAA CCC'), { + code: 'ERR_INVALID_ARG_VALUE', + message: /The argument 'command' must be a string without null bytes/ +}); + +// Tests for the 'file' argument + +throws(() => spawn('BBB\0XXX'), { + code: 'ERR_INVALID_ARG_VALUE', + message: /The argument 'file' must be a string without null bytes/ +}); + +throws(() => execFile('BBB\0XXX', mustNotCall()), { + code: 'ERR_INVALID_ARG_VALUE', + message: /The argument 'file' must be a string without null bytes/ +}); + +throws(() => execFileSync('BBB\0XXX'), { + code: 'ERR_INVALID_ARG_VALUE', + message: /The argument 'file' must be a string without null bytes/ +}); + +throws(() => spawn('BBB\0XXX'), { + code: 'ERR_INVALID_ARG_VALUE', + message: /The argument 'file' must be a string without null bytes/ +}); + +throws(() => spawnSync('BBB\0XXX'), { + code: 'ERR_INVALID_ARG_VALUE', + message: /The argument 'file' must be a string without null bytes/ +}); + +// Tests for the 'modulePath' argument + +throws(() => fork('BBB\0XXX'), { + code: 'ERR_INVALID_ARG_VALUE', + message: /The argument 'modulePath' must be a string or Uint8Array without null bytes/ +}); + +// Tests for the 'args' argument + +// Not testing exec() and execSync() because these accept 'args' as a part of +// 'command' as space-separated arguments. + +throws(() => execFile(process.execPath, [__filename, 'AAA', 'BBB\0XXX', 'CCC'], mustNotCall()), { + code: 'ERR_INVALID_ARG_VALUE', + message: /The argument 'args\[2\]' must be a string without null bytes/ +}); + +throws(() => execFileSync(process.execPath, [__filename, 'AAA', 'BBB\0XXX', 'CCC']), { + code: 'ERR_INVALID_ARG_VALUE', + message: /The argument 'args\[2\]' must be a string without null bytes/ +}); + +throws(() => fork(__filename, ['AAA', 'BBB\0XXX', 'CCC']), { + code: 'ERR_INVALID_ARG_VALUE', + message: /The argument 'args\[2\]' must be a string without null bytes/ +}); + +throws(() => spawn(process.execPath, [__filename, 'AAA', 'BBB\0XXX', 'CCC']), { + code: 'ERR_INVALID_ARG_VALUE', + message: /The argument 'args\[2\]' must be a string without null bytes/ +}); + +throws(() => spawnSync(process.execPath, [__filename, 'AAA', 'BBB\0XXX', 'CCC']), { + code: 'ERR_INVALID_ARG_VALUE', + message: /The argument 'args\[2\]' must be a string without null bytes/ +}); + +// Tests for the 'options.cwd' argument + +throws(() => exec(process.execPath, { cwd: 'BBB\0XXX' }, mustNotCall()), { + code: 'ERR_INVALID_ARG_VALUE', + message: /The property 'options\.cwd' must be a string or Uint8Array without null bytes/ +}); + +throws(() => execFile(process.execPath, { cwd: 'BBB\0XXX' }, mustNotCall()), { + code: 'ERR_INVALID_ARG_VALUE', + message: /The property 'options\.cwd' must be a string or Uint8Array without null bytes/ +}); + +throws(() => execFileSync(process.execPath, { cwd: 'BBB\0XXX' }), { + code: 'ERR_INVALID_ARG_VALUE', + message: /The property 'options\.cwd' must be a string or Uint8Array without null bytes/ +}); + +throws(() => execSync(process.execPath, { cwd: 'BBB\0XXX' }), { + code: 'ERR_INVALID_ARG_VALUE', + message: /The property 'options\.cwd' must be a string or Uint8Array without null bytes/ +}); + +throws(() => fork(__filename, { cwd: 'BBB\0XXX' }), { + code: 'ERR_INVALID_ARG_VALUE', + message: /The property 'options\.cwd' must be a string or Uint8Array without null bytes/ +}); + +throws(() => spawn(process.execPath, { cwd: 'BBB\0XXX' }), { + code: 'ERR_INVALID_ARG_VALUE', + message: /The property 'options\.cwd' must be a string or Uint8Array without null bytes/ +}); + +throws(() => spawnSync(process.execPath, { cwd: 'BBB\0XXX' }), { + code: 'ERR_INVALID_ARG_VALUE', + message: /The property 'options\.cwd' must be a string or Uint8Array without null bytes/ +}); + +// Tests for the 'options.argv0' argument + +throws(() => exec(process.execPath, { argv0: 'BBB\0XXX' }, mustNotCall()), { + code: 'ERR_INVALID_ARG_VALUE', + message: /The property 'options\.argv0' must be a string without null bytes/ +}); + +throws(() => execFile(process.execPath, { argv0: 'BBB\0XXX' }, mustNotCall()), { + code: 'ERR_INVALID_ARG_VALUE', + message: /The property 'options\.argv0' must be a string without null bytes/ +}); + +throws(() => execFileSync(process.execPath, { argv0: 'BBB\0XXX' }), { + code: 'ERR_INVALID_ARG_VALUE', + message: /The property 'options\.argv0' must be a string without null bytes/ +}); + +throws(() => execSync(process.execPath, { argv0: 'BBB\0XXX' }), { + code: 'ERR_INVALID_ARG_VALUE', + message: /The property 'options\.argv0' must be a string without null bytes/ +}); + +throws(() => fork(__filename, { argv0: 'BBB\0XXX' }), { + code: 'ERR_INVALID_ARG_VALUE', + message: /The property 'options\.argv0' must be a string without null bytes/ +}); + +throws(() => spawn(process.execPath, { argv0: 'BBB\0XXX' }), { + code: 'ERR_INVALID_ARG_VALUE', + message: /The property 'options\.argv0' must be a string without null bytes/ +}); + +throws(() => spawnSync(process.execPath, { argv0: 'BBB\0XXX' }), { + code: 'ERR_INVALID_ARG_VALUE', + message: /The property 'options\.argv0' must be a string without null bytes/ +}); + +// Tests for the 'options.shell' argument + +throws(() => exec(process.execPath, { shell: 'BBB\0XXX' }, mustNotCall()), { + code: 'ERR_INVALID_ARG_VALUE', + message: /The property 'options\.shell' must be a string without null bytes/ +}); + +throws(() => execFile(process.execPath, { shell: 'BBB\0XXX' }, mustNotCall()), { + code: 'ERR_INVALID_ARG_VALUE', + message: /The property 'options\.shell' must be a string without null bytes/ +}); + +throws(() => execFileSync(process.execPath, { shell: 'BBB\0XXX' }), { + code: 'ERR_INVALID_ARG_VALUE', + message: /The property 'options\.shell' must be a string without null bytes/ +}); + +throws(() => execSync(process.execPath, { shell: 'BBB\0XXX' }), { + code: 'ERR_INVALID_ARG_VALUE', + message: /The property 'options\.shell' must be a string without null bytes/ +}); + +// Not testing fork() because it doesn't accept the shell option (internally it +// explicitly sets shell to false). + +throws(() => spawn(process.execPath, { shell: 'BBB\0XXX' }), { + code: 'ERR_INVALID_ARG_VALUE', + message: /The property 'options\.shell' must be a string without null bytes/ +}); + +throws(() => spawnSync(process.execPath, { shell: 'BBB\0XXX' }), { + code: 'ERR_INVALID_ARG_VALUE', + message: /The property 'options\.shell' must be a string without null bytes/ +}); + +// Tests for the 'options.env' argument + +throws(() => exec(process.execPath, { env: { 'AAA': 'BBB\0XXX' } }, mustNotCall()), { + code: 'ERR_INVALID_ARG_VALUE', + message: /The property 'options\.env\['AAA'\]' must be a string without null bytes/ +}); + +throws(() => exec(process.execPath, { env: { 'BBB\0XXX': 'AAA' } }, mustNotCall()), { + code: 'ERR_INVALID_ARG_VALUE', + message: /The property 'options\.env\['BBB\0XXX'\]' must be a string without null bytes/ +}); + +throws(() => execFile(process.execPath, { env: { 'AAA': 'BBB\0XXX' } }, mustNotCall()), { + code: 'ERR_INVALID_ARG_VALUE', + message: /The property 'options\.env\['AAA'\]' must be a string without null bytes/ +}); + +throws(() => execFile(process.execPath, { env: { 'BBB\0XXX': 'AAA' } }, mustNotCall()), { + code: 'ERR_INVALID_ARG_VALUE', + message: /The property 'options\.env\['BBB\0XXX'\]' must be a string without null bytes/ +}); + +throws(() => execFileSync(process.execPath, { env: { 'AAA': 'BBB\0XXX' } }), { + code: 'ERR_INVALID_ARG_VALUE', + message: /The property 'options\.env\['AAA'\]' must be a string without null bytes/ +}); + +throws(() => execFileSync(process.execPath, { env: { 'BBB\0XXX': 'AAA' } }), { + code: 'ERR_INVALID_ARG_VALUE', + message: /The property 'options\.env\['BBB\0XXX'\]' must be a string without null bytes/ +}); + +throws(() => execSync(process.execPath, { env: { 'AAA': 'BBB\0XXX' } }), { + code: 'ERR_INVALID_ARG_VALUE', + message: /The property 'options\.env\['AAA'\]' must be a string without null bytes/ +}); + +throws(() => execSync(process.execPath, { env: { 'BBB\0XXX': 'AAA' } }), { + code: 'ERR_INVALID_ARG_VALUE', + message: /The property 'options\.env\['BBB\0XXX'\]' must be a string without null bytes/ +}); + +throws(() => fork(__filename, { env: { 'AAA': 'BBB\0XXX' } }), { + code: 'ERR_INVALID_ARG_VALUE', + message: /The property 'options\.env\['AAA'\]' must be a string without null bytes/ +}); + +throws(() => fork(__filename, { env: { 'BBB\0XXX': 'AAA' } }), { + code: 'ERR_INVALID_ARG_VALUE', + message: /The property 'options\.env\['BBB\0XXX'\]' must be a string without null bytes/ +}); + +throws(() => spawn(process.execPath, { env: { 'AAA': 'BBB\0XXX' } }), { + code: 'ERR_INVALID_ARG_VALUE', + message: /The property 'options\.env\['AAA'\]' must be a string without null bytes/ +}); + +throws(() => spawn(process.execPath, { env: { 'BBB\0XXX': 'AAA' } }), { + code: 'ERR_INVALID_ARG_VALUE', + message: /The property 'options\.env\['BBB\0XXX'\]' must be a string without null bytes/ +}); + +throws(() => spawnSync(process.execPath, { env: { 'AAA': 'BBB\0XXX' } }), { + code: 'ERR_INVALID_ARG_VALUE', + message: /The property 'options\.env\['AAA'\]' must be a string without null bytes/ +}); + +throws(() => spawnSync(process.execPath, { env: { 'BBB\0XXX': 'AAA' } }), { + code: 'ERR_INVALID_ARG_VALUE', + message: /The property 'options\.env\['BBB\0XXX'\]' must be a string without null bytes/ +}); + +// Tests for the 'options.execPath' argument +throws(() => fork(__filename, { execPath: 'BBB\0XXX' }), { + code: 'ERR_INVALID_ARG_VALUE', + message: /The property 'options\.execPath' must be a string without null bytes/ +}); + +// Tests for the 'options.execArgv' argument +throws(() => fork(__filename, { execArgv: ['AAA', 'BBB\0XXX', 'CCC'] }), { + code: 'ERR_INVALID_ARG_VALUE', + message: /The property 'options\.execArgv\[1\]' must be a string without null bytes/ +});