Skip to content

Commit 0039479

Browse files
panvaaduh95
authored andcommitted
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 <panva.ip@gmail.com> Assisted-by: GitHub Copilot PR-URL: #65845 Backport-PR-URL: #66233 Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
1 parent e790713 commit 0039479

4 files changed

Lines changed: 112 additions & 8 deletions

File tree

‎lib/internal/webidl.js‎

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,8 @@ const {
3636
isTypedArray,
3737
} = require('internal/util/types');
3838

39+
const { getSharedArrayBufferGrowable } = internalBinding('util');
40+
3941
const BIGINT_2_63 = 1n << 63n;
4042
const BIGINT_2_64 = 1n << 64n;
4143

@@ -934,12 +936,8 @@ function validateBufferSourceBacking(buffer, options) {
934936
function validateAllowGrowableSharedArrayBuffer(buffer, options) {
935937
// SharedArrayBuffer and ArrayBufferView conversion step 3:
936938
// IsFixedLengthArrayBuffer(buffer) must be true without [AllowResizable].
937-
// Do not use a primordial getter here. When this module is included in the
938-
// startup snapshot, an early-captured SharedArrayBuffer.prototype.growable
939-
// getter does not detect growable buffers created after deserialization.
940-
// Lazily capturing the getter would work, but it would observe the runtime
941-
// prototype at first comparison, so it would not be an actual primordial.
942-
if (!options.allowResizable && buffer.growable) {
939+
if (!options.allowResizable &&
940+
FunctionPrototypeCall(getSharedArrayBufferGrowable, buffer)) {
943941
throw makeException(
944942
'is backed by a growable SharedArrayBuffer, which is not allowed.',
945943
options);

‎src/node_util.cc‎

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -520,6 +520,25 @@ void Initialize(Local<Object> target,
520520
Environment* env = Environment::GetCurrent(context);
521521
Isolate* isolate = env->isolate();
522522

523+
{
524+
const Local<Object> prototype =
525+
SharedArrayBuffer::New(isolate, 0)->GetPrototypeV2().As<Object>();
526+
const Local<Object> descriptor =
527+
prototype
528+
->GetOwnPropertyDescriptor(
529+
context, FIXED_ONE_BYTE_STRING(isolate, "growable"))
530+
.ToLocalChecked()
531+
.As<Object>();
532+
const Local<Value> getter =
533+
descriptor->Get(context, env->get_string()).ToLocalChecked();
534+
CHECK(getter->IsFunction());
535+
target
536+
->Set(context,
537+
FIXED_ONE_BYTE_STRING(isolate, "getSharedArrayBufferGrowable"),
538+
getter)
539+
.Check();
540+
}
541+
523542
{
524543
Local<ObjectTemplate> tmpl = ObjectTemplate::New(isolate);
525544
#define V(PropertyName, _) \

‎test/parallel/test-internal-webidl-buffer-source.js‎

Lines changed: 88 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
1-
// Flags: --expose-internals
1+
// Flags: --expose-internals --experimental-wasm-rab-integration
22
'use strict';
33

4-
require('../common');
4+
const common = require('../common');
55
const assert = require('assert');
66
const { test } = require('node:test');
77
const vm = require('vm');
@@ -272,6 +272,92 @@ test('AllowSharedBufferSource handles growable shared buffers with explicit ' +
272272
}
273273
});
274274

275+
test('Shared buffer growability checks do not read JavaScript properties', () => {
276+
for (const [buffer, growable] of [
277+
[new SharedArrayBuffer(8), false],
278+
[new SharedArrayBuffer(8, { maxByteLength: 8 }), true],
279+
[new SharedArrayBuffer(8, { maxByteLength: 16 }), true],
280+
[vm.runInNewContext('new SharedArrayBuffer(8)'), false],
281+
[vm.runInNewContext('new SharedArrayBuffer(8, { maxByteLength: 16 })'), true],
282+
]) {
283+
const view = new Uint8Array(buffer);
284+
const dataView = new DataView(buffer);
285+
for (const mode of ['shadow', 'getter', 'prototype']) {
286+
if (mode === 'shadow') {
287+
Object.defineProperty(buffer, 'growable', {
288+
value: !growable,
289+
configurable: true,
290+
});
291+
} else if (mode === 'getter') {
292+
Object.defineProperty(buffer, 'growable', {
293+
get: common.mustNotCall('Unexpected growable getter'),
294+
configurable: true,
295+
});
296+
} else {
297+
delete buffer.growable;
298+
Object.setPrototypeOf(buffer, null);
299+
}
300+
301+
for (const value of [buffer, view, dataView]) {
302+
if (growable) {
303+
assert.throws(() => converters.AllowSharedBufferSource(value), {
304+
code: 'ERR_INVALID_ARG_TYPE',
305+
});
306+
} else {
307+
assert.strictEqual(converters.AllowSharedBufferSource(value), value);
308+
}
309+
assert.strictEqual(converters.AllowSharedBufferSource(value, {
310+
allowResizable: true,
311+
}), value);
312+
}
313+
314+
if (growable) {
315+
assert.throws(() => converters.Uint8Array(view, { allowShared: true }), {
316+
code: 'ERR_INVALID_ARG_TYPE',
317+
});
318+
} else {
319+
assert.strictEqual(converters.Uint8Array(view, { allowShared: true }), view);
320+
}
321+
assert.strictEqual(converters.Uint8Array(view, {
322+
allowShared: true,
323+
allowResizable: true,
324+
}), view);
325+
}
326+
}
327+
});
328+
329+
test('Shared WebAssembly buffer growability is checked per buffer', {
330+
skip: typeof WebAssembly === 'undefined',
331+
}, () => {
332+
const memory = new WebAssembly.Memory({ initial: 1, maximum: 2, shared: true });
333+
for (const [buffer, growable] of [
334+
[memory.buffer, false],
335+
[memory.toResizableBuffer(), true],
336+
[memory.toFixedLengthBuffer(), false],
337+
]) {
338+
for (const value of [buffer, new Uint8Array(buffer), new DataView(buffer)]) {
339+
if (growable) {
340+
assert.throws(() => converters.AllowSharedBufferSource(value), {
341+
code: 'ERR_INVALID_ARG_TYPE',
342+
});
343+
} else {
344+
assert.strictEqual(converters.AllowSharedBufferSource(value), value);
345+
}
346+
assert.strictEqual(converters.AllowSharedBufferSource(value, {
347+
allowResizable: true,
348+
}), value);
349+
}
350+
const view = new Uint8Array(buffer);
351+
if (growable) {
352+
assert.throws(() => converters.Uint8Array(view, { allowShared: true }), {
353+
code: 'ERR_INVALID_ARG_TYPE',
354+
});
355+
} else {
356+
assert.strictEqual(converters.Uint8Array(view, { allowShared: true }), view);
357+
}
358+
}
359+
});
360+
275361
test('BufferSource rejects objects with a forged @@toStringTag', () => {
276362
const fake = { [Symbol.toStringTag]: 'Uint8Array' };
277363
assert.throws(

‎typings/internalBinding/util.d.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,7 @@ export interface UtilBinding {
4747
styleText(format: Array<string> | string, text: string): string;
4848
isInsideNodeModules(frameLimit?: number): boolean;
4949
constructSharedArrayBuffer(length?: number): SharedArrayBuffer;
50+
getSharedArrayBufferGrowable(this: SharedArrayBuffer): boolean;
5051

5152
constants: {
5253
kPending: 0;

0 commit comments

Comments
 (0)