From bd7443ad0a99f786634791d85545a8c5557653c8 Mon Sep 17 00:00:00 2001 From: RafaelGSS Date: Tue, 27 Jun 2023 10:44:56 -0300 Subject: [PATCH] lib,permission: restrict process.binding when pm is enabled PR-URL: https://github.com/nodejs-private/node-private/pull/438 Fixes: https://github.com/nodejs-private/node-private/issues/422 CVE-ID: CVE-2023-32558 --- lib/internal/process/pre_execution.js | 4 +++ test/fixtures/permission/processbinding.js | 28 +++++++++++++++++++ .../test-permission-processbinding.js | 26 +++++++++++++++++ 3 files changed, 58 insertions(+) create mode 100644 test/fixtures/permission/processbinding.js create mode 100644 test/parallel/test-permission-processbinding.js diff --git a/lib/internal/process/pre_execution.js b/lib/internal/process/pre_execution.js index 316dac27d37f7c..d79aa41c53e7b6 100644 --- a/lib/internal/process/pre_execution.js +++ b/lib/internal/process/pre_execution.js @@ -33,6 +33,7 @@ const { ERR_MANIFEST_ASSERT_INTEGRITY, ERR_NO_CRYPTO, ERR_MISSING_OPTION, + ERR_ACCESS_DENIED, } = require('internal/errors').codes; const assert = require('internal/assert'); const { @@ -536,6 +537,9 @@ function initializeClusterIPC() { function initializePermission() { const experimentalPermission = getOptionValue('--experimental-permission'); if (experimentalPermission) { + process.binding = function binding(_module) { + throw new ERR_ACCESS_DENIED('process.binding'); + }; process.emitWarning('Permission is an experimental feature', 'ExperimentalWarning'); const { has, deny } = require('internal/process/permission'); diff --git a/test/fixtures/permission/processbinding.js b/test/fixtures/permission/processbinding.js new file mode 100644 index 00000000000000..bdb958fb01b5ca --- /dev/null +++ b/test/fixtures/permission/processbinding.js @@ -0,0 +1,28 @@ +const common = require('../../common'); +common.skipIfWorker(); + +const assert = require('assert'); + +{ + assert.throws(() => { + process.binding(); + }, common.expectsError({ + code: 'ERR_ACCESS_DENIED', + })); +} + +{ + assert.throws(() => { + process.binding('async_wrap'); + }, common.expectsError({ + code: 'ERR_ACCESS_DENIED', + })); +} + +{ + assert.throws(() => { + process.binding('fs'); + }, common.expectsError({ + code: 'ERR_ACCESS_DENIED', + })); +} diff --git a/test/parallel/test-permission-processbinding.js b/test/parallel/test-permission-processbinding.js new file mode 100644 index 00000000000000..0dd6fd450152cd --- /dev/null +++ b/test/parallel/test-permission-processbinding.js @@ -0,0 +1,26 @@ +'use strict'; + +const common = require('../common'); +common.skipIfWorker(); + +if (!common.hasCrypto) { + common.skip('no crypto'); +} + +const { spawnSync } = require('child_process'); +const assert = require('assert'); +const fixtures = require('../common/fixtures'); +const file = fixtures.path('permission', 'processbinding.js'); + +// Due to linting rules-utils.js:isBinding check, process.binding() should +// not be called when --experimental-permission is enabled. +// Always spawn a child process +{ + const { status, stderr } = spawnSync( + process.execPath, + [ + '--experimental-permission', '--allow-fs-read=*', file, + ], + ); + assert.strictEqual(status, 0, stderr.toString()); +}