Uh oh!
There was an error while loading. Please reload this page.
crypto: fix cross-realm ArrayBuffer validation - #57828
Conversation
nodejs-github-bot
commented
Apr 11, 2025
Review requested:
|
Uh oh!
There was an error while loading. Please reload this page.
legendecas
commented
Apr 11, 2025
In general, a patch should land on the main branch first. The same patch can apply to the main branch: node/lib/internal/crypto/webidl.js Lines 196 to 198 in 795dd8e Would you mind re-target the PR to the main branch? Thank you |
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
jasnell
left a comment
There was a problem hiding this comment.
This PR should target the main branch not the nodejs:v22.x branch.
941520a to
4a2a300Compare4a2a300 to
982d482Comparefforbeck
commented
Apr 14, 2025
Thanks for the review @jasnell, @ljharb, and @legendecas. This is ready for another round. |
nodejs-github-bot
commented
Apr 14, 2025
There was a problem hiding this comment.
Alternatively this could be:
constisNonSharedArrayBuffer=isArrayBuffer;Also, should isSharedArrayBuffer(...) also be similarly updated here?
There was a problem hiding this comment.
I would actually just remove the method and use isArrayBuffer() directly instead. It should be clear that it's not shared.
There was a problem hiding this comment.
I'm confused - a SharedArrayBuffer is also an ArrayBuffer, in terms of internal slots - what actual check does isArrayBuffer perform?
There was a problem hiding this comment.
util.types.isArrayBuffer() invokes v8::Value::isArrayBuffer, which returns false if it is a SharedArrayBuffer: https://source.chromium.org/chromium/chromium/src/+/main:v8/src/api/api.cc;l=3534-3538?q=Value::IsArrayBuffer.
ljharb
left a comment
There was a problem hiding this comment.
(with or without "remove isNonSharedArrayBuffer and use isArrayBuffer" directly)
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@## main #57828 +/- ##
==========================================
+ Coverage 90.24% 90.27% +0.03%
==========================================
Files 630 630 Lines 185670 186140 +470 Branches 36405 36481 +76 ==========================================
+ Hits 167555 168045 +490 + Misses 10998 10971 -27 - Partials 7117 7124 +7
🚀 New features to boost your workflow:
|
aduh95
commented
Apr 15, 2025
There's a linter and a test error that would need to be adressed, let us know if you need help. |
fforbeck
commented
Apr 15, 2025
can you point out which test it that? thank you! |
aduh95
commented
Apr 15, 2025
You can find it in the CI output, here it is: |
fforbeck
commented
Apr 17, 2025
@aduh95 - I've made the change to enable the |
nodejs-github-bot
commented
Apr 17, 2025
32bd471 to
3f31959Comparefforbeck
commented
Apr 21, 2025
@aduh95 I've fixed the test and fixed the commit message. Can you approve the workflow execution again? Thank you! |
This patch modifies the isNonSharedArrayBuffer function in the WebIDL implementation for the SubtleCrypto API to properly handle ArrayBuffer instances created in different JavaScript realms. Before this fix, when a TypedArray.buffer from a different realm (e.g., from a VM context or worker thread) was passed to SubtleCrypto.digest(), it would fail with: "TypeError: Failed to execute 'digest' on 'SubtleCrypto': 2nd argument is not instance of ArrayBuffer, Buffer, TypedArray, or DataView." The fix use the isArrayBuffer function from internal/util/types to detect cross-realm ArrayBuffer instances when the prototype chain check fails. This ensures compatibility with TypedArray.buffer across JavaScript realms. See storacha/w3up#1591 for more details.
3f31959 to
607a044Comparenodejs-github-bot
commented
Apr 22, 2025
aduh95
commented
Apr 22, 2025
Landed in 6bf7fd7 |
This patch modifies the `isNonSharedArrayBuffer` function in the WebIDL implementation for the SubtleCrypto API to properly handle `ArrayBuffer` instances created in different JavaScript realms. Before this fix, when a `TypedArray.buffer` from a different realm (e.g., from a VM context or worker thread) was passed to `SubtleCrypto.digest()`, it would fail with: > TypeError: Failed to execute 'digest' on 'SubtleCrypto': 2nd argument > is not instance of ArrayBuffer, Buffer, TypedArray, or DataView." The fix use the `isArrayBuffer` function from `internal/util/types` to detect cross-realm `ArrayBuffer` instances when the prototype chain check fails. PR-URL: #57828 Refs: storacha/w3up#1591 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Jordan Harband <ljharb@gmail.com> Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
This patch modifies the `isNonSharedArrayBuffer` function in the WebIDL implementation for the SubtleCrypto API to properly handle `ArrayBuffer` instances created in different JavaScript realms. Before this fix, when a `TypedArray.buffer` from a different realm (e.g., from a VM context or worker thread) was passed to `SubtleCrypto.digest()`, it would fail with: > TypeError: Failed to execute 'digest' on 'SubtleCrypto': 2nd argument > is not instance of ArrayBuffer, Buffer, TypedArray, or DataView." The fix use the `isArrayBuffer` function from `internal/util/types` to detect cross-realm `ArrayBuffer` instances when the prototype chain check fails. PR-URL: #57828 Refs: storacha/w3up#1591 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Jordan Harband <ljharb@gmail.com> Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
This patch modifies the `isNonSharedArrayBuffer` function in the WebIDL implementation for the SubtleCrypto API to properly handle `ArrayBuffer` instances created in different JavaScript realms. Before this fix, when a `TypedArray.buffer` from a different realm (e.g., from a VM context or worker thread) was passed to `SubtleCrypto.digest()`, it would fail with: > TypeError: Failed to execute 'digest' on 'SubtleCrypto': 2nd argument > is not instance of ArrayBuffer, Buffer, TypedArray, or DataView." The fix use the `isArrayBuffer` function from `internal/util/types` to detect cross-realm `ArrayBuffer` instances when the prototype chain check fails. PR-URL: #57828 Refs: storacha/w3up#1591 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Jordan Harband <ljharb@gmail.com> Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
This patch modifies the `isNonSharedArrayBuffer` function in the WebIDL implementation for the SubtleCrypto API to properly handle `ArrayBuffer` instances created in different JavaScript realms. Before this fix, when a `TypedArray.buffer` from a different realm (e.g., from a VM context or worker thread) was passed to `SubtleCrypto.digest()`, it would fail with: > TypeError: Failed to execute 'digest' on 'SubtleCrypto': 2nd argument > is not instance of ArrayBuffer, Buffer, TypedArray, or DataView." The fix use the `isArrayBuffer` function from `internal/util/types` to detect cross-realm `ArrayBuffer` instances when the prototype chain check fails. PR-URL: #57828 Refs: storacha/w3up#1591 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Jordan Harband <ljharb@gmail.com> Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
This patch modifies the `isNonSharedArrayBuffer` function in the WebIDL implementation for the SubtleCrypto API to properly handle `ArrayBuffer` instances created in different JavaScript realms. Before this fix, when a `TypedArray.buffer` from a different realm (e.g., from a VM context or worker thread) was passed to `SubtleCrypto.digest()`, it would fail with: > TypeError: Failed to execute 'digest' on 'SubtleCrypto': 2nd argument > is not instance of ArrayBuffer, Buffer, TypedArray, or DataView." The fix use the `isArrayBuffer` function from `internal/util/types` to detect cross-realm `ArrayBuffer` instances when the prototype chain check fails. PR-URL: #57828 Refs: storacha/w3up#1591 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Jordan Harband <ljharb@gmail.com> Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
This patch modifies the `isNonSharedArrayBuffer` function in the WebIDL implementation for the SubtleCrypto API to properly handle `ArrayBuffer` instances created in different JavaScript realms. Before this fix, when a `TypedArray.buffer` from a different realm (e.g., from a VM context or worker thread) was passed to `SubtleCrypto.digest()`, it would fail with: > TypeError: Failed to execute 'digest' on 'SubtleCrypto': 2nd argument > is not instance of ArrayBuffer, Buffer, TypedArray, or DataView." The fix use the `isArrayBuffer` function from `internal/util/types` to detect cross-realm `ArrayBuffer` instances when the prototype chain check fails. PR-URL: #57828 Refs: storacha/w3up#1591 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Jordan Harband <ljharb@gmail.com> Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
This patch modifies the `isNonSharedArrayBuffer` function in the WebIDL implementation for the SubtleCrypto API to properly handle `ArrayBuffer` instances created in different JavaScript realms. Before this fix, when a `TypedArray.buffer` from a different realm (e.g., from a VM context or worker thread) was passed to `SubtleCrypto.digest()`, it would fail with: > TypeError: Failed to execute 'digest' on 'SubtleCrypto': 2nd argument > is not instance of ArrayBuffer, Buffer, TypedArray, or DataView." The fix use the `isArrayBuffer` function from `internal/util/types` to detect cross-realm `ArrayBuffer` instances when the prototype chain check fails. PR-URL: #57828 Refs: storacha/w3up#1591 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Jordan Harband <ljharb@gmail.com> Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
This patch modifies the `isNonSharedArrayBuffer` function in the WebIDL implementation for the SubtleCrypto API to properly handle `ArrayBuffer` instances created in different JavaScript realms. Before this fix, when a `TypedArray.buffer` from a different realm (e.g., from a VM context or worker thread) was passed to `SubtleCrypto.digest()`, it would fail with: > TypeError: Failed to execute 'digest' on 'SubtleCrypto': 2nd argument > is not instance of ArrayBuffer, Buffer, TypedArray, or DataView." The fix use the `isArrayBuffer` function from `internal/util/types` to detect cross-realm `ArrayBuffer` instances when the prototype chain check fails. PR-URL: #57828 Refs: storacha/w3up#1591 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Jordan Harband <ljharb@gmail.com> Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
This patch modifies the `isNonSharedArrayBuffer` function in the WebIDL implementation for the SubtleCrypto API to properly handle `ArrayBuffer` instances created in different JavaScript realms. Before this fix, when a `TypedArray.buffer` from a different realm (e.g., from a VM context or worker thread) was passed to `SubtleCrypto.digest()`, it would fail with: > TypeError: Failed to execute 'digest' on 'SubtleCrypto': 2nd argument > is not instance of ArrayBuffer, Buffer, TypedArray, or DataView." The fix use the `isArrayBuffer` function from `internal/util/types` to detect cross-realm `ArrayBuffer` instances when the prototype chain check fails. PR-URL: #57828 Refs: storacha/w3up#1591 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Jordan Harband <ljharb@gmail.com> Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Problem
The Web Crypto API in Node.js v22+ fails to properly validate ArrayBuffer instances
created in different JavaScript realms. When an ArrayBuffer from a different realm
is passed to SubtleCrypto.digest(), it fails with:
This is a common issue when:
The problem is in
isNonSharedArrayBuffer()which checks using prototypeinheritance (
ObjectPrototypeIsPrototypeOf(ArrayBufferPrototype, V)) which failsfor cross-realm objects, even though structurally they are valid ArrayBuffers.
Solution
This PR modifies
isNonSharedArrayBuffer()to useisArrayBufferfrominternal/util/typesto properly check for cross-realm objects.Testing
Added a new test that verifies that ArrayBuffers created in different VM contexts
are correctly recognized as valid inputs to SubtleCrypto.digest().
Workaround
Until this fix is merged, users can work around this issue by wrapping cross-realm ArrayBuffers in a TypedArray view before passing them to WebCrypto functions. Here's a simple example demonstrating the workaround:
Ref: storacha/w3up#1591