Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
59 changes: 42 additions & 17 deletions src/jsc/bindings/webcrypto/SubtleCrypto.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -530,7 +530,11 @@
promise->reject(Exception { TypeError });
return std::nullopt;
},
[](auto& bufferSource) -> std::optional<KeyData> {
[&promise](auto& bufferSource) -> std::optional<KeyData> {
if (!WTF::isValidCapacityForVector<uint8_t>(bufferSource->byteLength())) {
promise->reject(OperationError, "Input data is too large"_s);
return std::nullopt;
}
return KeyData { Vector(std::span { static_cast<const uint8_t*>(bufferSource->data()), bufferSource->byteLength() }) };
}),
keyDataVariant);
Expand All @@ -551,9 +555,16 @@
RELEASE_ASSERT_NOT_REACHED();
}

static Vector<uint8_t> copyToVector(BufferSource&& data)
// WTF::Vector capacity is capped below the maximum legal ArrayBuffer size, and exceeding the cap
// CRASH()es inside Vector::allocateBuffer. Validate the length before copying and reject the
// promise instead, mirroring the toKeyData contract: nullopt means the promise was already rejected.
static std::optional<Vector<uint8_t>> copyToVector(BufferSource&& data, Ref<DeferredPromise>& promise)
{
return std::span { data.data(), data.length() };
if (!WTF::isValidCapacityForVector<uint8_t>(data.length())) {
promise->reject(OperationError, "Input data is too large"_s);
return std::nullopt;
}
return Vector<uint8_t> { std::span { data.data(), data.length() } };

Check notice on line 567 in src/jsc/bindings/webcrypto/SubtleCrypto.cpp

View check run for this annotation

Claude / Claude Code Review

Algorithm-parameter BufferSources still abort on oversized input

This guards the top-level `data`/`signature`/`wrappedKey`/key-data arguments, but BufferSource members inside the algorithm dictionaries are still copied into `Vector<uint8_t>` unguarded — e.g. AES-GCM `additionalData`/`iv`, AES-CBC/CTR `iv`/`counter`, HKDF `salt`/`info`, PBKDF2 `salt`, RSA-OAEP `label`. `crypto.subtle.encrypt({name:'AES-GCM', iv, additionalData: new Uint8Array(2**31)}, key, small)` will still hit `Vector::allocateBuffer`'s `CRASH()` and abort the process. This is pre-existing (
Comment thread
Jarred-Sumner marked this conversation as resolved.
}

static bool isSupportedExportKey(JSGlobalObject& state, CryptoAlgorithmIdentifier identifier)
Expand Down Expand Up @@ -630,7 +641,9 @@
}
auto params = paramsOrException.releaseReturnValue();

auto data = copyToVector(WTF::move(dataBufferSource));
auto data = copyToVector(WTF::move(dataBufferSource), promise);
if (!data)
return;

if (params->identifier != key.algorithmIdentifier()) {
promise->reject(InvalidAccessError, "CryptoKey doesn't match AlgorithmIdentifier"_s);
Expand All @@ -656,7 +669,7 @@
rejectWithException(promise.releaseNonNull(), ec, msg);
};

algorithm->encrypt(*params, key, WTF::move(data), WTF::move(callback), WTF::move(exceptionCallback), *scriptExecutionContext(), m_workQueue);
algorithm->encrypt(*params, key, WTF::move(*data), WTF::move(callback), WTF::move(exceptionCallback), *scriptExecutionContext(), m_workQueue);
}

void SubtleCrypto::decrypt(JSC::JSGlobalObject& state, AlgorithmIdentifier&& algorithmIdentifier, CryptoKey& key, BufferSource&& dataBufferSource, Ref<DeferredPromise>&& promise)
Expand All @@ -673,7 +686,9 @@
}
auto params = paramsOrException.releaseReturnValue();

auto data = copyToVector(WTF::move(dataBufferSource));
auto data = copyToVector(WTF::move(dataBufferSource), promise);
if (!data)
return;

if (params->identifier != key.algorithmIdentifier()) {
promise->reject(InvalidAccessError, "CryptoKey doesn't match AlgorithmIdentifier"_s);
Expand All @@ -699,7 +714,7 @@
rejectWithException(promise.releaseNonNull(), ec, msg);
};

algorithm->decrypt(*params, key, WTF::move(data), WTF::move(callback), WTF::move(exceptionCallback), *scriptExecutionContext(), m_workQueue);
algorithm->decrypt(*params, key, WTF::move(*data), WTF::move(callback), WTF::move(exceptionCallback), *scriptExecutionContext(), m_workQueue);
}

