webcrypto: reject oversized BufferSource inputs instead of aborting

[Jarred-Sumner]

Every SubtleCrypto entry point copies its caller-supplied BufferSource into a WTF::Vector<uint8_t>, whose capacity is capped below the maximum legal ArrayBuffer size; exceeding the cap aborts the process inside Vector::allocateBuffer. This makes the copy fallible: the length is validated up front and the promise is rejected with an OperationError DOMException when the input is too large, matching the existing toKeyData reject-and-return-nullopt pattern in the same file.

Covers encrypt, decrypt, sign, verify (both arguments), digest, unwrapKey, and importKey/toKeyData for raw/spki/pkcs8 key data.

Adds a subprocess test to test/js/web/crypto/web-crypto.test.ts that exercises each entry point with a 2 GiB buffer and asserts the promise rejects (with a SKIP escape hatch when the allocation itself fails), plus a normal-sized round-trip in the same process to confirm ordinary inputs are unaffected.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 925514d2-4052-48f7-8c3d-177667168e15

📥 Commits

Reviewing files that changed from the base of the PR and between ae286f1 and dae2a7c.

📒 Files selected for processing (1)

• test/js/web/crypto/web-crypto.test.ts

---

Walkthrough

This PR hardens WebCrypto operations against buffer overflow crashes by validating BufferSource input sizes before conversion to WTF::Vector. A validation helper family and promise-aware buffer converter reject oversized inputs with OperationError. All affected crypto operations—encrypt, decrypt, sign, verify, digest, importKey, unwrapKey—now validate and use dereferenced optional results, and algorithm parameter normalization validates additional buffer fields like RSA public exponents, HKDF salts, and AES initialization vectors.

Changes

WebCrypto Buffer Overflow Prevention

Layer / File(s)	Summary
Buffer Size Validation Helpers src/jsc/bindings/webcrypto/SubtleCrypto.cpp	isAcceptableVectorSource overload family validates BufferSource, optional BufferSource, and Uint8Array RSA public exponent inputs fit within WTF::Vector capacity.
Algorithm Parameter Validation src/jsc/bindings/webcrypto/SubtleCrypto.cpp	normalizeCryptoAlgorithmParameters validates RSA-OAEP labels, AES IV/counter/additionalData, HKDF salt/info, PBKDF2 salt, and RSA key generation publicExponent; all oversized fields are rejected with OperationError before parameter construction.
Promise-Aware Buffer Conversion src/jsc/bindings/webcrypto/SubtleCrypto.cpp	New promise-aware copyToVector helper returns std::optional<Vector<uint8_t>> and rejects promise on oversized input; importKey toKeyData path validates buffer before conversion.
Encryption and Decryption Operations src/jsc/bindings/webcrypto/SubtleCrypto.cpp	encrypt and decrypt now validate buffers via promise-aware copyToVector, early-return on failure, and dereference validated optionals when invoking algorithm methods.
Signing and Verification Operations src/jsc/bindings/webcrypto/SubtleCrypto.cpp	sign and verify validate buffers via promise-aware copyToVector; verify validates both signature and data independently, both early-return on failure and dereference validated optionals in algorithm calls.
Digest Hash Operation src/jsc/bindings/webcrypto/SubtleCrypto.cpp	digest validates input buffer via promise-aware copyToVector, early-returns on failure, and dereferences validated optional in algorithm invocation.
Key Unwrapping Operation src/jsc/bindings/webcrypto/SubtleCrypto.cpp	unwrapKey validates wrapped key buffer via promise-aware copyToVector, early-returns on failure, and dereferences validated wrapped key in both unwrap and decrypt paths.
Oversized Input Test Suite test/js/web/crypto/web-crypto.test.ts	Subprocess-based test suite allocates ~2 GiB Uint8Array and verifies all crypto.subtle operations reject with OperationError without crashing; small-input operations continue to succeed in same subprocess.

