From 993fa885ca0fb8d92f38530292bc0d33a92060f8 Mon Sep 17 00:00:00 2001 From: LiviaMedeiros Date: Mon, 10 Jul 2023 22:57:43 +0800 Subject: [PATCH] child_process: harden against prototype pollution --- 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 59c37b97672d39..abaaabcb307d57 100644 --- a/lib/child_process.js +++ b/lib/child_process.js @@ -136,7 +136,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'); @@ -194,7 +194,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 { @@ -327,6 +327,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, @@ -701,6 +702,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, @@ -826,6 +828,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; +}