void SubtleCrypto::sign(JSC::JSGlobalObject& state, AlgorithmIdentifier&& algorithmIdentifier, CryptoKey& key, BufferSource&& dataBufferSource, Ref<DeferredPromise>&& promise)
Expand All @@ -711,7 +726,9 @@
}
auto params = paramsOrException.releaseReturnValue();

auto data = copyToVector(WTF::move(dataBufferSource));
auto data = copyToVector(WTF::move(dataBufferSource), promise);
if (!data)
return;

if (params->identifier != key.algorithmIdentifier()) {
promise->reject(InvalidAccessError, "CryptoKey doesn't match AlgorithmIdentifier"_s);
Expand All @@ -737,7 +754,7 @@
rejectWithException(promise.releaseNonNull(), ec, msg);
};

algorithm->sign(*params, key, WTF::move(data), WTF::move(callback), WTF::move(exceptionCallback), *scriptExecutionContext(), m_workQueue);
algorithm->sign(*params, key, WTF::move(*data), WTF::move(callback), WTF::move(exceptionCallback), *scriptExecutionContext(), m_workQueue);
}

void SubtleCrypto::verify(JSC::JSGlobalObject& state, AlgorithmIdentifier&& algorithmIdentifier, CryptoKey& key, BufferSource&& signatureBufferSource, BufferSource&& dataBufferSource, Ref<DeferredPromise>&& promise)
Expand All @@ -749,8 +766,12 @@
}
auto params = paramsOrException.releaseReturnValue();

auto signature = copyToVector(WTF::move(signatureBufferSource));
auto data = copyToVector(WTF::move(dataBufferSource));
auto signature = copyToVector(WTF::move(signatureBufferSource), promise);
if (!signature)
return;
auto data = copyToVector(WTF::move(dataBufferSource), promise);
if (!data)
return;

if (params->identifier != key.algorithmIdentifier()) {
promise->reject(InvalidAccessError, "CryptoKey doesn't match AlgorithmIdentifier"_s);
Expand All @@ -776,7 +797,7 @@
rejectWithException(promise.releaseNonNull(), ec, msg);
};

algorithm->verify(*params, key, WTF::move(signature), WTF::move(data), WTF::move(callback), WTF::move(exceptionCallback), *scriptExecutionContext(), m_workQueue);
algorithm->verify(*params, key, WTF::move(*signature), WTF::move(*data), WTF::move(callback), WTF::move(exceptionCallback), *scriptExecutionContext(), m_workQueue);
}

void SubtleCrypto::digest(JSC::JSGlobalObject& state, AlgorithmIdentifier&& algorithmIdentifier, BufferSource&& dataBufferSource, Ref<DeferredPromise>&& promise)
Expand All @@ -791,7 +812,9 @@
}
auto params = paramsOrException.releaseReturnValue();

auto data = copyToVector(WTF::move(dataBufferSource));
auto data = copyToVector(WTF::move(dataBufferSource), promise);
if (!data)
return;

auto algorithm = CryptoAlgorithmRegistry::singleton().create(params->identifier);

Expand All @@ -807,7 +830,7 @@
rejectWithException(promise.releaseNonNull(), ec, msg);
};

algorithm->digest(WTF::move(data), WTF::move(callback), WTF::move(exceptionCallback), *scriptExecutionContext(), m_workQueue);
algorithm->digest(WTF::move(*data), WTF::move(callback), WTF::move(exceptionCallback), *scriptExecutionContext(), m_workQueue);
}

void SubtleCrypto::generateKey(JSC::JSGlobalObject& state, AlgorithmIdentifier&& algorithmIdentifier, bool extractable, Vector<CryptoKeyUsage>&& keyUsages, Ref<DeferredPromise>&& promise)
Expand Down Expand Up @@ -1170,7 +1193,9 @@

void SubtleCrypto::unwrapKey(JSC::JSGlobalObject& state, KeyFormat format, BufferSource&& wrappedKeyBufferSource, CryptoKey& unwrappingKey, AlgorithmIdentifier&& unwrapAlgorithmIdentifier, AlgorithmIdentifier&& unwrappedKeyAlgorithmIdentifier, bool extractable, Vector<CryptoKeyUsage>&& keyUsages, Ref<DeferredPromise>&& promise)
{
auto wrappedKey = copyToVector(WTF::move(wrappedKeyBufferSource));
auto wrappedKey = copyToVector(WTF::move(wrappedKeyBufferSource), promise);
if (!wrappedKey)
return;

bool isDecryption = false;

Expand Down Expand Up @@ -1284,11 +1309,11 @@
// The 11 December 2014 version of the specification suggests we should perform the following task asynchronously:
// https://www.w3.org/TR/WebCryptoAPI/#SubtleCrypto-method-unwrapKey
// It is not beneficial for less time consuming operations. Therefore, we perform it synchronously.
unwrapAlgorithm->unwrapKey(unwrappingKey, WTF::move(wrappedKey), WTF::move(callback), WTF::move(exceptionCallback));
unwrapAlgorithm->unwrapKey(unwrappingKey, WTF::move(*wrappedKey), WTF::move(callback), WTF::move(exceptionCallback));
return;
}

