lib: fix shared buffer growability validation · nodejs/node@0039479 · GitHub
Skip to content

Commit 0039479

Browse files
panvaaduh95
panva
authored and
aduh95
committed
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

‎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

0 commit comments

Comments
 (0)