lib: fix shared buffer growability validation by panva · Pull Request #65845 · nodejs/node

panva · 2026-09-06T08:41:03Z

Use the intrinsic growable getter instead of buffer.growable so shadowed properties cannot bypass validation or reject fixed buffers.

cc @nodejs/web-standards

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

Renegade334 · 2026-09-06T09:17:03Z

+  {
+    const Local<Object> prototype =
+        SharedArrayBuffer::New(isolate, 0)->GetPrototypeV2().As<Object>();
+    const Local<Object> descriptor =
+        prototype
+            ->GetOwnPropertyDescriptor(
+                context, FIXED_ONE_BYTE_STRING(isolate, "growable"))
+            .ToLocalChecked()
+            .As<Object>();
+    const Local<Value> getter =
+        descriptor->Get(context, env->get_string()).ToLocalChecked();
+    CHECK(getter->IsFunction());
+    target
+        ->Set(context,
+              FIXED_ONE_BYTE_STRING(isolate, "getSharedArrayBufferGrowable"),
+              getter)
+        .Check();
+  }


Is storing this and invoking with FunctionPrototypeCall definitely quicker than a fast binding that returns sab->GetBackingStore()->IsResizableByUserJavaScript()?

I haven't benchmarked against a fast binding. Copilot tried that predicate first, but it wasn't equivalent. Shared Wasm memory can expose fixed and growable SAB wrappers over the same backing store.

node/deps/v8/src/objects/backing-store.h

Lines 142 to 146 in f16b556

// For shared Wasm memories, this field never changes, but may differ from the

// value of the is_resizable_by_js field of SharedArrayBuffers it backs.

// WebAssembly.Memory can create multiple SharedArrayBuffers backed by the

// same BackingStore, some of which are exposed as growable, and some of which

// as fixed-length.

The getter checks the individual buffer's growability, which is what Web IDL requires.

Nice catch.

