From 359a79d2e377d507e6036028ceddc3d24c67f9bc Mon Sep 17 00:00:00 2001 From: Rich Trott Date: Thu, 23 Jun 2016 22:10:49 -0700 Subject: [PATCH 1/3] child_process: validate fork/execFile arguments Fixes: https://github.com/nodejs/node/issues/2681 Refs: https://github.com/nodejs/node/pull/4508 --- lib/child_process.js | 25 +++++++++++++++---------- 1 file changed, 15 insertions(+), 10 deletions(-) diff --git a/lib/child_process.js b/lib/child_process.js index 5eee5228f040d3..d282e1159a841c 100644 --- a/lib/child_process.js +++ b/lib/child_process.js @@ -19,15 +19,20 @@ const ChildProcess = exports.ChildProcess = child_process.ChildProcess; exports.fork = function(modulePath /*, args, options*/) { // Get options and args arguments. - var options, args, execArgv; - if (Array.isArray(arguments[1])) { - args = arguments[1]; - options = util._extend({}, arguments[2]); - } else if (arguments[1] && typeof arguments[1] !== 'object') { - throw new TypeError('Incorrect value of args option'); - } else { - args = []; - options = util._extend({}, arguments[1]); + var execArgv; + var options = {}; + var args = []; + var pos = 1; + if (Array.isArray(arguments[pos])) { + args = arguments[pos++]; + } + + if (arguments[pos] != null) { + if (typeof arguments[pos] !== 'object') { + throw new TypeError('Incorrect value of args option'); + } else { + options = util._extend({}, arguments[pos++]); + } } // Prepare arguments for fork: @@ -132,7 +137,7 @@ exports.execFile = function(file /*, args, options, callback*/) { callback = arguments[pos++]; } - if (pos === 1 && arguments.length > 1) { + if (!callback && arguments[pos] != null) { throw new TypeError('Incorrect value of args option'); } From 7c77fd43e63f7710ac1f7d0c1c822f822b4d0ca0 Mon Sep 17 00:00:00 2001 From: Chuck Langford Date: Wed, 30 Dec 2015 16:27:56 -0500 Subject: [PATCH 2/3] test: validate execFile & fork arguments --- .../test-child-process-spawn-typeerror.js | 35 +++++++++++++++++-- 1 file changed, 32 insertions(+), 3 deletions(-) diff --git a/test/parallel/test-child-process-spawn-typeerror.js b/test/parallel/test-child-process-spawn-typeerror.js index bf59779a75ff5d..20b6e4691055f4 100644 --- a/test/parallel/test-child-process-spawn-typeerror.js +++ b/test/parallel/test-child-process-spawn-typeerror.js @@ -111,13 +111,36 @@ assert.doesNotThrow(function() { execFile(cmd, a, o, u); }); assert.doesNotThrow(function() { execFile(cmd, n, o, c); }); assert.doesNotThrow(function() { execFile(cmd, a, n, c); }); assert.doesNotThrow(function() { execFile(cmd, a, o, n); }); +assert.doesNotThrow(function() { execFile(cmd, u, u, u); }); +assert.doesNotThrow(function() { execFile(cmd, u, u, c); }); +assert.doesNotThrow(function() { execFile(cmd, u, o, u); }); +assert.doesNotThrow(function() { execFile(cmd, a, u, u); }); +assert.doesNotThrow(function() { execFile(cmd, n, n, n); }); +assert.doesNotThrow(function() { execFile(cmd, n, n, c); }); +assert.doesNotThrow(function() { execFile(cmd, n, o, n); }); +assert.doesNotThrow(function() { execFile(cmd, a, n, n); }); +assert.doesNotThrow(function() { execFile(cmd, a, u); }); +assert.doesNotThrow(function() { execFile(cmd, a, n); }); +assert.doesNotThrow(function() { execFile(cmd, o, u); }); +assert.doesNotThrow(function() { execFile(cmd, o, n); }); +assert.doesNotThrow(function() { execFile(cmd, c, u); }); +assert.doesNotThrow(function() { execFile(cmd, c, n); }); // string is invalid in arg position (this may seem strange, but is // consistent across node API, cf. `net.createServer('not options', 'not // callback')` assert.throws(function() { execFile(cmd, s, o, c); }, TypeError); -assert.doesNotThrow(function() { execFile(cmd, a, s, c); }); -assert.doesNotThrow(function() { execFile(cmd, a, o, s); }); +assert.throws(function() { execFile(cmd, a, s, c); }, TypeError); +assert.throws(function() { execFile(cmd, a, o, s); }, TypeError); +assert.throws(function() { execFile(cmd, a, s); }, TypeError); +assert.throws(function() { execFile(cmd, o, s); }, TypeError); +assert.throws(function() { execFile(cmd, u, u, s); }, TypeError); +assert.throws(function() { execFile(cmd, n, n, s); }, TypeError); +assert.throws(function() { execFile(cmd, a, u, s); }, TypeError); +assert.throws(function() { execFile(cmd, a, n, s); }, TypeError); +assert.throws(function() { execFile(cmd, u, o, s); }, TypeError); +assert.throws(function() { execFile(cmd, n, o, s); }, TypeError); +assert.doesNotThrow(function() { execFile(cmd, c, s); }); // verify that fork has same argument parsing behaviour as spawn @@ -131,6 +154,12 @@ assert.doesNotThrow(function() { fork(empty); }); assert.doesNotThrow(function() { fork(empty, a); }); assert.doesNotThrow(function() { fork(empty, a, o); }); assert.doesNotThrow(function() { fork(empty, o); }); +assert.doesNotThrow(function() { fork(empty, u, u); }); +assert.doesNotThrow(function() { fork(empty, u, o); }); +assert.doesNotThrow(function() { fork(empty, a, u); }); +assert.doesNotThrow(function() { fork(empty, n, n); }); +assert.doesNotThrow(function() { fork(empty, n, o); }); +assert.doesNotThrow(function() { fork(empty, a, n); }); assert.throws(function() { fork(empty, s); }, TypeError); -assert.doesNotThrow(function() { fork(empty, a, s); }, TypeError); +assert.throws(function() { fork(empty, a, s); }, TypeError); From d07430aef7a6fd230768bff8adfdd22cafd6220f Mon Sep 17 00:00:00 2001 From: Rich Trott Date: Mon, 27 Jun 2016 16:54:43 -0700 Subject: [PATCH 3/3] squash: optimization nits --- lib/child_process.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/child_process.js b/lib/child_process.js index d282e1159a841c..6fd3af380f824c 100644 --- a/lib/child_process.js +++ b/lib/child_process.js @@ -23,11 +23,11 @@ exports.fork = function(modulePath /*, args, options*/) { var options = {}; var args = []; var pos = 1; - if (Array.isArray(arguments[pos])) { + if (pos < arguments.length && Array.isArray(arguments[pos])) { args = arguments[pos++]; } - if (arguments[pos] != null) { + if (pos < arguments.length && arguments[pos] != null) { if (typeof arguments[pos] !== 'object') { throw new TypeError('Incorrect value of args option'); } else {