🚥 Pre-merge checks | ✅ 4

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title accurately and concisely summarizes the main change: rejecting oversized BufferSource inputs instead of aborting, which directly matches the core objective of the PR.
Description check	✅ Passed	The PR description addresses both required template sections: it clearly explains what the PR does (the problem and solution) and how the code was verified (the subprocess test with allocation escape hatch and normal-sized round-trip validation).
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

---

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[robobun]

^{Updated 4:07 PM PT - May 24th, 2026}

❌ @Jarred-Sumner, your commit dae2a7c has 1 failures in Build #57656 (All Failures):

• test/js/web/streams/streams-leak.test.ts - code 1 on 🐧 13 x64

---

🧪   To try this PR locally:

bunx bun-pr 31356

That installs a local version of the PR into your bun-31356 executable, so you can run:

bun-31356 --bun

[claude]

LGTM — mechanical conversion of a process-abort into a promise rejection, following the existing toKeyData nullopt contract; the inline note about algorithm-param BufferSources is pre-existing and non-blocking.

Extended reasoning...

Overview

This PR changes copyToVector in src/jsc/bindings/webcrypto/SubtleCrypto.cpp to return std::optional<Vector<uint8_t>> and reject the promise with OperationError when the input BufferSource exceeds WTF::isValidCapacityForVector<uint8_t>, instead of letting Vector::allocateBuffer hit CRASH(). All seven call sites (encrypt, decrypt, sign, verify ×2, digest, unwrapKey) are updated identically: pass promise, early-return on nullopt, dereference at the move site. The same guard is added to the buffer-source arm of toKeyData for importKey. A subprocess test in web-crypto.test.ts exercises every entry point with a 2 GiB buffer and asserts OperationError, with a SKIP fallback if the allocation itself fails and a small-input round-trip to confirm the happy path is unaffected.

Security risks

None introduced. The change is purely defensive input-size validation; no key material handling, algorithm selection, or cryptographic primitive is touched. It strictly hardens the API by turning a user-triggerable process abort (DoS) into a spec-style promise rejection. The one flagged issue (algorithm-dictionary BufferSources like AES-GCM additionalData still unguarded) is pre-existing and orthogonal — this PR does not regress it.

Level of scrutiny

Low-to-moderate. Although the file is WebKit-derived WebCrypto bindings, the diff is a mechanical refactor that follows the exact reject-and-return-nullopt contract already established by toKeyData a few lines above. Each call-site edit is the same three-line pattern, and isValidCapacityForVector is the canonical WTF predicate for this check. There is no change to control flow ordering relative to the original (e.g. unwrapKey already copied the buffer before normalization).

Other factors

