child_process: improve ChildProcess validation

cjihrig · cjihrig · commit 97a77288cec2 · 2017-04-17T16:20:29.000-04:00
This commit improves input validation for the ChildProcess internals. It became officially supported API a while back, but never had any validation. Refs: #12177 PR-URL: #12348 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Santiago Gimeno <santiago.gimeno@gmail.com>
diff --git a/lib/internal/child_process.js b/lib/internal/child_process.js
@@ -254,6 +254,9 @@ ChildProcess.prototype.spawn = function(options) {
   var ipcFd;
   var i;
 
+  if (options === null || typeof options !== 'object')
+    throw new TypeError('"options" must be an object');
+
   // If no `stdio` option was given - use default
   var stdio = options.stdio || 'pipe';
 
@@ -265,12 +268,25 @@ ChildProcess.prototype.spawn = function(options) {
 
   if (ipc !== undefined) {
     // Let child process know about opened IPC channel
-    options.envPairs = options.envPairs || [];
+    if (options.envPairs === undefined)
+      options.envPairs = [];
+    else if (!Array.isArray(options.envPairs))
+      throw new TypeError('"envPairs" must be an array');
+
     options.envPairs.push('NODE_CHANNEL_FD=' + ipcFd);
   }
 
-  this.spawnfile = options.file;
-  this.spawnargs = options.args;
+  if (typeof options.file === 'string')
+    this.spawnfile = options.file;
+  else
+    throw new TypeError('"file" must be a string');
+
+  if (Array.isArray(options.args))
+    this.spawnargs = options.args;
+  else if (options.args === undefined)
+    this.spawnargs = [];
+  else
+    throw new TypeError('"args" must be an array');
 
   var err = this._handle.spawn(options);
 
diff --git a/test/parallel/test-child-process-constructor.js b/test/parallel/test-child-process-constructor.js
@@ -5,6 +5,50 @@ const assert = require('assert');
 const { ChildProcess } = require('child_process');
 assert.strictEqual(typeof ChildProcess, 'function');
 
+{
+  // Verify that invalid options to spawn() throw.
+  const child = new ChildProcess();
+
+  [undefined, null, 'foo', 0, 1, NaN, true, false].forEach((options) => {
+    assert.throws(() => {
+      child.spawn(options);
+    }, /^TypeError: "options" must be an object$/);
+  });
+}
+
+{
+  // Verify that spawn throws if file is not a string.
+  const child = new ChildProcess();
+
+  [undefined, null, 0, 1, NaN, true, false, {}].forEach((file) => {
+    assert.throws(() => {
+      child.spawn({ file });
+    }, /^TypeError: "file" must be a string$/);
+  });
+}
+
+{
+  // Verify that spawn throws if envPairs is not an array or undefined.
+  const child = new ChildProcess();
+
+  [null, 0, 1, NaN, true, false, {}, 'foo'].forEach((envPairs) => {
+    assert.throws(() => {
+      child.spawn({ envPairs, stdio: ['ignore', 'ignore', 'ignore', 'ipc'] });
+    }, /^TypeError: "envPairs" must be an array$/);
+  });
+}
+
+{
+  // Verify that spawn throws if args is not an array or undefined.
+  const child = new ChildProcess();
+
+  [null, 0, 1, NaN, true, false, {}, 'foo'].forEach((args) => {
+    assert.throws(() => {
+      child.spawn({ file: 'foo', args });
+    }, /^TypeError: "args" must be an array$/);
+  });
+}
+
 // test that we can call spawn
 const child = new ChildProcess();
 child.spawn({
