src,loader,permission: throw on InternalWorker use

Previously this PR it was expected that InternalWorker
usage doesn't require the --allow-worker when the permission
model is enabled. This, however, exposes a vulnerability
whenever the instance gets accessed by the user. For example
through diagnostics_channel.subscribe('worker_threads')

PR-URL: https://github.com/nodejs-private/node-private/pull/629
Refs: https://hackerone.com/reports/2575105
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Robert Nagy <ronagy@icloud.com>
CVE-ID: CVE-2025-23083
pull/56451/merge
RafaelGSS 2024-08-27 18:00:12 -03:00
parent 8306457110
commit bf59539b98
5 changed files with 68 additions and 8 deletions

View File

@ -1061,6 +1061,8 @@ added:
Enable module mocking in the test runner.
This feature requires `--allow-worker` if used with the [Permission Model][].
### `--experimental-transform-types`
<!-- YAML

View File

@ -495,11 +495,9 @@ Worker::~Worker() {
void Worker::New(const FunctionCallbackInfo<Value>& args) {
Environment* env = Environment::GetCurrent(args);
THROW_IF_INSUFFICIENT_PERMISSIONS(
env, permission::PermissionScope::kWorkerThreads, "");
bool is_internal = args[5]->IsTrue();
if (!is_internal) {
THROW_IF_INSUFFICIENT_PERMISSIONS(
env, permission::PermissionScope::kWorkerThreads, "");
}
Isolate* isolate = args.GetIsolate();
CHECK(args.IsConstructCall());

View File

@ -179,7 +179,7 @@ describe('Loader hooks', { concurrency: !process.env.TEST_PARALLEL }, () => {
});
});
it('should work without worker permission', async () => {
it('should not work without worker permission', async () => {
const { code, signal, stdout, stderr } = await spawnPromisified(execPath, [
'--no-warnings',
'--permission',
@ -190,9 +190,9 @@ describe('Loader hooks', { concurrency: !process.env.TEST_PARALLEL }, () => {
fixtures.path('es-modules/esm-top-level-await.mjs'),
]);
assert.strictEqual(stderr, '');
assert.match(stdout, /^1\r?\n2\r?\n$/);
assert.strictEqual(code, 0);
assert.match(stderr, /Error: Access to this API has been restricted/);
assert.strictEqual(stdout, '');
assert.strictEqual(code, 1);
assert.strictEqual(signal, null);
});

View File

@ -0,0 +1,19 @@
// Flags: --experimental-permission --allow-fs-read=* --experimental-test-module-mocks
'use strict';
const common = require('../common');
const assert = require('node:assert');
{
const diagnostics_channel = require('node:diagnostics_channel');
diagnostics_channel.subscribe('worker_threads', common.mustNotCall());
const { mock } = require('node:test');
// Module mocking should throw instead of posting to worker_threads dc
assert.throws(() => {
mock.module('node:path');
}, common.expectsError({
code: 'ERR_ACCESS_DENIED',
permission: 'WorkerThreads',
}));
}

View File

@ -650,3 +650,44 @@ test('wrong import syntax should throw error after module mocking', async () =>
assert.match(stderr, /Error \[ERR_MODULE_NOT_FOUND\]: Cannot find module/);
assert.strictEqual(code, 1);
});
test('should throw ERR_ACCESS_DENIED when permission model is enabled', async (t) => {
const cwd = fixtures.path('test-runner');
const fixture = fixtures.path('test-runner', 'mock-nm.js');
const args = [
'--experimental-permission',
'--allow-fs-read=*',
'--experimental-test-module-mocks',
fixture,
];
const {
code,
stdout,
} = await common.spawnPromisified(process.execPath, args, { cwd });
assert.strictEqual(code, 1);
assert.match(stdout, /Error: Access to this API has been restricted/);
assert.match(stdout, /permission: 'WorkerThreads'/);
});
test('should work when --allow-worker is passed and permission model is enabled', async (t) => {
const cwd = fixtures.path('test-runner');
const fixture = fixtures.path('test-runner', 'mock-nm.js');
const args = [
'--experimental-permission',
'--allow-fs-read=*',
'--allow-worker',
'--experimental-test-module-mocks',
fixture,
];
const {
code,
stdout,
stderr,
signal,
} = await common.spawnPromisified(process.execPath, args, { cwd });
assert.strictEqual(code, 0, stderr);
assert.strictEqual(signal, null);
assert.match(stdout, /pass 1/, stderr);
});