No CODEOWNERS cover this path. No prior human review comments to address. The test is thorough (covers both verify arguments separately, includes a positive round-trip, isolates the 2 GiB allocation in a subprocess). The bug-hunter inline comment is explicitly tagged [Pre-existing] and recommends a follow-up rather than a change to this PR.

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@test/js/web/crypto/web-crypto.test.ts`:
- Around line 292-297: The test currently spawns a subprocess with Bun.spawn
using stderr: "inherit"; change it to pipe stderr so the test can assert it's
empty (follow the JWK leak test pattern). Specifically, update the Bun.spawn
call (the proc variable creation) to use stderr: "pipe", then after awaiting
proc.exited gather stdout and stderr from proc.stdout/pipe and assert
expect(stderr).toBe("") before asserting the exitCode to ensure the
oversized-input rejection doesn’t produce stderr output or abort the process.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 1d511260-1957-43fc-8814-de46d67f85da

📥 Commits

Reviewing files that changed from the base of the PR and between 49c97de and ae286f1.

📒 Files selected for processing (2)

• src/jsc/bindings/webcrypto/SubtleCrypto.cpp
• test/js/web/crypto/web-crypto.test.ts

[claude]

The publicExponent overload in ae286f1 closes the last gap I flagged and I found no further issues on this revision — deferring final sign-off to a human since this is WebCrypto bindings code.

Extended reasoning...

Overview

This PR makes every SubtleCrypto entry point validate caller-supplied BufferSource lengths against WTF::isValidCapacityForVector<uint8_t> before copying into a Vector<uint8_t>, rejecting the promise with OperationError instead of hitting CRASH() in Vector::allocateBuffer. After two follow-up commits it now covers: top-level data/signature/wrappedKey arguments via a fallible copyToVector, raw/spki/pkcs8 key data in toKeyData, every BufferSource member of the algorithm dictionaries (RSA-OAEP label, AES-CBC/CFB iv, AES-CTR counter, AES-GCM iv/additionalData, HKDF salt/info, PBKDF2 salt) validated during normalizeCryptoAlgorithmParameters, and RsaKeyGenParams.publicExponent (a RefPtr<Uint8Array>, not a BufferSource). A subprocess test exercises each path with a 2 GiB buffer plus a normal-sized round-trip.

Security risks

The change is strictly defensive — it adds early-reject guards and does not alter any cryptographic computation, key handling, or success-path data flow. The new checks only fire on inputs that previously aborted the process, so there is no behavioral change for valid inputs. I see no new attack surface introduced.

Level of scrutiny

This is C++ in the WebCrypto bindings, which is security-sensitive by nature even when the change itself is hardening. The pattern is mechanical and consistent across all call sites (check size → return Exception{OperationError} or reject + return nullopt), and the std::optional early-return contract mirrors the pre-existing toKeyData convention in the same file. Still, crypto-adjacent native code warrants a human reviewer's sign-off rather than bot approval.

Other factors

Both of my earlier inline comments (algorithm-dictionary members, then the BigInteger-typed publicExponent) were addressed in fa677ed and ae286f1 respectively, and the bug-hunting pass on ae286f1 surfaced nothing new. The robobun CI comment still references failures on the intermediate commit fa677ed and hasn't updated for ae286f1, so CI status on the final commit should be confirmed before merge.

[claude]

All earlier feedback is addressed and the change looks correct and well-tested, but since it threads new control flow through the WebCrypto bindings I'd like a human to give it a final glance.

Extended reasoning...

Overview

This PR hardens SubtleCrypto.cpp against process aborts when caller-supplied BufferSource inputs exceed the WTF::Vector<uint8_t> capacity cap. It makes copyToVector fallible (returning std::optional and rejecting the promise on overflow), guards toKeyData for raw/spki/pkcs8, adds three isAcceptableVectorSource overloads, and applies them to every BufferSource/BigInteger member in normalizeCryptoAlgorithmParameters (RSA-OAEP label, AES-CBC/CFB iv, AES-CTR counter, AES-GCM iv/additionalData, HKDF salt/info, PBKDF2 salt, RSA publicExponent). All seven entry points that consume the optional now early-return on nullopt and dereference at the use site. A new subprocess test exercises every path with a 2 GiB buffer and asserts OperationError plus a clean stderr and a working small round-trip.

Security risks

The change is purely defensive input validation — it adds an early reject path and does not alter any cryptographic algorithm, key material handling, or usage checks. The only behavioral change for in-range inputs is none (the happy path is byte-identical). I see no new injection, auth, or data-exposure surface; if anything this removes a user-reachable CRASH().

Level of scrutiny

This is WebKit-derived WebCrypto binding code. The diff is mechanical and consistent, but it changes a helper signature and threads promise-rejection control flow through ~8 call sites in a file that gates all SubtleCrypto operations. Per my guidelines, crypto-adjacent bindings warrant a human sign-off even when the change is defensive.

Other factors

Both of my earlier inline findings (unguarded algorithm-dictionary members; the RefPtr<Uint8Array> publicExponent gap) were fixed in follow-up commits, and CodeRabbit's stderr-assertion suggestion was applied. The bug-hunting pass on the current revision found nothing. The one CI failure (streams-leak.test.ts on a single Linux shard) is unrelated to this change. Test coverage for the new behavior is thorough.