child_process: harden against prototype pollution

PR-URL: nodejs#48726 Reviewed-By: Matthew Aitken <[email protected]> Reviewed-By: Mohammed Keyvanzadeh <[email protected]> Reviewed-By: Antoine du Hamel <[email protected]>
Ceres6 · Aug 14, 2023 · e41a9c5 · e41a9c5
1 parent 97a6bb7
commit e41a9c5
Show file tree

Hide file tree

Showing 2 changed files with 64 additions and 2 deletions.
diff --git a/lib/child_process.js b/lib/child_process.js
@@ -138,7 +138,7 @@ function fork(modulePath, args = [], options) {
   if (options != null) {
     validateObject(options, 'options');
   }
-  options = { ...options, shell: false };
+  options = { __proto__: null, ...options, shell: false };
   options.execPath = options.execPath || process.execPath;
   validateArgumentNullCheck(options.execPath, 'options.execPath');
 
@@ -196,7 +196,7 @@ function normalizeExecArgs(command, options, callback) {
   }
 
   // Make a shallow copy so we don't clobber the user's options object.
-  options = { ...options };
+  options = { __proto__: null, ...options };
   options.shell = typeof options.shell === 'string' ? options.shell : true;
 
   return {
@@ -329,6 +329,7 @@ function execFile(file, args, options, callback) {
   ({ file, args, options, callback } = normalizeExecFileArgs(file, args, options, callback));
 
   options = {
+    __proto__: null,
     encoding: 'utf8',
     timeout: 0,
     maxBuffer: MAX_BUFFER,
@@ -703,6 +704,7 @@ function normalizeSpawnArguments(file, args, options) {
 
   return {
     // Make a shallow copy so we don't clobber the user's options object.
+    __proto__: null,
     ...options,
     args,
     cwd,
@@ -828,6 +830,7 @@ function spawn(file, args, options) {
  */
 function spawnSync(file, args, options) {
   options = {
+    __proto__: null,
     maxBuffer: MAX_BUFFER,
     ...normalizeSpawnArguments(file, args, options),
   };

diff --git a/test/parallel/test-child-process-prototype-tampering.mjs b/test/parallel/test-child-process-prototype-tampering.mjs
@@ -0,0 +1,59 @@
+import * as common from '../common/index.mjs';
+import * as fixtures from '../common/fixtures.mjs';
+import { EOL } from 'node:os';
+import { strictEqual } from 'node:assert';
+import cp from 'node:child_process';
+
+// TODO(LiviaMedeiros): test on different platforms
+if (!common.isLinux)
+  common.skip();
+
+const expectedCWD = process.cwd();
+const expectedUID = process.getuid();
+
+for (const tamperedCwd of ['', '/tmp', '/not/existing/malicious/path', 42n]) {
+  Object.prototype.cwd = tamperedCwd;
+
+  cp.exec('pwd', common.mustSucceed((out) => {
+    strictEqual(`${out}`, `${expectedCWD}${EOL}`);
+  }));
+  strictEqual(`${cp.execSync('pwd')}`, `${expectedCWD}${EOL}`);
+  cp.execFile('pwd', common.mustSucceed((out) => {
+    strictEqual(`${out}`, `${expectedCWD}${EOL}`);
+  }));
+  strictEqual(`${cp.execFileSync('pwd')}`, `${expectedCWD}${EOL}`);
+  cp.spawn('pwd').stdout.on('data', common.mustCall((out) => {
+    strictEqual(`${out}`, `${expectedCWD}${EOL}`);
+  }));
+  strictEqual(`${cp.spawnSync('pwd').stdout}`, `${expectedCWD}${EOL}`);
+
+  delete Object.prototype.cwd;
+}
+
+for (const tamperedUID of [0, 1, 999, 1000, 0n, 'gwak']) {
+  Object.prototype.uid = tamperedUID;
+
+  cp.exec('id -u', common.mustSucceed((out) => {
+    strictEqual(`${out}`, `${expectedUID}${EOL}`);
+  }));
+  strictEqual(`${cp.execSync('id -u')}`, `${expectedUID}${EOL}`);
+  cp.execFile('id', ['-u'], common.mustSucceed((out) => {
+    strictEqual(`${out}`, `${expectedUID}${EOL}`);
+  }));
+  strictEqual(`${cp.execFileSync('id', ['-u'])}`, `${expectedUID}${EOL}`);
+  cp.spawn('id', ['-u']).stdout.on('data', common.mustCall((out) => {
+    strictEqual(`${out}`, `${expectedUID}${EOL}`);
+  }));
+  strictEqual(`${cp.spawnSync('id', ['-u']).stdout}`, `${expectedUID}${EOL}`);
+
+  delete Object.prototype.uid;
+}
+
+{
+  Object.prototype.execPath = '/not/existing/malicious/path';
+
+  // Does not throw ENOENT
+  cp.fork(fixtures.path('empty.js'));
+
+  delete Object.prototype.execPath;
+}