unwrapAlgorithm->decrypt(*unwrapParams, unwrappingKey, WTF::move(wrappedKey), WTF::move(callback), WTF::move(exceptionCallback), *scriptExecutionContext(), m_workQueue);
unwrapAlgorithm->decrypt(*unwrapParams, unwrappingKey, WTF::move(*wrappedKey), WTF::move(callback), WTF::move(exceptionCallback), *scriptExecutionContext(), m_workQueue);
}

}
Expand Down
81 changes: 81 additions & 0 deletions test/js/web/crypto/web-crypto.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -207,6 +207,87 @@ describe("Web Crypto", () => {
});
});

describe("oversized inputs", () => {
// Every SubtleCrypto entry point copies its BufferSource argument into a
// WTF::Vector<uint8_t>, whose capacity is capped below the maximum legal
// ArrayBuffer size. Inputs above the cap must reject the promise instead of
// aborting the process. Run in a subprocess so the ~2GiB allocation does not
// bloat the test runner; the buffer is never written so RSS stays small.
it("rejects >2 GiB inputs instead of aborting", async () => {
const script = `
let big;
try {
big = new Uint8Array(2 ** 31);
} catch {
console.log("SKIP");
process.exit(0);
}

const aesKey = await crypto.subtle.importKey("raw", new Uint8Array(32).fill(1), { name: "AES-GCM" }, false, [
"encrypt",
"decrypt",
"unwrapKey",
]);
const hmacKey = await crypto.subtle.importKey(
"raw",
new Uint8Array(32).fill(2),
{ name: "HMAC", hash: "SHA-256" },
false,
["sign", "verify"],
);
const iv = new Uint8Array(12).fill(3);

const results = {};
const record = (label, promise) =>
promise.then(
() => (results[label] = "resolved"),
e => (results[label] = e.name),
);

await record("digest", crypto.subtle.digest("SHA-256", big));
await record("encrypt", crypto.subtle.encrypt({ name: "AES-GCM", iv }, aesKey, big));
await record("decrypt", crypto.subtle.decrypt({ name: "AES-GCM", iv }, aesKey, big));
await record("sign", crypto.subtle.sign("HMAC", hmacKey, big));
await record("verify data", crypto.subtle.verify("HMAC", hmacKey, new Uint8Array(32), big));
await record("verify signature", crypto.subtle.verify("HMAC", hmacKey, big, new Uint8Array(32)));
await record("importKey", crypto.subtle.importKey("raw", big, { name: "AES-GCM" }, false, ["encrypt"]));
await record(
"unwrapKey",
crypto.subtle.unwrapKey("raw", big, aesKey, { name: "AES-GCM", iv }, { name: "AES-GCM" }, false, ["encrypt"]),
);

// Normal-sized inputs must keep working in the same process.
await crypto.subtle.digest("SHA-256", new Uint8Array(16));
const ciphertext = await crypto.subtle.encrypt({ name: "AES-GCM", iv }, aesKey, new Uint8Array(16).fill(4));
const roundTrip = new Uint8Array(await crypto.subtle.decrypt({ name: "AES-GCM", iv }, aesKey, ciphertext));
results["small round-trip"] = roundTrip.every(b => b === 4) ? "ok" : "mismatch";

console.log(JSON.stringify(results));
`;
await using proc = Bun.spawn({
cmd: [bunExe(), "-e", script],
env: bunEnv,
stdout: "pipe",
stderr: "inherit",
});
Comment thread
coderabbitai[bot] marked this conversation as resolved.
const [stdout, exitCode] = await Promise.all([proc.stdout.text(), proc.exited]);
if (stdout.trim() !== "SKIP") {
expect(JSON.parse(stdout)).toEqual({
"digest": "OperationError",
"encrypt": "OperationError",
"decrypt": "OperationError",
"sign": "OperationError",
"verify data": "OperationError",
"verify signature": "OperationError",
"importKey": "OperationError",
"unwrapKey": "OperationError",
"small round-trip": "ok",
});
}
expect(exitCode).toBe(0);
});
});

describe("Ed25519", () => {
describe("generateKey", () => {
it("should return CryptoKeys without namedCurve in algorithm field", async () => {
Expand Down
Loading