Skip to content

reject growable SharedArrayBuffer without hooks#1523

Merged
saghul merged 1 commit into
quickjs-ng:masterfrom
jdcaperon:jack-sab-grow-hooks
Jun 4, 2026
Merged

reject growable SharedArrayBuffer without hooks#1523
saghul merged 1 commit into
quickjs-ng:masterfrom
jdcaperon:jack-sab-grow-hooks

Conversation

@jdcaperon
Copy link
Copy Markdown
Contributor

@jdcaperon jdcaperon commented Jun 1, 2026

This PR is AI assisted and human reviewed to the limits of the human, feel free to close and I can re-raise as an issue.

In quickjs.c, js_array_buffer_constructor3() allowed growable SharedArrayBuffer construction without SAB allocator hooks.

Context

Growable SharedArrayBuffer construction could succeed on runtimes created with JS_NewRuntime() when the embedder had not installed SharedArrayBuffer allocator hooks.

In that configuration, construction allocated only the initial byteLength, while SharedArrayBuffer.prototype.grow() later assumed the backing store covered maxByteLength and only updated the logical byte length.

That mismatch could let typed array and DataView access reach beyond the native allocation after growth.

Intent

Fail closed during JS-created growable SharedArrayBuffer construction when sab_alloc is unavailable.

Fixed-length SharedArrayBuffer construction remains allowed without hooks. Runtimes that install SAB allocator hooks can still construct and grow SABs backed by the upfront maxByteLength allocation.

The C API regression coverage checks the default-runtime rejection path, preserves fixed-length SAB behavior, and verifies hooked-runtime growth exposes zero-filled bytes beyond the initial length.

@jdcaperon jdcaperon changed the title [SAB] Reject growable SharedArrayBuffer without hooks reject growable SharedArrayBuffer without hooks Jun 1, 2026
Comment thread api-test.c Outdated
assert(!JS_IsException(ret));
JS_FreeValue(ctx, ret);

ret = eval(ctx, "new SharedArrayBuffer(16, { maxByteLength: 16 })");
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this succeed, since the current size if the same as the max?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes sense to me

Comment thread api-test.c
"const u8 = new Uint8Array(sab);"
"sab.grow(16384);"
"u8[1024] === 0 && u8.byteLength === 16384");
assert(JS_IsBool(ret));
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe for completeness assert that it's not an exception

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ack

@jdcaperon jdcaperon force-pushed the jack-sab-grow-hooks branch from 3d9ca45 to 520540c Compare June 2, 2026 11:23
@jdcaperon jdcaperon requested a review from saghul June 2, 2026 23:19
@saghul saghul merged commit 0c7e836 into quickjs-ng:master Jun 4, 2026
124 checks passed
@saghul
Copy link
Copy Markdown
Contributor

saghul commented Jun 4, 2026

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants