From cc33a1864bd4bb0f79414c7cf529a3155b664fa4 Mon Sep 17 00:00:00 2001 From: Livia Medeiros Date: Fri, 14 Jul 2023 18:37:37 +0900 Subject: [PATCH] child_process: harden against prototype pollution PR-URL: https://github.com/nodejs/node/pull/48726 Reviewed-By: Matthew Aitken Reviewed-By: Mohammed Keyvanzadeh Reviewed-By: Antoine du Hamel --- lib/child_process.js | 7 ++- ...test-child-process-prototype-tampering.mjs | 59 +++++++++++++++++++ 2 files changed, 64 insertions(+), 2 deletions(-) create mode 100644 test/parallel/test-child-process-prototype-tampering.mjs diff --git a/lib/child_process.js b/lib/child_process.js index c32756437833b6..5bdc474c80169c 100644 --- 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 new file mode 100644 index 00000000000000..5657458f911521 --- /dev/null +++ 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; +}