net: reject immutable onread buffers

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,

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);
     }
   }
 }

src/node_util.cc

@@ -188,6 +188,14 @@ void ArrayBufferViewHasBuffer(const FunctionCallbackInfo<Value>& args) {
   args.GetReturnValue().Set(args[0].As<ArrayBufferView>()->HasBuffer());
 }
 
+static void IsArrayBufferViewImmutable(
+    const FunctionCallbackInfo<Value>& args) {
+  CHECK(args[0]->IsArrayBufferView());
+  Local<ArrayBufferView> view = args[0].As<ArrayBufferView>();
+  // 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<Object> target,
   SetMethod(context, target, "parseEnv", ParseEnv);
   SetMethod(
       context, target, "arrayBufferViewHasBuffer", ArrayBufferViewHasBuffer);
+  SetMethodNoSideEffect(context,
+                        target,
+                        "isArrayBufferViewImmutable",
+                        IsArrayBufferViewImmutable);
   SetMethod(context,
             target,
             "constructSharedArrayBuffer",

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);
+  });
+}

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]);
+});

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;