From ffac8fd0c8071c507410d565217d278ae267637e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fatih=20G=C3=BCzel?= Date: Wed, 11 Nov 2026 00:20:52 +0300 Subject: [PATCH] net: reject immutable onread buffers MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit reject immutable buffers supplied directly or returned by an onread buffer factory before using them for socket reads. emit ERR_INVALID_ARG_VALUE without modifying the backing store. Refs: https://github.com/nodejs/node/issues/66346 Signed-off-by: Fatih Güzel --- lib/internal/stream_base_commons.js | 30 +++- lib/net.js | 5 +- src/node_util.cc | 13 ++ .../test-net-onread-immutable-buffer.js | 155 ++++++++++++++++++ .../test-tls-onread-immutable-buffer.js | 83 ++++++++++ typings/internalBinding/util.d.ts | 1 + 6 files changed, 283 insertions(+), 4 deletions(-) create mode 100644 test/parallel/test-net-onread-immutable-buffer.js create mode 100644 test/parallel/test-tls-onread-immutable-buffer.js diff --git a/lib/internal/stream_base_commons.js b/lib/internal/stream_base_commons.js index 7bf3a2bab638..4028b9409663 100644 --- a/lib/internal/stream_base_commons.js +++ b/lib/internal/stream_base_commons.js @@ -15,8 +15,12 @@ const { streamBaseState, } = internalBinding('stream_wrap'); const { UV_EOF } = internalBinding('uv'); +const { isArrayBufferViewImmutable } = internalBinding('util'); const { ErrnoException, + codes: { + ERR_INVALID_ARG_VALUE, + }, } = require('internal/errors'); const { owner_symbol } = require('internal/async_hooks').symbols; const { @@ -168,6 +172,25 @@ function afterWriteDispatched(handle, ret, buffer, cb) { return req; } +function onUserBufferError(stream, buffer) { + stream.destroy(new ERR_INVALID_ARG_VALUE( + 'onread.buffer', buffer, 'must not be backed by an immutable ArrayBuffer')); +} + +function setUserBuffer(stream, buffer, deferError = false) { + if (isArrayBufferViewImmutable(buffer)) { + if (deferError) { + // Error formatting can invoke user code, so initialization must finish. + process.nextTick(onUserBufferError, stream, buffer); + } else { + onUserBufferError(stream, buffer); + } + return false; + } + stream[kBuffer] = buffer; + return true; +} + function onStreamRead(arrayBuffer) { const nread = streamBaseState[kReadBytesOrError]; @@ -185,8 +208,10 @@ function onStreamRead(arrayBuffer) { const bufGen = stream[kBufferGen]; if (bufGen !== null) { const nextBuf = bufGen(); - if (isUint8Array(nextBuf)) - stream[kBuffer] = ret = nextBuf; + if (isUint8Array(nextBuf)) { + if (!setUserBuffer(stream, nextBuf)) return; + ret = nextBuf; + } } } else { const offset = streamBaseState[kArrayBufferOffset]; @@ -280,6 +305,7 @@ module.exports = { writevGeneric, writeGeneric, onStreamRead, + setUserBuffer, kAfterAsyncWrite, kMaybeDestroy, kUpdateTimer, diff --git a/lib/net.js b/lib/net.js index ae7cca8ed6e6..8b5a788ff835 100644 --- a/lib/net.js +++ b/lib/net.js @@ -93,6 +93,7 @@ const { writevGeneric, writeGeneric, onStreamRead, + setUserBuffer, kAfterAsyncWrite, kHandle, kUpdateTimer, @@ -357,9 +358,9 @@ function initSocketHandle(self) { userBuf = bufGen(); if (!isUint8Array(userBuf)) return; - self[kBuffer] = userBuf; } - self._handle.useUserBuffer(userBuf); + if (setUserBuffer(self, userBuf, true)) + self._handle.useUserBuffer(userBuf); } } } diff --git a/src/node_util.cc b/src/node_util.cc index e19ba77e264d..64378e24bbf0 100644 --- a/src/node_util.cc +++ b/src/node_util.cc @@ -188,6 +188,14 @@ void ArrayBufferViewHasBuffer(const FunctionCallbackInfo& args) { args.GetReturnValue().Set(args[0].As()->HasBuffer()); } +static void IsArrayBufferViewImmutable( + const FunctionCallbackInfo& args) { + CHECK(args[0]->IsArrayBufferView()); + Local view = args[0].As(); + // On-heap views cannot have immutable backing storage. + args.GetReturnValue().Set(view->HasBuffer() && view->Buffer()->IsImmutable()); +} + static uint32_t GetUVHandleTypeCode(const uv_handle_type type) { // TODO(anonrig): We can use an enum here and then create the array in the // binding, which will remove the hard-coding in C++ and JS land. @@ -508,6 +516,7 @@ void RegisterExternalReferences(ExternalReferenceRegistry* registry) { registry->Register(GetExternalValue); registry->Register(Sleep); registry->Register(ArrayBufferViewHasBuffer); + registry->Register(IsArrayBufferViewImmutable); registry->Register(GuessHandleType); registry->Register(fast_guess_handle_type_); registry->Register(ParseEnv); @@ -637,6 +646,10 @@ void Initialize(Local target, SetMethod(context, target, "parseEnv", ParseEnv); SetMethod( context, target, "arrayBufferViewHasBuffer", ArrayBufferViewHasBuffer); + SetMethodNoSideEffect(context, + target, + "isArrayBufferViewImmutable", + IsArrayBufferViewImmutable); SetMethod(context, target, "constructSharedArrayBuffer", diff --git a/test/parallel/test-net-onread-immutable-buffer.js b/test/parallel/test-net-onread-immutable-buffer.js new file mode 100644 index 000000000000..a4fac28ec385 --- /dev/null +++ b/test/parallel/test-net-onread-immutable-buffer.js @@ -0,0 +1,155 @@ +// Flags: --js-immutable-arraybuffer +'use strict'; + +const common = require('../common'); +const assert = require('assert'); +const net = require('net'); +const { once } = require('events'); +const { test } = require('node:test'); + +if (typeof ArrayBuffer.prototype.transferToImmutable !== 'function') + common.skip('ArrayBuffer.prototype.transferToImmutable is not available'); + +const message = Buffer.from('test'); + +async function createServer(t) { + const connections = []; + const server = net.createServer((socket) => { + connections.push(socket); + // Rejecting a replacement buffer may close the client with unread data. + socket.on('error', (err) => { + if (err.code !== 'ECONNRESET') throw err; + }); + socket.end(message); + }); + t.after(async () => { + for (const socket of connections) socket.destroy(); + await new Promise((resolve) => server.close(resolve)); + }); + server.listen(0, '127.0.0.1'); + await once(server, 'listening'); + return server.address().port; +} + +for (const View of [Buffer, Uint8Array]) { + for (const offset of [0, 1]) { + for (const mode of ['direct', 'first', 'replacement']) { + test(`${View.name}, offset ${offset}, ${mode} immutable buffer`, { + timeout: common.platformTimeout(5000), + }, async (t) => { + const original = [0xaa, 0xbb, 0xcc, 0xdd, 0xee, 0xff]; + const backing = Uint8Array.from(original).buffer.transferToImmutable(); + const target = View === Buffer ? + Buffer.from(backing, offset, 4) : new Uint8Array(backing, offset, 4); + const mutable = new Uint8Array(1); + let generated = 0; + const factory = () => { + generated++; + return mode === 'replacement' && generated === 1 ? mutable : target; + }; + const reads = []; + const errors = []; + const socket = new net.Socket({ + onread: { + buffer: mode === 'direct' ? target : factory, + callback(nread, buffer) { + reads.push({ buffer, bytes: [...buffer.subarray(0, nread)] }); + }, + }, + }); + t.after(() => socket.destroy()); + socket.on('error', (err) => errors.push(err)); + socket.on('close', common.mustCall()); + socket.on('data', common.mustNotCall()); + const closed = new Promise((resolve) => socket.once('close', resolve)); + const port = await createServer(t); + socket.connect(port, '127.0.0.1'); + await closed; + + assert.strictEqual(errors.length, 1); + assert.strictEqual(errors[0].name, 'TypeError'); + assert.strictEqual(errors[0].code, 'ERR_INVALID_ARG_VALUE'); + assert.match(errors[0].message, /onread\.buffer.*immutable ArrayBuffer/); + assert.strictEqual(socket.connecting, false); + assert.strictEqual(backing.immutable, true); + assert.deepStrictEqual([...new Uint8Array(backing)], original); + if (mode === 'replacement') { + assert.strictEqual(generated, 2); + assert.strictEqual(reads.length, 1); + assert.strictEqual(reads[0].buffer, mutable); + assert.deepStrictEqual(reads[0].bytes, [...message.subarray(0, 1)]); + } else { + assert.strictEqual(generated, mode === 'first' ? 1 : 0); + assert.deepStrictEqual(reads, []); + } + }); + } + } +} + +for (const kind of ['bound', 'retry']) { + test(`reject an immutable buffer during ${kind} initialization`, { + timeout: common.platformTimeout(5000), + }, async (t) => { + const port = await createServer(t); + const backing = new ArrayBuffer(4).transferToImmutable(); + const target = new Uint8Array(backing); + let generated = 0; + const options = { + host: '127.0.0.1', + port, + onread: { + buffer: target, + callback: common.mustNotCall(), + }, + }; + let socket; + if (kind === 'bound') { + options.handle = new net.BoundSocket({ host: '127.0.0.1' }); + socket = net.connect(options); + } else { + options.host = 'localhost'; + options.autoSelectFamily = true; + options.autoSelectFamilyAttemptTimeout = common.platformTimeout(10); + options.lookup = (host, opts, callback) => callback(null, [ + { address: '127.0.0.2', family: 4 }, + { address: '127.0.0.1', family: 4 }, + ]); + options.onread.buffer = () => (++generated === 1 ? new Uint8Array(4) : target); + socket = net.connect(options).unref(); + } + t.after(() => socket.destroy()); + socket.on('close', common.mustCall()); + socket.on('error', common.expectsError({ + name: 'TypeError', + code: 'ERR_INVALID_ARG_VALUE', + })); + await new Promise((resolve) => socket.once('close', resolve)); + assert.deepStrictEqual([...target], [0, 0, 0, 0]); + if (kind === 'retry') assert.strictEqual(generated, 2); + }); +} + +for (const shared of [false, true]) { + test(`mutable ${shared ? 'shared' : 'on-heap'} replacement buffers`, { + timeout: common.platformTimeout(5000), + }, async (t) => { + const chunks = []; + const socket = new net.Socket({ + onread: { + buffer: () => (shared ? + new Uint8Array(new SharedArrayBuffer(1)) : new Uint8Array(1)), + callback(nread, buffer) { + chunks.push(Buffer.from(buffer.subarray(0, nread))); + }, + }, + }); + t.after(() => socket.destroy()); + socket.on('error', common.mustNotCall()); + const closed = new Promise((resolve) => socket.once('close', resolve)); + const port = await createServer(t); + socket.connect(port, '127.0.0.1'); + await closed; + assert.deepStrictEqual(Buffer.concat(chunks), message); + }); +} diff --git a/test/parallel/test-tls-onread-immutable-buffer.js b/test/parallel/test-tls-onread-immutable-buffer.js new file mode 100644 index 000000000000..bf71836608f8 --- /dev/null +++ b/test/parallel/test-tls-onread-immutable-buffer.js @@ -0,0 +1,83 @@ +// Flags: --js-immutable-arraybuffer +'use strict'; + +const common = require('../common'); +if (!common.hasCrypto) + common.skip('missing crypto'); + +const assert = require('assert'); +const tls = require('tls'); +const { once } = require('events'); +const { test } = require('node:test'); +const fixtures = require('../common/fixtures'); + +if (typeof ArrayBuffer.prototype.transferToImmutable !== 'function') + common.skip('ArrayBuffer.prototype.transferToImmutable is not available'); + +for (const factory of [false, true]) { + test(`immutable ${factory ? 'generated' : 'static'} TLS onread buffer`, { + timeout: common.platformTimeout(5000), + }, async (t) => { + const backing = new ArrayBuffer(4).transferToImmutable(); + const target = new Uint8Array(backing); + const socket = new tls.TLSSocket(undefined, { + onread: { + buffer: factory ? () => target : target, + callback: common.mustNotCall(), + }, + }); + t.after(() => socket.destroy()); + socket.on('error', common.expectsError({ + name: 'TypeError', + code: 'ERR_INVALID_ARG_VALUE', + })); + socket.on('close', common.mustCall()); + await new Promise((resolve) => socket.once('close', resolve)); + assert.deepStrictEqual([...target], [0, 0, 0, 0]); + }); +} + +test('reject an immutable TLS replacement buffer', { + timeout: common.platformTimeout(5000), +}, async (t) => { + const backing = new ArrayBuffer(4).transferToImmutable(); + const target = new Uint8Array(backing); + const mutable = new Uint8Array(1); + const connections = []; + let socket; + const server = tls.createServer({ + key: fixtures.readKey('agent2-key.pem'), + cert: fixtures.readKey('agent2-cert.pem'), + }, (socket) => { + connections.push(socket); + socket.on('error', (err) => { + if (err.code !== 'ECONNRESET') throw err; + }); + socket.end('test'); + }); + t.after(async () => { + socket?.destroy(); + for (const socket of connections) socket.destroy(); + await new Promise((resolve) => server.close(resolve)); + }); + server.listen(0, '127.0.0.1'); + await once(server, 'listening'); + let generated = 0; + socket = tls.connect({ + host: '127.0.0.1', + port: server.address().port, + rejectUnauthorized: false, + onread: { + buffer: common.mustCall(() => (++generated === 1 ? mutable : target), 2), + callback: common.mustCall((nread, buffer) => { + assert.strictEqual(buffer, mutable); + assert.strictEqual(nread, 1); + assert.strictEqual(buffer[0], 't'.charCodeAt(0)); + }), + }, + }); + socket.on('error', common.expectsError({ code: 'ERR_INVALID_ARG_VALUE' })); + socket.on('close', common.mustCall()); + await new Promise((resolve) => socket.once('close', resolve)); + assert.deepStrictEqual([...target], [0, 0, 0, 0]); +}); diff --git a/typings/internalBinding/util.d.ts b/typings/internalBinding/util.d.ts index 4c3eeca8ed82..6c2cc56ce231 100644 --- a/typings/internalBinding/util.d.ts +++ b/typings/internalBinding/util.d.ts @@ -32,6 +32,7 @@ export interface UtilBinding { sleep(msec: number): void; isConstructor(fn: Function): boolean; arrayBufferViewHasBuffer(view: ArrayBufferView): boolean; + isArrayBufferViewImmutable(view: ArrayBufferView): boolean; propertyFilter: { ALL_PROPERTIES: 0; ONLY_WRITABLE: 1;