Skip to content

Commit

Permalink
child_process: harden against prototype pollution
Browse files Browse the repository at this point in the history
PR-URL: nodejs#48726
Reviewed-By: Matthew Aitken <maitken033380023@gmail.com>
Reviewed-By: Mohammed Keyvanzadeh <mohammadkeyvanzade94@gmail.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
  • Loading branch information
LiviaMedeiros authored and Ceres6 committed Aug 14, 2023
1 parent 97a6bb7 commit e41a9c5
Show file tree
Hide file tree
Showing 2 changed files with 64 additions and 2 deletions.
7 changes: 5 additions & 2 deletions lib/child_process.js
Original file line number Diff line number Diff line change
Expand Up @@ -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');

Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -828,6 +830,7 @@ function spawn(file, args, options) {
*/
function spawnSync(file, args, options) {
options = {
__proto__: null,
maxBuffer: MAX_BUFFER,
...normalizeSpawnArguments(file, args, options),
};
Expand Down
59 changes: 59 additions & 0 deletions test/parallel/test-child-process-prototype-tampering.mjs
Original file line number Diff line number Diff line change
@@ -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;
}

0 comments on commit e41a9c5

Please sign in to comment.