Skip to content
Merged
Show file tree
Hide file tree
Changes from 2 commits
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
87 changes: 70 additions & 17 deletions src/jsc/bindings/webcrypto/SubtleCrypto.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,22 @@
// return context && context->settingsValues().webCryptoSafeCurvesEnabled;
}

// The lazy *Vector() accessors on the parameter classes copy these dictionary members into
// Vector<uint8_t> with no size check, and exceeding the Vector capacity cap CRASH()es in
// allocateBuffer. Validate them while normalizing so an oversized member rejects instead.
static bool isAcceptableVectorSource(const BufferSource& data)
{
return WTF::isValidCapacityForVector<uint8_t>(data.length());
}

static bool isAcceptableVectorSource(const std::optional<BufferSource::VariantType>& data)
{
if (!data)
return true;
auto length = std::visit([](auto& buffer) -> size_t { return buffer ? buffer->byteLength() : 0; }, *data);
return WTF::isValidCapacityForVector<uint8_t>(length);
}

Check failure on line 123 in src/jsc/bindings/webcrypto/SubtleCrypto.cpp

View check run for this annotation

Claude / Claude Code Review

RsaKeyGenParams.publicExponent not bounded by oversized-BufferSource guard

The follow-up commit covers every dictionary member typed `BufferSource`, but `RsaKeyGenParams.publicExponent` is typed `RefPtr<Uint8Array>` (WebIDL `BigInteger`) so it slips past both `isAcceptableVectorSource` overloads and the `Operations::GenerateKey` branch adds no check for it. `publicExponentVector()` (CryptoAlgorithmRsaKeyGenParams.h:48) does the same unguarded `append(std::span{data, byteLength})`, so `crypto.subtle.generateKey({name:'RSA-OAEP', modulusLength:2048, publicExponent:new Ui
Comment thread
Jarred-Sumner marked this conversation as resolved.

static ExceptionOr<std::unique_ptr<CryptoAlgorithmParameters>> normalizeCryptoAlgorithmParameters(JSGlobalObject& state, SubtleCrypto::AlgorithmIdentifier algorithmIdentifier, Operations operation)
{
VM& vm = state.vm();
Expand Down Expand Up @@ -143,25 +159,33 @@
case CryptoAlgorithmIdentifier::RSA_OAEP: {
auto params = convertDictionary<CryptoAlgorithmRsaOaepParams>(state, value.get());
RETURN_IF_EXCEPTION(scope, Exception { ExistingExceptionError });
if (!isAcceptableVectorSource(params.label))
return Exception { OperationError, "Input data is too large"_s };
result = makeUnique<CryptoAlgorithmRsaOaepParams>(params);
break;
}
case CryptoAlgorithmIdentifier::AES_CBC:
case CryptoAlgorithmIdentifier::AES_CFB: {
auto params = convertDictionary<CryptoAlgorithmAesCbcCfbParams>(state, value.get());
RETURN_IF_EXCEPTION(scope, Exception { ExistingExceptionError });
if (!isAcceptableVectorSource(params.iv))
return Exception { OperationError, "Input data is too large"_s };
result = makeUnique<CryptoAlgorithmAesCbcCfbParams>(params);
break;
}
case CryptoAlgorithmIdentifier::AES_CTR: {
auto params = convertDictionary<CryptoAlgorithmAesCtrParams>(state, value.get());
RETURN_IF_EXCEPTION(scope, Exception { ExistingExceptionError });
if (!isAcceptableVectorSource(params.counter))
return Exception { OperationError, "Input data is too large"_s };
result = makeUnique<CryptoAlgorithmAesCtrParams>(params);
break;
}
case CryptoAlgorithmIdentifier::AES_GCM: {
auto params = convertDictionary<CryptoAlgorithmAesGcmParams>(state, value.get());
RETURN_IF_EXCEPTION(scope, Exception { ExistingExceptionError });
if (!isAcceptableVectorSource(params.iv) || !isAcceptableVectorSource(params.additionalData))
return Exception { OperationError, "Input data is too large"_s };
result = makeUnique<CryptoAlgorithmAesGcmParams>(params);
break;
}
Expand Down Expand Up @@ -309,6 +333,8 @@
case CryptoAlgorithmIdentifier::HKDF: {
auto params = convertDictionary<CryptoAlgorithmHkdfParams>(state, value.get());
RETURN_IF_EXCEPTION(scope, Exception { ExistingExceptionError });
if (!isAcceptableVectorSource(params.salt) || !isAcceptableVectorSource(params.info))
return Exception { OperationError, "Input data is too large"_s };
auto hashIdentifier = toHashIdentifier(state, params.hash);
RETURN_IF_EXCEPTION(scope, Exception { ExistingExceptionError });
if (hashIdentifier.hasException()) return hashIdentifier.releaseException();
Expand All @@ -319,6 +345,8 @@
case CryptoAlgorithmIdentifier::PBKDF2: {
auto params = convertDictionary<CryptoAlgorithmPbkdf2Params>(state, value.get());
RETURN_IF_EXCEPTION(scope, Exception { ExistingExceptionError });
if (!isAcceptableVectorSource(params.salt))
return Exception { OperationError, "Input data is too large"_s };
auto hashIdentifier = toHashIdentifier(state, params.hash);
RETURN_IF_EXCEPTION(scope, Exception { ExistingExceptionError });
if (hashIdentifier.hasException()) return hashIdentifier.releaseException();
Expand Down Expand Up @@ -530,7 +558,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 +583,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() } };
Comment thread
Jarred-Sumner marked this conversation as resolved.
}

static bool isSupportedExportKey(JSGlobalObject& state, CryptoAlgorithmIdentifier identifier)
Expand Down Expand Up @@ -630,7 +669,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 +697,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 +714,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 +742,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 +754,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 +782,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 +794,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 +825,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 +840,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 +858,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 +1221,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 +1337,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
101 changes: 101 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,107 @@ 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 cbcKey = await crypto.subtle.importKey("raw", new Uint8Array(32).fill(5), { name: "AES-CBC" }, false, [
"encrypt",
]);
const hkdfKey = await crypto.subtle.importKey("raw", new Uint8Array(32).fill(6), "HKDF", false, ["deriveBits"]);
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"]),
);

// BufferSource members of the algorithm dictionaries are copied into
// Vectors by the parameter classes' lazy accessors, not by the entry
// points, so they need their own guard.
await record(
"encrypt additionalData",
crypto.subtle.encrypt({ name: "AES-GCM", iv, additionalData: big }, aesKey, new Uint8Array(16)),
);
await record("encrypt iv", crypto.subtle.encrypt({ name: "AES-CBC", iv: big }, cbcKey, new Uint8Array(16)));
await record(
"deriveBits salt",
crypto.subtle.deriveBits({ name: "HKDF", hash: "SHA-256", salt: big, info: new Uint8Array(0) }, hkdfKey, 256),
);

// 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",
"encrypt additionalData": "OperationError",
"encrypt iv": "OperationError",
"deriveBits salt": "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