child_process: harden the API

lib/child_process.js

@@ -26,7 +26,8 @@ const { Object, ObjectPrototype } = primordials;
 const {
   promisify,
   convertToValidSignal,
-  getSystemErrorName
+  getSystemErrorName,
+  shallowCloneWithoutPrototype
 } = require('internal/util');
 const { isArrayBufferView } = require('internal/util/types');
 const debug = require('internal/util/debuglog').debuglog('child_process');
@@ -73,7 +74,7 @@ function fork(modulePath /* , args, options */) {
       throw new ERR_INVALID_ARG_VALUE(`arguments[${pos}]`, arguments[pos]);
     }
 
-    options = { ...arguments[pos++] };
+    options = hardenOptions(arguments[pos++]);
   }
 
   // Prepare arguments for fork:
@@ -128,8 +129,7 @@ function normalizeExecArgs(command, options, callback) {
     options = undefined;
   }
 
-  // Make a shallow copy so we don't clobber the user's options object.
-  options = { ...options };
+  options = hardenOptions(options);
   options.shell = typeof options.shell === 'string' ? options.shell : true;
 
   return {
@@ -202,16 +202,15 @@ function execFile(file /* , args, options, callback */) {
     throw new ERR_INVALID_ARG_VALUE('args', arguments[pos]);
   }
 
-  options = {
+  options = hardenOptions({
     encoding: 'utf8',
     timeout: 0,
     maxBuffer: MAX_BUFFER,
     killSignal: 'SIGTERM',
     cwd: null,
     env: null,
-    shell: false,
-    ...options
-  };
+    shell: false
+  }, options);
 
   // Validate the timeout, if present.
   validateTimeout(options.timeout);
@@ -416,6 +415,8 @@ function normalizeSpawnArguments(file, args, options) {
   else if (options === null || typeof options !== 'object')
     throw new ERR_INVALID_ARG_TYPE('options', 'object', options);
 
+  options = hardenOptions(options);
+
   // Validate the cwd, if present.
   if (options.cwd != null &&
       typeof options.cwd !== 'string') {
@@ -501,13 +502,16 @@ function normalizeSpawnArguments(file, args, options) {
     args.unshift(file);
   }
 
-  const env = options.env || process.env;
+  const env = options.env || shallowCloneWithoutPrototype(process.env);
   const envPairs = [];
 
   // process.env.NODE_V8_COVERAGE always propagates, making it possible to
   // collect coverage for programs that spawn with white-listed environment.
   if (process.env.NODE_V8_COVERAGE &&
-      !ObjectPrototype.hasOwnProperty(options.env || {}, 'NODE_V8_COVERAGE')) {
+      !ObjectPrototype.hasOwnProperty(
+        options.env || Object.create(null),
+        'NODE_V8_COVERAGE'
+      )) {
     env.NODE_V8_COVERAGE = process.env.NODE_V8_COVERAGE;
   }
 
@@ -519,16 +523,15 @@ function normalizeSpawnArguments(file, args, options) {
     }
   }
 
-  return {
-    // Make a shallow copy so we don't clobber the user's options object.
+  return shallowCloneWithoutPrototype({
     ...options,
     args,
     detached: !!options.detached,
     envPairs,
     file,
     windowsHide: !!options.windowsHide,
     windowsVerbatimArguments: !!windowsVerbatimArguments
-  };
+  });
 }
 
 
@@ -671,6 +674,14 @@ function sanitizeKillSignal(killSignal) {
   }
 }
 