The flag check on the SAB itself would be sab.As<ArrayBuffer>()->IsResizableByUserJavaScript() – this checks the flag on the AB handle directly (https://github.com/v8/v8/blob/9f30cc5fc4ad6c3236c522fd23224ddc94591a20/src/api/api.cc#L4374-L4376). SAB handles are AB handles, so providing sab is known to be a SharedArrayBuffer, this should be directly equivalent to sab.growable.

Maybe worth a mini benchmark?

Copilot ran a mini benchmark, about 10-24% faster for this check. But sab.As<ArrayBuffer>() fails with V8_ENABLE_CHECKS because the cast rejects shared buffers.

That's interesting, we already use this relationship elsewhere... although I think technically we cast v8::Value handles that are known to be SABs, rather than casting a v8::SharedArrayBuffer handle directly.

codecov · 2026-09-06T10:36:09Z

Codecov Report

❌ Patch coverage is 94.11765% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 90.18%. Comparing base (6f41e41) to head (4e6aadf).
⚠️ Report is 58 commits behind head on main.

Files with missing lines	Patch %	Lines
src/node_util.cc	92.30%	0 Missing and 1 partial ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main   #65845      +/-   ##
==========================================
+ Coverage   90.17%   90.18%   +0.01%     
==========================================
  Files         770      771       +1     
  Lines      264483   264633     +150     
  Branches    50262    50227      -35     
==========================================
+ Hits       238491   238671     +180     
+ Misses      16981    16958      -23     
+ Partials     9011     9004       -7

Files with missing lines	Coverage Δ
lib/internal/webidl.js	`100.00% <100.00%> (ø)`
src/node_util.cc	`81.79% <92.30%> (+0.39%)`	⬆️

... and 35 files with indirect coverage changes

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

nodejs-github-bot · 2026-09-07T06:30:21Z

CI: https://ci.nodejs.org/job/node-test-pull-request/77121/ 💛

nodejs-github-bot · 2026-09-09T13:57:39Z

Landed in fd6682c

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 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>

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>

Ref: nodejs/node#65845

…e test Chromium's V8 removed --experimental-wasm-rab-integration after shipping it; the new test-internal-webidl-buffer-source.js passes it. Ref: nodejs/node#65845 Ref: https://chromium-review.googlesource.com/c/v8/v8/+/7708297

Ref: nodejs/node#65845

…e test Chromium's V8 removed --experimental-wasm-rab-integration after shipping it; the new test-internal-webidl-buffer-source.js passes it. Ref: nodejs/node#65845 Ref: https://chromium-review.googlesource.com/c/v8/v8/+/7708297

* chore: bump node in DEPS to v24.22.0 * fix(patch): enable the V8 sandbox in shared-cage builds Upstream now enables the V8 sandbox only for shared-cage builds. Keep Electron's behaviour of enabling it whenever pointer compression is on, matching Chromium's V8. The unofficial.gni x86 hunk landed upstream. Ref: nodejs/node#62237 Ref: nodejs/node#65491 * fix(patch): use names for asymmetric key algorithms Upstream dropped the BoringSSL guard in DhKeyGenTraits::Setup(), removed ReadASN1Element() from ncrypto, and no longer includes <openssl/evp.h> from env.h, so those hunks are no longer needed. Ref: nodejs/node#65966 Ref: nodejs/node#66108 Ref: nodejs/node#65484 * fix(patch): skip RSA-PSS keygen deprecation tests with BoringSSL The skip is now upstream. Ref: nodejs/node#65808 * fix(patch): detach cppgc wrappers from their Realm before it is freed Upstream's CppgcWrapperList::Cleanup() now pops nodes itself and the wrapper owns and frees its list node, so the local PopFront/delete hunk is dropped. Ref: nodejs/node#65778 * fix(patch): gate jspi flag on V8_ENABLE_WEBASSEMBLY Ref: nodejs/node#66082 * chore: remove upstreamed patches - nodejs/node#63985 - nodejs/node#65630 - nodejs/node#64453 - nodejs/node#65778 * chore: update patches (trivial only) * chore: update filenames.auto.gni * fix(patch): discover hashes from OpenSSL providers ncrypto's DigestCache::aliases() now has a function-local static, so ncrypto needs the no_exit_time_destructors config too. Ref: nodejs/node#65484 * fix(patch): read the peer certificate chain without consuming it sk_X509_num() returns size_t in BoringSSL. Ref: nodejs/node#65602 * fix(patch): fix shared buffer growability validation Ref: nodejs/node#65845 * chore: update patches (trivial only) * node#65820: src: fix null pointer call when running without a startup snapshot SnapshotData::AsEmbedderWrapper() was removed. Both call sites wrap the embedded snapshot, so use EmbedderSnapshotData::BuiltinSnapshotData(). Ref: nodejs/node#65820 * node#65407: src: apply IsolateSettings when using a snapshot node::CreateEnvironment() no longer installs the per-isolate message listener when it builds the environment from a snapshot, so stop compensating for that and let SetIsolateUpForNode() add it. Ref: nodejs/node#65407 * test: drop removed wasm rab integration flag from webidl buffer source test Chromium's V8 removed --experimental-wasm-rab-integration after shipping it; the new test-internal-webidl-buffer-source.js passes it. Ref: nodejs/node#65845 Ref: https://chromium-review.googlesource.com/c/v8/v8/+/7708297 * chore: remove obsolete TLS external-memory accounting patch Upstream moved the SecureContext external-memory Increase() into Init() and guards the Decrease() on ctx_, so the leak this patch fixed is gone. The patch's removal of that guard now double-decrements when v24.22.0's TLSSocket closes a single-use context before the wrapper is destroyed, tripping V8's ExternalMemoryAccounter DCHECK in every TLS test. Already dropped on main in #53524. * test: skip new rmSync permission error test when the Node suite runs as root test-fs-rmSync-special-char-additional-error.js is new in v24.22.0 and relies on a read-only directory blocking removal, which root bypasses; the Linux Node.js test job runs as root. Ref: nodejs/node#64453 --------- Co-authored-by: electron-roller[bot] <84116207+electron-roller[bot]@users.noreply.github.com> Co-authored-by: Sam Attard <sattard@anthropic.com>

* chore: bump node in DEPS to v24.22.0 * fix(patch): enable the V8 sandbox in shared-cage builds Upstream now enables the V8 sandbox only for shared-cage builds. Keep Electron's behaviour of enabling it whenever pointer compression is on, matching Chromium's V8. The unofficial.gni x86 hunk landed upstream. Ref: nodejs/node#62237 Ref: nodejs/node#65491 * fix(patch): use names for asymmetric key algorithms Upstream dropped the BoringSSL guard in DhKeyGenTraits::Setup(), removed ReadASN1Element() from ncrypto, and no longer includes <openssl/evp.h> from env.h, so those hunks are no longer needed. Ref: nodejs/node#65966 Ref: nodejs/node#66108 Ref: nodejs/node#65484 * fix(patch): skip RSA-PSS keygen deprecation tests with BoringSSL The skip is now upstream. Ref: nodejs/node#65808 * fix(patch): detach cppgc wrappers from their Realm before it is freed Upstream's CppgcWrapperList::Cleanup() now pops nodes itself and the wrapper owns and frees its list node, so the local PopFront/delete hunk is dropped. Ref: nodejs/node#65778 * fix(patch): gate jspi flag on V8_ENABLE_WEBASSEMBLY Ref: nodejs/node#66082 * chore: remove upstreamed patches - nodejs/node#63985 - nodejs/node#65630 - nodejs/node#64453 - nodejs/node#65778 * chore: update patches (trivial only) * chore: update filenames.auto.gni * fix(patch): discover hashes from OpenSSL providers ncrypto's DigestCache::aliases() now has a function-local static, so ncrypto needs the no_exit_time_destructors config too. Ref: nodejs/node#65484 * fix(patch): read the peer certificate chain without consuming it sk_X509_num() returns size_t in BoringSSL. Ref: nodejs/node#65602 * fix(patch): fix shared buffer growability validation Ref: nodejs/node#65845 * chore: update patches (trivial only) * node#65820: src: fix null pointer call when running without a startup snapshot SnapshotData::AsEmbedderWrapper() was removed. Both call sites wrap the embedded snapshot, so use EmbedderSnapshotData::BuiltinSnapshotData(). Ref: nodejs/node#65820 * node#65407: src: apply IsolateSettings when using a snapshot node::CreateEnvironment() no longer installs the per-isolate message listener when it builds the environment from a snapshot, so stop compensating for that and let SetIsolateUpForNode() add it. Ref: nodejs/node#65407 * test: drop removed wasm rab integration flag from webidl buffer source test Chromium's V8 removed --experimental-wasm-rab-integration after shipping it; the new test-internal-webidl-buffer-source.js passes it. Ref: nodejs/node#65845 Ref: https://chromium-review.googlesource.com/c/v8/v8/+/7708297 * test: skip new rmSync permission error test when the Node suite runs as root test-fs-rmSync-special-char-additional-error.js is new in v24.22.0 and relies on a read-only directory blocking removal, which root bypasses; the Linux Node.js test job runs as root. Ref: nodejs/node#64453 --------- Co-authored-by: electron-roller[bot] <84116207+electron-roller[bot]@users.noreply.github.com> Co-authored-by: Sam Attard <sattard@anthropic.com>

* chore: bump node in DEPS to v24.22.0 * fix(patch): enable the V8 sandbox in shared-cage builds Upstream now enables the V8 sandbox only for shared-cage builds. Keep Electron's behaviour of enabling it whenever pointer compression is on, matching Chromium's V8. The unofficial.gni x86 hunk landed upstream. Ref: nodejs/node#62237 Ref: nodejs/node#65491 * fix(patch): use names for asymmetric key algorithms Upstream dropped the BoringSSL guard in DhKeyGenTraits::Setup(), removed ReadASN1Element() from ncrypto, and no longer includes <openssl/evp.h> from env.h, so those hunks are no longer needed. Ref: nodejs/node#65966 Ref: nodejs/node#66108 Ref: nodejs/node#65484 * fix(patch): skip RSA-PSS keygen deprecation tests with BoringSSL The skip is now upstream. Ref: nodejs/node#65808 * fix(patch): detach cppgc wrappers from their Realm before it is freed Upstream's CppgcWrapperList::Cleanup() now pops nodes itself and the wrapper owns and frees its list node, so the local PopFront/delete hunk is dropped. Ref: nodejs/node#65778 * fix(patch): gate jspi flag on V8_ENABLE_WEBASSEMBLY Ref: nodejs/node#66082 * chore: remove upstreamed patches - nodejs/node#63985 - nodejs/node#65630 - nodejs/node#64453 - nodejs/node#65778 * chore: update patches (trivial only) * chore: update filenames.auto.gni * fix(patch): discover hashes from OpenSSL providers ncrypto's DigestCache::aliases() now has a function-local static, so ncrypto needs the no_exit_time_destructors config too. Ref: nodejs/node#65484 * fix(patch): read the peer certificate chain without consuming it sk_X509_num() returns size_t in BoringSSL. Ref: nodejs/node#65602 * chore: update patches (trivial only) * node#65820: src: fix null pointer call when running without a startup snapshot SnapshotData::AsEmbedderWrapper() was removed. Both call sites wrap the embedded snapshot, so use EmbedderSnapshotData::BuiltinSnapshotData(). Ref: nodejs/node#65820 * node#65407: src: apply IsolateSettings when using a snapshot node::CreateEnvironment() no longer installs the per-isolate message listener when it builds the environment from a snapshot, so stop compensating for that and let SetIsolateUpForNode() add it. Ref: nodejs/node#65407 * test: drop removed wasm rab integration flag from webidl buffer source test Chromium's V8 removed --experimental-wasm-rab-integration after shipping it; the new test-internal-webidl-buffer-source.js passes it. Ref: nodejs/node#65845 Ref: https://chromium-review.googlesource.com/c/v8/v8/+/7708297 * chore: remove obsolete TLS external-memory accounting patch Upstream moved the SecureContext external-memory Increase() into Init() and guards the Decrease() on ctx_, so the leak this patch fixed is gone. The patch's removal of that guard now double-decrements when v24.22.0's TLSSocket closes a single-use context before the wrapper is destroyed, tripping V8's ExternalMemoryAccounter DCHECK in every TLS test. Already dropped on main in #53524. * test: skip new rmSync permission error test when the Node suite runs as root test-fs-rmSync-special-char-additional-error.js is new in v24.22.0 and relies on a read-only directory blocking removal, which root bypasses; the Linux Node.js test job runs as root. Ref: nodejs/node#64453 --------- Co-authored-by: electron-roller[bot] <84116207+electron-roller[bot]@users.noreply.github.com> Co-authored-by: Sam Attard <sattard@anthropic.com>

* chore: bump node in DEPS to v24.22.0 * fix(patch): enable the V8 sandbox in shared-cage builds Upstream now enables the V8 sandbox only for shared-cage builds. Keep Electron's behaviour of enabling it whenever pointer compression is on, matching Chromium's V8. The unofficial.gni x86 hunk landed upstream. Ref: nodejs/node#62237 Ref: nodejs/node#65491 * fix(patch): use names for asymmetric key algorithms Upstream dropped the BoringSSL guard in DhKeyGenTraits::Setup(), removed ReadASN1Element() from ncrypto, and no longer includes <openssl/evp.h> from env.h, so those hunks are no longer needed. Ref: nodejs/node#65966 Ref: nodejs/node#66108 Ref: nodejs/node#65484 * fix(patch): skip RSA-PSS keygen deprecation tests with BoringSSL The skip is now upstream. Ref: nodejs/node#65808 * fix(patch): detach cppgc wrappers from their Realm before it is freed Upstream's CppgcWrapperList::Cleanup() now pops nodes itself and the wrapper owns and frees its list node, so the local PopFront/delete hunk is dropped. Ref: nodejs/node#65778 * fix(patch): gate jspi flag on V8_ENABLE_WEBASSEMBLY Ref: nodejs/node#66082 * chore: remove upstreamed patches - nodejs/node#63985 - nodejs/node#65630 - nodejs/node#64453 - nodejs/node#65778 * chore: update patches (trivial only) * chore: update filenames.auto.gni * fix(patch): discover hashes from OpenSSL providers ncrypto's DigestCache::aliases() now has a function-local static, so ncrypto needs the no_exit_time_destructors config too. Ref: nodejs/node#65484 * fix(patch): read the peer certificate chain without consuming it sk_X509_num() returns size_t in BoringSSL. Ref: nodejs/node#65602 * fix(patch): fix shared buffer growability validation Ref: nodejs/node#65845 * chore: update patches (trivial only) * node#65820: src: fix null pointer call when running without a startup snapshot SnapshotData::AsEmbedderWrapper() was removed. Both call sites wrap the embedded snapshot, so use EmbedderSnapshotData::BuiltinSnapshotData(). Ref: nodejs/node#65820 * node#65407: src: apply IsolateSettings when using a snapshot node::CreateEnvironment() no longer installs the per-isolate message listener when it builds the environment from a snapshot, so stop compensating for that and let SetIsolateUpForNode() add it. Ref: nodejs/node#65407 * test: drop removed wasm rab integration flag from webidl buffer source test Chromium's V8 removed --experimental-wasm-rab-integration after shipping it; the new test-internal-webidl-buffer-source.js passes it. Ref: nodejs/node#65845 Ref: https://chromium-review.googlesource.com/c/v8/v8/+/7708297 * test: skip new rmSync permission error test when the Node suite runs as root test-fs-rmSync-special-char-additional-error.js is new in v24.22.0 and relies on a read-only directory blocking removal, which root bypasses; the Linux Node.js test job runs as root. Ref: nodejs/node#64453 --------- Co-authored-by: electron-roller[bot] <84116207+electron-roller[bot]@users.noreply.github.com> Co-authored-by: Sam Attard <sattard@anthropic.com>

lib: fix shared buffer growability validation

4e6aadf

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

panva added the web-standards Issues and PRs related to web-platform APIs and standards compliance. label Sep 6, 2026

nodejs-github-bot added c++ Issues and PRs that require attention from people who are familiar with C++. needs-ci PRs that need a full CI run. typings Issues and PRs related to internal TypeScript declarations. util Issues and PRs related to the built-in util module. labels Sep 6, 2026

Renegade334 reviewed Sep 6, 2026

View reviewed changes

jasnell approved these changes Sep 6, 2026

View reviewed changes

panva added author ready PRs with CI started, the required approvals, and no outstanding review comments. request-ci Add this label to start a Jenkins CI on a PR. Only starts once the PR has an approving review. labels Sep 6, 2026

github-actions Bot removed the request-ci Add this label to start a Jenkins CI on a PR. Only starts once the PR has an approving review. label Sep 6, 2026

This comment has been minimized.

Sign in to view

panva added the commit-queue PRs queued for automated landing through the Commit Queue. label Sep 7, 2026

nodejs-github-bot added the lacks-second-approval Commit Queue PRs awaiting a second collaborator approval or completion of the required wait. label Sep 8, 2026

panva requested review from Renegade334 and avivkeller September 8, 2026 11:36

anonrig approved these changes Sep 9, 2026

View reviewed changes

Comment thread lib/internal/webidl.js

nodejs-github-bot merged commit fd6682c into nodejs:main Sep 9, 2026
113 of 114 checks passed

nodejs-github-bot removed commit-queue PRs queued for automated landing through the Commit Queue. lacks-second-approval Commit Queue PRs awaiting a second collaborator approval or completion of the required wait. labels Sep 9, 2026

panva deleted the webidl-shared-buffer-growability branch September 9, 2026 15:23

github-actions Bot mentioned this pull request Sep 20, 2026

2026-09-22, Version 26.10.0 (Current) #66163

Merged

panva mentioned this pull request Sep 23, 2026

[v24.x backport] crypto and Web IDL fixes and features #66233

Closed

panva added the backport-open-v24.x Indicate that the PR has an open backport label Sep 23, 2026

panva added backported-to-v24.x PRs backported to the v24.x-staging branch. and removed backport-open-v24.x Indicate that the PR has an open backport labels Sep 25, 2026

github-actions Bot mentioned this pull request Oct 11, 2026

2026-10-14, Version 24.22.0 'Krypton' (LTS) #66667

Merged

MarshallOfSound added a commit to electron/electron that referenced this pull request Oct 15, 2026

fix(patch): fix shared buffer growability validation

2f6c8eb

Ref: nodejs/node#65845

MarshallOfSound added a commit to electron/electron that referenced this pull request Oct 15, 2026

fix(patch): fix shared buffer growability validation

e44c85d

Ref: nodejs/node#65845

MarshallOfSound added a commit to electron/electron that referenced this pull request Oct 15, 2026

fix(patch): fix shared buffer growability validation

906b56f

Ref: nodejs/node#65845

MarshallOfSound added a commit to electron/electron that referenced this pull request Oct 15, 2026

fix(patch): fix shared buffer growability validation

fd213d9

Ref: nodejs/node#65845

Navigation Menu

Repository navigation

lib: fix shared buffer growability validation - #65845

lib: fix shared buffer growability validation#65845
nodejs-github-bot merged 1 commit into
nodejs:mainfrom
panva:webidl-shared-buffer-growability

panva commented Sep 6, 2026

Uh oh!

Renegade334 Sep 6, 2026

Uh oh!

panva Sep 6, 2026

Uh oh!

Renegade334 Sep 6, 2026

Uh oh!

panva Sep 6, 2026

Uh oh!

Renegade334 Sep 6, 2026

Uh oh!

codecov Bot commented Sep 6, 2026 •

edited

Loading

Uh oh!

This comment has been minimized.

This comment has been minimized.

nodejs-github-bot commented Sep 7, 2026 •

edited by jasnell

Loading

Uh oh!

Uh oh!

Uh oh!

nodejs-github-bot commented Sep 9, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

	// For shared Wasm memories, this field never changes, but may differ from the
	// value of the is_resizable_by_js field of SharedArrayBuffers it backs.
	// WebAssembly.Memory can create multiple SharedArrayBuffers backed by the
	// same BackingStore, some of which are exposed as growable, and some of which
	// as fixed-length.

Conversation

panva commented Sep 6, 2026

Uh oh!

Renegade334 Sep 6, 2026

Choose a reason for hiding this comment

Uh oh!

panva Sep 6, 2026

Choose a reason for hiding this comment

Uh oh!

Renegade334 Sep 6, 2026

Choose a reason for hiding this comment

Uh oh!

panva Sep 6, 2026

Choose a reason for hiding this comment

Uh oh!

Renegade334 Sep 6, 2026

Choose a reason for hiding this comment

Uh oh!

codecov Bot commented Sep 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

This comment has been minimized.

This comment has been minimized.

nodejs-github-bot commented Sep 7, 2026 • edited by jasnell Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

nodejs-github-bot commented Sep 9, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

codecov Bot commented Sep 6, 2026 •

edited

Loading

nodejs-github-bot commented Sep 7, 2026 •

edited by jasnell

Loading