From 4e6aadfa644e5b8255cb389f94b9e4b893d44708 Mon Sep 17 00:00:00 2001 From: Filip Skokan Date: Sun, 6 Sep 2026 10:16:22 +0200 Subject: [PATCH] lib: fix shared buffer growability validation Use the intrinsic growable getter instead of buffer.growable so shadowed properties cannot bypass validation or reject fixed buffers. Signed-off-by: Filip Skokan Assisted-by: GitHub Copilot --- lib/internal/webidl.js | 10 +-- src/node_util.cc | 19 ++++ .../test-internal-webidl-buffer-source.js | 88 ++++++++++++++++++- typings/internalBinding/util.d.ts | 1 + 4 files changed, 111 insertions(+), 7 deletions(-) diff --git a/lib/internal/webidl.js b/lib/internal/webidl.js index 71513bbe86f4..801435d97e96 100644 --- a/lib/internal/webidl.js +++ b/lib/internal/webidl.js @@ -38,6 +38,8 @@ const { isTypedArray, } = require('internal/util/types'); +const { getSharedArrayBufferGrowable } = internalBinding('util'); + const BIGINT_2_63 = 1n << 63n; const BIGINT_2_64 = 1n << 64n; @@ -945,12 +947,8 @@ function validateBufferSourceBacking(buffer, options) { function validateAllowGrowableSharedArrayBuffer(buffer, options) { // SharedArrayBuffer and ArrayBufferView conversion step 3: // IsFixedLengthArrayBuffer(buffer) must be true without [AllowResizable]. - // Do not use a primordial getter here. When this module is included in the - // startup snapshot, an early-captured SharedArrayBuffer.prototype.growable - // getter does not detect growable buffers created after deserialization. - // Lazily capturing the getter would work, but it would observe the runtime - // prototype at first comparison, so it would not be an actual primordial. - if (!options.allowResizable && buffer.growable) { + if (!options.allowResizable && + FunctionPrototypeCall(getSharedArrayBufferGrowable, buffer)) { throw makeException( 'is backed by a growable SharedArrayBuffer, which is not allowed.', options); diff --git a/src/node_util.cc b/src/node_util.cc index 6d3373caae6c..578c2bd1153a 100644 --- a/src/node_util.cc +++ b/src/node_util.cc @@ -497,6 +497,25 @@ void Initialize(Local target, Environment* env = Environment::GetCurrent(context); Isolate* isolate = env->isolate(); + { + const Local prototype = + SharedArrayBuffer::New(isolate, 0)->GetPrototypeV2().As(); + const Local descriptor = + prototype + ->GetOwnPropertyDescriptor( + context, FIXED_ONE_BYTE_STRING(isolate, "growable")) + .ToLocalChecked() + .As(); + const Local getter = + descriptor->Get(context, env->get_string()).ToLocalChecked(); + CHECK(getter->IsFunction()); + target + ->Set(context, + FIXED_ONE_BYTE_STRING(isolate, "getSharedArrayBufferGrowable"), + getter) + .Check(); + } + { Local tmpl = ObjectTemplate::New(isolate); #define V(PropertyName, _) \ diff --git a/test/parallel/test-internal-webidl-buffer-source.js b/test/parallel/test-internal-webidl-buffer-source.js index 9e522d7d7b8a..8e81f42a9456 100644 --- a/test/parallel/test-internal-webidl-buffer-source.js +++ b/test/parallel/test-internal-webidl-buffer-source.js @@ -1,7 +1,7 @@ // Flags: --expose-internals 'use strict'; -require('../common'); +const common = require('../common'); const assert = require('assert'); const { test } = require('node:test'); const vm = require('vm'); @@ -272,6 +272,92 @@ test('AllowSharedBufferSource handles growable shared buffers with explicit ' + } }); +test('Shared buffer growability checks do not read JavaScript properties', () => { + for (const [buffer, growable] of [ + [new SharedArrayBuffer(8), false], + [new SharedArrayBuffer(8, { maxByteLength: 8 }), true], + [new SharedArrayBuffer(8, { maxByteLength: 16 }), true], + [vm.runInNewContext('new SharedArrayBuffer(8)'), false], + [vm.runInNewContext('new SharedArrayBuffer(8, { maxByteLength: 16 })'), true], + ]) { + const view = new Uint8Array(buffer); + const dataView = new DataView(buffer); + for (const mode of ['shadow', 'getter', 'prototype']) { + if (mode === 'shadow') { + Object.defineProperty(buffer, 'growable', { + value: !growable, + configurable: true, + }); + } else if (mode === 'getter') { + Object.defineProperty(buffer, 'growable', { + get: common.mustNotCall('Unexpected growable getter'), + configurable: true, + }); + } else { + delete buffer.growable; + Object.setPrototypeOf(buffer, null); + } + + for (const value of [buffer, view, dataView]) { + if (growable) { + assert.throws(() => converters.AllowSharedBufferSource(value), { + code: 'ERR_INVALID_ARG_TYPE', + }); + } else { + assert.strictEqual(converters.AllowSharedBufferSource(value), value); + } + assert.strictEqual(converters.AllowSharedBufferSource(value, { + allowResizable: true, + }), value); + } + + if (growable) { + assert.throws(() => converters.Uint8Array(view, { allowShared: true }), { + code: 'ERR_INVALID_ARG_TYPE', + }); + } else { + assert.strictEqual(converters.Uint8Array(view, { allowShared: true }), view); + } + assert.strictEqual(converters.Uint8Array(view, { + allowShared: true, + allowResizable: true, + }), view); + } + } +}); + +test('Shared WebAssembly buffer growability is checked per buffer', { + skip: typeof WebAssembly === 'undefined', +}, () => { + const memory = new WebAssembly.Memory({ initial: 1, maximum: 2, shared: true }); + for (const [buffer, growable] of [ + [memory.buffer, false], + [memory.toResizableBuffer(), true], + [memory.toFixedLengthBuffer(), false], + ]) { + for (const value of [buffer, new Uint8Array(buffer), new DataView(buffer)]) { + if (growable) { + assert.throws(() => converters.AllowSharedBufferSource(value), { + code: 'ERR_INVALID_ARG_TYPE', + }); + } else { + assert.strictEqual(converters.AllowSharedBufferSource(value), value); + } + assert.strictEqual(converters.AllowSharedBufferSource(value, { + allowResizable: true, + }), value); + } + const view = new Uint8Array(buffer); + if (growable) { + assert.throws(() => converters.Uint8Array(view, { allowShared: true }), { + code: 'ERR_INVALID_ARG_TYPE', + }); + } else { + assert.strictEqual(converters.Uint8Array(view, { allowShared: true }), view); + } + } +}); + test('BufferSource rejects objects with a forged @@toStringTag', () => { const fake = { [Symbol.toStringTag]: 'Uint8Array' }; assert.throws( diff --git a/typings/internalBinding/util.d.ts b/typings/internalBinding/util.d.ts index a3026b5a0305..4c3eeca8ed82 100644 --- a/typings/internalBinding/util.d.ts +++ b/typings/internalBinding/util.d.ts @@ -47,6 +47,7 @@ export interface UtilBinding { styleText(format: Array | string, text: string): string; isInsideNodeModules(frameLimit?: number): boolean; constructSharedArrayBuffer(length?: number): SharedArrayBuffer; + getSharedArrayBufferGrowable(this: SharedArrayBuffer): boolean; constants: { kPending: 0;