child_process: improve input validation

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: https://github.com/nodejs/node/issues/8096
Fixes: https://github.com/nodejs/node/issues/8539
Refs: https://github.com/nodejs/node/issues/9722
PR-URL: https://github.com/nodejs/node/pull/8312
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>

1 files changed, 65 insertions, 1 deletions

diff --git a/lib/child_process.js b/lib/child_process.js
index 2f5dda870d..03956a999e 100644
--- a/lib/child_process.js
+++ b/lib/child_process.js
@@ -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];
@@ -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);
 
@@ -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) {