From 97a77288cec23beedd666b9454bc336e0b438099 Mon Sep 17 00:00:00 2001
From: cjihrig <cjihrig@gmail.com>
Date: Tue, 11 Apr 2017 16:37:07 -0400
Subject: [PATCH] child_process: improve ChildProcess validation

This commit improves input validation for the ChildProcess
internals. It became officially supported API a while back, but
never had any validation.

Refs: https://github.com/nodejs/node/issues/12177
PR-URL: https://github.com/nodejs/node/pull/12348
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Santiago Gimeno <santiago.gimeno@gmail.com>
---
 lib/internal/child_process.js                 | 22 ++++++++--
 .../test-child-process-constructor.js         | 44 +++++++++++++++++++
 2 files changed, 63 insertions(+), 3 deletions(-)

diff --git a/lib/internal/child_process.js b/lib/internal/child_process.js
index c80b0130bea6fa..e411a4661dc599 100644
--- 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
index e39bdf867d200b..7e86ad198fceeb 100644
--- 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({