Skip to content

Commit

Permalink
child_process: validate arguments for null bytes
Browse files Browse the repository at this point in the history
This change adds validation to reject an edge case where the
child_process.spawn() 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: #44768
Signed-off-by: Darshan Sen <raisinten@gmail.com>
  • Loading branch information
RaisinTen committed Sep 25, 2022
1 parent a50e7c5 commit 67903b0
Show file tree
Hide file tree
Showing 2 changed files with 28 additions and 0 deletions.
15 changes: 15 additions & 0 deletions lib/child_process.js
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ const {
ObjectPrototypeHasOwnProperty,
RegExpPrototypeExec,
SafeSet,
StringPrototypeIncludes,
StringPrototypeSlice,
StringPrototypeToUpperCase,
} = primordials;
Expand Down Expand Up @@ -734,6 +735,7 @@ function abortChildProcess(child, killSignal) {
*/
function spawn(file, args, options) {
options = normalizeSpawnArguments(file, args, options);
validateArgumentsNullCheck(options.args);
validateTimeout(options.timeout);
validateAbortSignal(options.signal, 'options.signal');
const killSignal = sanitizeKillSignal(options.killSignal);
Expand Down Expand Up @@ -818,6 +820,10 @@ function spawnSync(file, args, options) {

debug('spawnSync', options);

// Validate the arguments for null bytes. The normalizeSpawnArguments()
// function already makes sure that the array is present.
validateArgumentsNullCheck(options.args);

// Validate the timeout, if present.
validateTimeout(options.timeout);

Expand Down Expand Up @@ -949,6 +955,15 @@ function execSync(command, options) {
}


function validateArgumentsNullCheck(args) {
for (let i = 0; i < args.length; ++i) {
if (StringPrototypeIncludes(args[i], '\u0000')) {
throw new ERR_INVALID_ARG_VALUE('args', args, 'cannot contain null bytes');
}
}
}


function validateTimeout(timeout) {
if (timeout != null && !(NumberIsInteger(timeout) && timeout >= 0)) {
throw new ERR_OUT_OF_RANGE('timeout', 'an unsigned integer', timeout);
Expand Down
13 changes: 13 additions & 0 deletions test/parallel/test-child-process-spawn-reject-null-bytes.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
'use strict';
require('../common');

// Regression test for https://github.com/nodejs/node/issues/44768

const { throws } = require('assert');
const { spawn, spawnSync } = require('child_process');

throws(() => spawn(process.execPath, [__filename, 'AAA', 'BBB\0XXX', 'CCC']),
{ code: 'ERR_INVALID_ARG_VALUE' });

throws(() => spawnSync(process.execPath, [__filename, 'AAA', 'BBB\0XXX', 'CCC']),
{ code: 'ERR_INVALID_ARG_VALUE' });

0 comments on commit 67903b0

Please sign in to comment.