From 8ee962d79f0bbdef63667a31ad3ff8d2795fd3c6 Mon Sep 17 00:00:00 2001 From: Dylan Conway <35280289+dylan-conway@users.noreply.github.com> Date: Tue, 25 Mar 2025 20:51:36 -0700 Subject: [PATCH] Fix #14881 (#18483) --- src/bun.js/bindings/ErrorCode.cpp | 18 +++++++++++++ src/bun.js/bindings/ErrorCode.h | 1 + .../bindings/node/crypto/CryptoUtil.cpp | 6 ++--- src/bun.js/bindings/node/crypto/JSVerify.cpp | 27 ++++++++++++++----- test/js/node/crypto/node-crypto.test.js | 20 ++++++++++++++ 5 files changed, 62 insertions(+), 10 deletions(-) diff --git a/src/bun.js/bindings/ErrorCode.cpp b/src/bun.js/bindings/ErrorCode.cpp index d74a1a3c05..b72aab1b27 100644 --- a/src/bun.js/bindings/ErrorCode.cpp +++ b/src/bun.js/bindings/ErrorCode.cpp @@ -647,6 +647,24 @@ JSC::EncodedJSValue INVALID_ARG_TYPE(JSC::ThrowScope& throwScope, JSC::JSGlobalO return {}; } +JSC::EncodedJSValue INVALID_ARG_TYPE_INSTANCE(JSC::ThrowScope& throwScope, JSC::JSGlobalObject* globalObject, WTF::ASCIILiteral arg_name, WTF::ASCIILiteral expected_type, WTF::ASCIILiteral expected_instance_types, JSC::JSValue val_actual_value) +{ + JSC::VM& vm = globalObject->vm(); + WTF::StringBuilder builder; + builder.append("The \""_s); + builder.append(arg_name); + builder.append("\" argument must be of type "_s); + builder.append(expected_type); + builder.append(" or an instance of "_s); + builder.append(expected_instance_types); + builder.append(". Received "_s); + determineSpecificType(vm, globalObject, builder, val_actual_value); + RETURN_IF_EXCEPTION(throwScope, {}); + + throwScope.throwException(globalObject, createError(globalObject, ErrorCode::ERR_INVALID_ARG_TYPE, builder.toString())); + return {}; +} + // When you want INVALID_ARG_TYPE to say "The argument must be an instance of X. Received Y." instead of "The argument must be of type X. Received Y." JSC::EncodedJSValue INVALID_ARG_INSTANCE(JSC::ThrowScope& throwScope, JSC::JSGlobalObject* globalObject, const WTF::String& arg_name, const WTF::String& expected_type, JSC::JSValue val_actual_value) { diff --git a/src/bun.js/bindings/ErrorCode.h b/src/bun.js/bindings/ErrorCode.h index d0ec9ce166..1e61d02b76 100644 --- a/src/bun.js/bindings/ErrorCode.h +++ b/src/bun.js/bindings/ErrorCode.h @@ -68,6 +68,7 @@ namespace ERR { JSC::EncodedJSValue INVALID_ARG_TYPE(JSC::ThrowScope& throwScope, JSC::JSGlobalObject* globalObject, const WTF::String& arg_name, const WTF::String& expected_type, JSC::JSValue val_actual_value); JSC::EncodedJSValue INVALID_ARG_TYPE(JSC::ThrowScope& throwScope, JSC::JSGlobalObject* globalObject, JSC::JSValue arg_name, const WTF::String& expected_type, JSC::JSValue val_actual_value); JSC::EncodedJSValue INVALID_ARG_INSTANCE(JSC::ThrowScope& throwScope, JSC::JSGlobalObject* globalObject, const WTF::String& arg_name, const WTF::String& expected_type, JSC::JSValue val_actual_value); +JSC::EncodedJSValue INVALID_ARG_TYPE_INSTANCE(JSC::ThrowScope& throwScope, JSC::JSGlobalObject* globalObject, WTF::ASCIILiteral arg_name, WTF::ASCIILiteral expected_type, WTF::ASCIILiteral expected_instance_types, JSC::JSValue val_actual_value); JSC::EncodedJSValue OUT_OF_RANGE(JSC::ThrowScope& throwScope, JSC::JSGlobalObject* globalObject, const WTF::String& arg_name, double lower, double upper, JSC::JSValue actual); JSC::EncodedJSValue OUT_OF_RANGE(JSC::ThrowScope& throwScope, JSC::JSGlobalObject* globalObject, JSC::JSValue arg_name, double lower, double upper, JSC::JSValue actual); JSC::EncodedJSValue OUT_OF_RANGE(JSC::ThrowScope& throwScope, JSC::JSGlobalObject* globalObject, JSC::JSValue arg_name_val, double bound_num, Bound bound, JSC::JSValue actual); diff --git a/src/bun.js/bindings/node/crypto/CryptoUtil.cpp b/src/bun.js/bindings/node/crypto/CryptoUtil.cpp index 70e3d04c89..87d40e6cd2 100644 --- a/src/bun.js/bindings/node/crypto/CryptoUtil.cpp +++ b/src/bun.js/bindings/node/crypto/CryptoUtil.cpp @@ -502,7 +502,7 @@ JSC::JSArrayBufferView* getArrayBufferOrView(JSGlobalObject* globalObject, Throw auto* view = jsDynamicCast(buf); if (!view) { - Bun::ERR::INVALID_ARG_INSTANCE(scope, globalObject, argName, "Buffer, TypedArray, or DataView"_s, value); + ERR::INVALID_ARG_TYPE_INSTANCE(scope, globalObject, argName, "string"_s, "Buffer, TypedArray, or DataView"_s, value); return {}; } @@ -515,13 +515,13 @@ JSC::JSArrayBufferView* getArrayBufferOrView(JSGlobalObject* globalObject, Throw } if (!value.isCell() || !JSC::isTypedArrayTypeIncludingDataView(value.asCell()->type())) { - Bun::ERR::INVALID_ARG_INSTANCE(scope, globalObject, argName, "Buffer, TypedArray, or DataView"_s, value); + ERR::INVALID_ARG_TYPE_INSTANCE(scope, globalObject, argName, "string"_s, "Buffer, TypedArray, or DataView"_s, value); return {}; } auto* view = JSC::jsDynamicCast(value); if (!view) { - Bun::ERR::INVALID_ARG_INSTANCE(scope, globalObject, argName, "Buffer, TypedArray, or DataView"_s, value); + ERR::INVALID_ARG_TYPE_INSTANCE(scope, globalObject, argName, "string"_s, "Buffer, TypedArray, or DataView"_s, value); return {}; } diff --git a/src/bun.js/bindings/node/crypto/JSVerify.cpp b/src/bun.js/bindings/node/crypto/JSVerify.cpp index 78f5e2108b..29825a3c96 100644 --- a/src/bun.js/bindings/node/crypto/JSVerify.cpp +++ b/src/bun.js/bindings/node/crypto/JSVerify.cpp @@ -456,12 +456,25 @@ JSC_DEFINE_HOST_FUNCTION(jsVerifyOneShot, (JSC::JSGlobalObject * globalObject, J return Bun::ERR::INVALID_ARG_TYPE(scope, globalObject, "data"_s, "Buffer, TypedArray, or DataView"_s, dataValue); } - // Get signature argument + // Get signature argument. Can only be a buffer type, not a string JSValue signatureValue = callFrame->argument(3); - JSC::JSArrayBufferView* signatureView = getArrayBufferOrView(globalObject, scope, signatureValue, "signature"_s, jsUndefined()); - RETURN_IF_EXCEPTION(scope, {}); - if (!signatureView) { - return Bun::ERR::INVALID_ARG_TYPE(scope, globalObject, "signature"_s, "Buffer, TypedArray, or DataView"_s, signatureValue); + std::span signature; + if (JSArrayBufferView* view = jsDynamicCast(signatureValue)) { + if (UNLIKELY(view->isDetached())) { + throwVMTypeError(globalObject, scope, "Buffer is detached"_s); + return {}; + } + signature = view->span(); + } else if (JSArrayBuffer* buf = jsDynamicCast(signatureValue)) { + ArrayBuffer* bufImpl = buf->impl(); + if (UNLIKELY(bufImpl->isDetached())) { + throwVMTypeError(globalObject, scope, "Buffer is detached"_s); + return {}; + } + + signature = bufImpl->span(); + } else { + ERR::INVALID_ARG_INSTANCE(scope, globalObject, "signature"_s, "Buffer, TypedArray, or DataView"_s, signatureValue); } // Get key argument @@ -506,8 +519,8 @@ JSC_DEFINE_HOST_FUNCTION(jsVerifyOneShot, (JSC::JSGlobalObject * globalObject, J // Create signature buffer ncrypto::Buffer sigBuf { - .data = static_cast(signatureView->vector()), - .len = signatureView->byteLength() + .data = signature.data(), + .len = signature.size(), }; // Create a new EVP_MD_CTX for verification diff --git a/test/js/node/crypto/node-crypto.test.js b/test/js/node/crypto/node-crypto.test.js index b302647daa..2826ab80b0 100644 --- a/test/js/node/crypto/node-crypto.test.js +++ b/test/js/node/crypto/node-crypto.test.js @@ -548,6 +548,26 @@ it("createDecipheriv should validate iv and password", () => { expect(() => crypto.createDecipheriv("aes-128-cbc", key, Buffer.alloc(16)).setAutoPadding(false)).not.toThrow(); }); +it("Cipheriv.update throws expected error for invalid data", () => { + const key = crypto.randomBytes(32); + const iv = crypto.randomBytes(16); + const cipher = crypto.createCipheriv("aes-256-gcm", key, iv); + expect(() => { + cipher.update(["c", "d"]); + }).toThrow( + 'The "data" argument must be of type string or an instance of Buffer, TypedArray, or DataView. Received an instance of Array', + ); +}); + +it("verifyOneShot should not accept strings for signatures", () => { + const data = Buffer.alloc(1); + expect(() => { + crypto.verify(null, data, "test", "oops"); + }).toThrow( + "The \"signature\" argument must be an instance of Buffer, TypedArray, or DataView. Received type string ('oops')", + ); +}); + it("x25519", () => { // Generate Alice's keys const alice = crypto.generateKeyPairSync("x25519", {