Skip to content

Commit

Permalink
child_process: improve input validation
Browse files Browse the repository at this point in the history
This commit applies stricter input validation in
normalizeSpawnArguments(), which is run by all of the
child_process methods. Additional checks are added for spawnSync()
specific inputs.

Fixes: nodejs#8096
Fixes: nodejs#8539
Refs: nodejs#9722
PR-URL: nodejs#8312
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
  • Loading branch information
cjihrig committed Dec 25, 2016
1 parent b374ee8 commit fc7b0dd
Show file tree
Hide file tree
Showing 3 changed files with 278 additions and 2 deletions.
66 changes: 65 additions & 1 deletion lib/child_process.js
Original file line number Diff line number Diff line change
Expand Up @@ -315,6 +315,9 @@ function _convertCustomFds(options) {
function normalizeSpawnArguments(file /*, args, options*/) {
var args, options;

if (typeof file !== 'string' || file.length === 0)
throw new TypeError('"file" argument must be a non-empty string');

if (Array.isArray(arguments[1])) {
args = arguments[1].slice(0);
options = arguments[2];
Expand All @@ -331,6 +334,47 @@ function normalizeSpawnArguments(file /*, args, options*/) {
else if (options === null || typeof options !== 'object')
throw new TypeError('"options" argument must be an object');

// Validate the cwd, if present.
if (options.cwd != null &&
typeof options.cwd !== 'string') {
throw new TypeError('"cwd" must be a string');
}

// Validate detached, if present.
if (options.detached != null &&
typeof options.detached !== 'boolean') {
throw new TypeError('"detached" must be a boolean');
}

// Validate the uid, if present.
if (options.uid != null && !Number.isInteger(options.uid)) {
throw new TypeError('"uid" must be an integer');
}

// Validate the gid, if present.
if (options.gid != null && !Number.isInteger(options.gid)) {
throw new TypeError('"gid" must be an integer');
}

// Validate the shell, if present.
if (options.shell != null &&
typeof options.shell !== 'boolean' &&
typeof options.shell !== 'string') {
throw new TypeError('"shell" must be a boolean or string');
}

// Validate argv0, if present.
if (options.argv0 != null &&
typeof options.argv0 !== 'string') {
throw new TypeError('"argv0" must be a string');
}

// Validate windowsVerbatimArguments, if present.
if (options.windowsVerbatimArguments != null &&
typeof options.windowsVerbatimArguments !== 'boolean') {
throw new TypeError('"windowsVerbatimArguments" must be a boolean');
}

// Make a shallow copy so we don't clobber the user's options object.
options = Object.assign({}, options);

Expand Down Expand Up @@ -420,13 +464,33 @@ function spawnSync(/*file, args, options*/) {

debug('spawnSync', opts.args, options);

// Validate the timeout, if present.
if (options.timeout != null &&
!(Number.isInteger(options.timeout) && options.timeout >= 0)) {
throw new TypeError('"timeout" must be an unsigned integer');
}

// Validate maxBuffer, if present.
if (options.maxBuffer != null &&
!(Number.isInteger(options.maxBuffer) && options.maxBuffer >= 0)) {
throw new TypeError('"maxBuffer" must be an unsigned integer');
}

options.file = opts.file;
options.args = opts.args;
options.envPairs = opts.envPairs;

if (options.killSignal)
// Validate the kill signal, if present.
if (typeof options.killSignal === 'string' ||
typeof options.killSignal === 'number') {
options.killSignal = lookupSignal(options.killSignal);

if (options.killSignal === 0)
throw new RangeError('"killSignal" cannot be 0');
} else if (options.killSignal != null) {
throw new TypeError('"killSignal" must be a string or number');
}

options.stdio = _validateStdio(options.stdio || 'pipe', true).stdio;

if (options.input) {
Expand Down
13 changes: 12 additions & 1 deletion test/parallel/test-child-process-spawn-typeerror.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@ const cmd = common.isWindows ? 'rundll32' : 'ls';
const invalidcmd = 'hopefully_you_dont_have_this_on_your_machine';
const invalidArgsMsg = /Incorrect value of args option/;
const invalidOptionsMsg = /"options" argument must be an object/;
const invalidFileMsg =
/^TypeError: "file" argument must be a non-empty string$/;
const empty = common.fixturesDir + '/empty.js';

assert.throws(function() {
Expand Down Expand Up @@ -36,7 +38,16 @@ assert.doesNotThrow(function() {
// verify that invalid argument combinations throw
assert.throws(function() {
spawn();
}, /Bad argument/);
}, invalidFileMsg);

assert.throws(function() {
spawn('');
}, invalidFileMsg);

assert.throws(function() {
const file = { toString() { throw new Error('foo'); } };
spawn(file);
}, invalidFileMsg);

assert.throws(function() {
spawn(cmd, null);
Expand Down
201 changes: 201 additions & 0 deletions test/parallel/test-child-process-spawnsync-validation-errors.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,201 @@
'use strict';
const common = require('../common');
const assert = require('assert');
const spawnSync = require('child_process').spawnSync;
const noop = function() {};

function pass(option, value) {
// Run the command with the specified option. Since it's not a real command,
// spawnSync() should run successfully but return an ENOENT error.
const child = spawnSync('not_a_real_command', { [option]: value });

assert.strictEqual(child.error.code, 'ENOENT');
}

function fail(option, value, message) {
assert.throws(() => {
spawnSync('not_a_real_command', { [option]: value });
}, message);
}

{
// Validate the cwd option
const err = /^TypeError: "cwd" must be a string$/;

pass('cwd', undefined);
pass('cwd', null);
pass('cwd', __dirname);
fail('cwd', 0, err);
fail('cwd', 1, err);
fail('cwd', true, err);
fail('cwd', false, err);
fail('cwd', [], err);
fail('cwd', {}, err);
fail('cwd', noop, err);
}

{
// Validate the detached option
const err = /^TypeError: "detached" must be a boolean$/;

pass('detached', undefined);
pass('detached', null);
pass('detached', true);
pass('detached', false);
fail('detached', 0, err);
fail('detached', 1, err);
fail('detached', __dirname, err);
fail('detached', [], err);
fail('detached', {}, err);
fail('detached', noop, err);
}

if (!common.isWindows) {
{
// Validate the uid option
if (process.getuid() !== 0) {
const err = /^TypeError: "uid" must be an integer$/;

pass('uid', undefined);
pass('uid', null);
pass('uid', process.getuid());
fail('uid', __dirname, err);
fail('uid', true, err);
fail('uid', false, err);
fail('uid', [], err);
fail('uid', {}, err);
fail('uid', noop, err);
fail('uid', NaN, err);
fail('uid', Infinity, err);
fail('uid', 3.1, err);
fail('uid', -3.1, err);
}
}

{
// Validate the gid option
if (process.getgid() !== 0) {
const err = /^TypeError: "gid" must be an integer$/;

pass('gid', undefined);
pass('gid', null);
pass('gid', process.getgid());
fail('gid', __dirname, err);
fail('gid', true, err);
fail('gid', false, err);
fail('gid', [], err);
fail('gid', {}, err);
fail('gid', noop, err);
fail('gid', NaN, err);
fail('gid', Infinity, err);
fail('gid', 3.1, err);
fail('gid', -3.1, err);
}
}
}

{
// Validate the shell option
const err = /^TypeError: "shell" must be a boolean or string$/;

pass('shell', undefined);
pass('shell', null);
pass('shell', false);
fail('shell', 0, err);
fail('shell', 1, err);
fail('shell', [], err);
fail('shell', {}, err);
fail('shell', noop, err);
}

{
// Validate the argv0 option
const err = /^TypeError: "argv0" must be a string$/;

pass('argv0', undefined);
pass('argv0', null);
pass('argv0', 'myArgv0');
fail('argv0', 0, err);
fail('argv0', 1, err);
fail('argv0', true, err);
fail('argv0', false, err);
fail('argv0', [], err);
fail('argv0', {}, err);
fail('argv0', noop, err);
}

{
// Validate the windowsVerbatimArguments option
const err = /^TypeError: "windowsVerbatimArguments" must be a boolean$/;

pass('windowsVerbatimArguments', undefined);
pass('windowsVerbatimArguments', null);
pass('windowsVerbatimArguments', true);
pass('windowsVerbatimArguments', false);
fail('windowsVerbatimArguments', 0, err);
fail('windowsVerbatimArguments', 1, err);
fail('windowsVerbatimArguments', __dirname, err);
fail('windowsVerbatimArguments', [], err);
fail('windowsVerbatimArguments', {}, err);
fail('windowsVerbatimArguments', noop, err);
}

{
// Validate the timeout option
const err = /^TypeError: "timeout" must be an unsigned integer$/;

pass('timeout', undefined);
pass('timeout', null);
pass('timeout', 1);
pass('timeout', 0);
fail('timeout', -1, err);
fail('timeout', true, err);
fail('timeout', false, err);
fail('timeout', __dirname, err);
fail('timeout', [], err);
fail('timeout', {}, err);
fail('timeout', noop, err);
fail('timeout', NaN, err);
fail('timeout', Infinity, err);
fail('timeout', 3.1, err);
fail('timeout', -3.1, err);
}

{
// Validate the maxBuffer option
const err = /^TypeError: "maxBuffer" must be an unsigned integer$/;

pass('maxBuffer', undefined);
pass('maxBuffer', null);
pass('maxBuffer', 1);
pass('maxBuffer', 0);
fail('maxBuffer', 3.14, err);
fail('maxBuffer', -1, err);
fail('maxBuffer', NaN, err);
fail('maxBuffer', Infinity, err);
fail('maxBuffer', true, err);
fail('maxBuffer', false, err);
fail('maxBuffer', __dirname, err);
fail('maxBuffer', [], err);
fail('maxBuffer', {}, err);
fail('maxBuffer', noop, err);
}

{
// Validate the killSignal option
const typeErr = /^TypeError: "killSignal" must be a string or number$/;
const rangeErr = /^RangeError: "killSignal" cannot be 0$/;
const unknownSignalErr = /^Error: Unknown signal:/;

pass('killSignal', undefined);
pass('killSignal', null);
pass('killSignal', 'SIGKILL');
pass('killSignal', 500);
fail('killSignal', 0, rangeErr);
fail('killSignal', 'SIGNOTAVALIDSIGNALNAME', unknownSignalErr);
fail('killSignal', true, typeErr);
fail('killSignal', false, typeErr);
fail('killSignal', [], typeErr);
fail('killSignal', {}, typeErr);
fail('killSignal', noop, typeErr);
}

0 comments on commit fc7b0dd

Please sign in to comment.