Skip to content

Commit

Permalink
child_process: fix incomplete prototype pollution hardening
Browse files Browse the repository at this point in the history
Prior pull request (#48726) hardened against prototype pollution
vulnerabilities but effectively missed some use-cases which
opened a window for prototype pollution for some child_process
functions such as spawn(), spawnSync(), and execFileSync().
  • Loading branch information
lirantal committed Jul 9, 2024
1 parent cbd2c38 commit 1317a3e
Showing 1 changed file with 33 additions and 1 deletion.
34 changes: 33 additions & 1 deletion test/parallel/test-child-process-prototype-tampering.mjs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import * as common from '../common/index.mjs';

Check failure on line 1 in test/parallel/test-child-process-prototype-tampering.mjs

View workflow job for this annotation

GitHub Actions / test-asan

--- stderr --- node:internal/modules/run_main:115 triggerUncaughtException( ^ AssertionError [ERR_ASSERTION]: Missing expected exception. at file:///home/runner/work/node/node/test/parallel/test-child-process-prototype-tampering.mjs:80:3 at ModuleJob.run (node:internal/modules/esm/module_job:262:25) at async onImport.tracePromise.__proto__ (node:internal/modules/esm/loader:485:26) at async asyncRunEntryPointWithESMLoader (node:internal/modules/run_main:109:5) { generatedMessage: false, code: 'ERR_ASSERTION', actual: undefined, expected: [Function (anonymous)], operator: 'throws' } Node.js v23.0.0-pre Command: out/Release/node --test-reporter=spec --test-reporter-destination=stdout --test-reporter=./tools/github_reporter/index.js --test-reporter-destination=stdout /home/runner/work/node/node/test/parallel/test-child-process-prototype-tampering.mjs

Check failure on line 1 in test/parallel/test-child-process-prototype-tampering.mjs

View workflow job for this annotation

GitHub Actions / test-linux

--- stderr --- node:internal/modules/run_main:115 triggerUncaughtException( ^ AssertionError [ERR_ASSERTION]: Missing expected exception. at file:///home/runner/work/node/node/test/parallel/test-child-process-prototype-tampering.mjs:80:3 at ModuleJob.run (node:internal/modules/esm/module_job:262:25) at async onImport.tracePromise.__proto__ (node:internal/modules/esm/loader:485:26) at async asyncRunEntryPointWithESMLoader (node:internal/modules/run_main:109:5) { generatedMessage: false, code: 'ERR_ASSERTION', actual: undefined, expected: [Function (anonymous)], operator: 'throws' } Node.js v23.0.0-pre Command: out/Release/node --test-reporter=spec --test-reporter-destination=stdout --test-reporter=./tools/github_reporter/index.js --test-reporter-destination=stdout /home/runner/work/node/node/test/parallel/test-child-process-prototype-tampering.mjs
import * as fixtures from '../common/fixtures.mjs';
import { EOL } from 'node:os';
import { strictEqual } from 'node:assert';
import { strictEqual, notStrictEqual, throws } from 'node:assert';
import cp from 'node:child_process';

// TODO(LiviaMedeiros): test on different platforms
Expand Down Expand Up @@ -57,3 +57,35 @@ for (const tamperedUID of [0, 1, 999, 1000, 0n, 'gwak']) {

delete Object.prototype.execPath;
}

for (const shellCommandArgument of ['-L && echo "tampered"']) {
Object.prototype.shell = true;
const cmd = 'pwd';
let cmdExitCode = '';

const program = cp.spawn(cmd, [shellCommandArgument], { cwd: expectedCWD });
program.stderr.on('data', common.mustCall());
program.stdout.on('data', common.mustNotCall());

program.on('exit', common.mustCall((code) => {
notStrictEqual(code, 0);
}));

cp.execFile(cmd, [shellCommandArgument], { cwd: expectedCWD },
common.mustCall((err) => {
notStrictEqual(err.code, 0);
})
);

throws(() => {
cp.execFileSync(cmd, [shellCommandArgument], { cwd: expectedCWD });
}, (e) => {
notStrictEqual(e.status, 0);
return true;
});

cmdExitCode = cp.spawnSync(cmd, [shellCommandArgument], { cwd: expectedCWD }).status;
notStrictEqual(cmdExitCode, 0);

delete Object.prototype.shell;
}

0 comments on commit 1317a3e

Please sign in to comment.