+function hardenOptions(...args) {
+  const options = shallowCloneWithoutPrototype(...args);
+  if (typeof options.env === 'object' && options.env !== null) {
+    options.env = shallowCloneWithoutPrototype(options.env);
+  }
+  return options;
+}
+
 module.exports = {
   _forkChild,
   ChildProcess,

lib/internal/util.js

@@ -319,6 +319,10 @@ function join(output, separator) {
   return str;
 }
 
+function shallowCloneWithoutPrototype(...objs) {
+  return Object.assign(Object.create(null), ...objs);
+}
+
 // As of V8 6.6, depending on the size of the array, this is anywhere
 // between 1.5-10x faster than the two-arg version of Array#splice()
 function spliceOne(list, index) {
@@ -391,6 +395,7 @@ module.exports = {
   normalizeEncoding,
   once,
   promisify,
+  shallowCloneWithoutPrototype,
   spliceOne,
   removeColors,
 

test/fixtures/child-process-echo-env.js

@@ -0,0 +1 @@
+console.log(process.env[process.argv[2]]);

test/parallel/test-child-process-exec-prototype-pollution.js

@@ -0,0 +1,19 @@
+'use strict';
+
+const common = require('../common');
+
+const assert = require('assert');
+const { exec } = require('child_process');
+
+const command = common.isWindows ? 'echo $Env:POLLUTED' : 'echo $POLLUTED';
+
+Object.prototype.POLLUTED = 'yes!';
+
+assert.strictEqual(({}).POLLUTED, 'yes!');
+
+exec(command, common.mustCall((err, stdout, stderr) => {
+  assert.ifError(err);
+  assert.strictEqual(stderr, '');
+  assert.strictEqual(stdout.trim(), '');
+  delete Object.prototype.POLLUTED;
+}));

test/parallel/test-child-process-execfile-prototype-pollution.js

@@ -0,0 +1,24 @@
+'use strict';
+
+const common = require('../common');
+const fixtures = require('../common/fixtures');
+
+const assert = require('assert');
+const { execFile } = require('child_process');
+
+const fixture = fixtures.path('child-process-echo-env.js');
+
+Object.prototype.POLLUTED = 'yes!';
+
+assert.deepStrictEqual(({}).POLLUTED, 'yes!');
+
+execFile(
+  process.execPath,
+  [fixture, 'POLLUTED'],
+  common.mustCall((err, stdout, stderr) => {
+    assert.ifError(err);
+    assert.strictEqual(stderr, '');
+    assert.strictEqual(stdout.trim(), 'undefined');
+    delete Object.prototype.POLLUTED;
+  })
+);

test/parallel/test-child-process-execfilesync-prototype-pollution.js

@@ -0,0 +1,20 @@
+'use strict';
+
+require('../common');
+
+const fixtures = require('../common/fixtures');
+
+const assert = require('assert');
+const { execFileSync } = require('child_process');
+
+const fixture = fixtures.path('child-process-echo-env.js');
+
+Object.prototype.POLLUTED = 'yes!';
+
+assert.strictEqual(({}).POLLUTED, 'yes!');
+
+const stdout = execFileSync(process.execPath, [fixture, 'POLLUTED']);
+
+assert.strictEqual(stdout.toString().trim(), 'undefined');
+
+delete Object.prototype.POLLUTED;

test/parallel/test-child-process-execsync-prototype-pollution.js

@@ -0,0 +1,18 @@
+'use strict';
+
+const common = require('../common');
+
+const assert = require('assert');
+const { execSync } = require('child_process');
+
+const command = common.isWindows ? 'echo $Env:POLLUTED' : 'echo $POLLUTED';
+
+Object.prototype.POLLUTED = 'yes!';
+
+assert.deepStrictEqual(({}).POLLUTED, 'yes!');
+
+const stdout = execSync(command, { shell: true });
+
+assert.strictEqual(stdout.toString().trim(), '');
+
+delete Object.prototype.POLLUTED;

test/parallel/test-child-process-fork-prototype-pollution.js

@@ -0,0 +1,25 @@
+'use strict';
+
+const common = require('../common');
+const fixtures = require('../common/fixtures');
+
+const assert = require('assert');
+const { fork } = require('child_process');
+
+const fixture = fixtures.path('child-process-echo-options.js');
+
+Object.prototype.POLLUTED = 'yes!';
+
+assert.deepStrictEqual(({}).POLLUTED, 'yes!');
+
+const cp = fork(fixture);
+
+delete Object.prototype.POLLUTED;
+
+cp.on('message', common.mustCall(({ env }) => {
+  assert.strictEqual(env.POLLUTED, undefined);
+}));
+
+cp.on('exit', common.mustCall((code) => {
+  assert.strictEqual(code, 0);
+}));

test/parallel/test-child-process-spawn-prototype-pollution.js

@@ -0,0 +1,23 @@
+'use strict';
+
+const common = require('../common');
+
+const assert = require('assert');
+const { spawn } = require('child_process');
+
+const command = common.isWindows ? 'echo $Env:POLLUTED' : 'echo $POLLUTED';
+const buffers = [];
+
+Object.prototype.POLLUTED = 'yes!';
+
+assert.deepStrictEqual(({}).POLLUTED, 'yes!');
+
+const cp = spawn(command, { shell: true });
+
+cp.stdout.on('data', common.mustCall(buffers.push.bind(buffers)));
+
+cp.stdout.on('end', () => {
+  const result = Buffer.concat(buffers).toString().trim();
+  assert.strictEqual(result, '');
+  delete Object.prototype.POLLUTED;
+});

test/parallel/test-child-process-spawnsync-prototype-pollution.js

@@ -0,0 +1,20 @@
+'use strict';
+
+const common = require('../common');
+
+const assert = require('assert');
+const { spawnSync } = require('child_process');
+
+const command = common.isWindows ? 'echo $Env:POLLUTED' : 'echo $POLLUTED';
+
+Object.prototype.POLLUTED = 'yes!';
+
+assert.deepStrictEqual(({}).POLLUTED, 'yes!');
+
+const { stdout, stderr, error } = spawnSync(command, { shell: true });
+
+assert.ifError(error);
+assert.deepStrictEqual(stderr, Buffer.alloc(0));
+assert.strictEqual(stdout.toString().trim(), '');
+
+delete Object.prototype.POLLUTED;