From e1d40226eeac4e46caee9f89e85729ded6b0e42d Mon Sep 17 00:00:00 2001 From: Darshan Sen Date: Sun, 27 Mar 2022 18:02:52 +0530 Subject: [PATCH 1/3] src,crypto: remove uses of AllocatedBuffer from crypto_dh.cc Refs: https://github.com/nodejs/node/pull/39941 Signed-off-by: Darshan Sen --- src/crypto/crypto_dh.cc | 76 ++++++++++++++++++++++++++++------------- 1 file changed, 52 insertions(+), 24 deletions(-) diff --git a/src/crypto/crypto_dh.cc b/src/crypto/crypto_dh.cc index 8a2c559d783eda..b6147995a24276 100644 --- a/src/crypto/crypto_dh.cc +++ b/src/crypto/crypto_dh.cc @@ -13,6 +13,8 @@ namespace node { +using v8::ArrayBuffer; +using v8::BackingStore; using v8::ConstructorBehavior; using v8::DontDelete; using v8::FunctionCallback; @@ -50,10 +52,6 @@ static void ZeroPadDiffieHellmanSecret(size_t remainder_size, memset(data, 0, padding); } } -static void ZeroPadDiffieHellmanSecret(size_t remainder_size, - AllocatedBuffer* ret) { - ZeroPadDiffieHellmanSecret(remainder_size, ret->data(), ret->size()); -} } // namespace DiffieHellman::DiffieHellman(Environment* env, Local wrap) @@ -275,13 +273,24 @@ void DiffieHellman::GenerateKeys(const FunctionCallbackInfo& args) { const BIGNUM* pub_key; DH_get0_key(diffieHellman->dh_.get(), &pub_key, nullptr); - const int size = BN_num_bytes(pub_key); - CHECK_GE(size, 0); - AllocatedBuffer data = AllocatedBuffer::AllocateManaged(env, size); - CHECK_EQ(size, - BN_bn2binpad( - pub_key, reinterpret_cast(data.data()), size)); - args.GetReturnValue().Set(data.ToBuffer().FromMaybe(Local())); + + std::unique_ptr bs; + { + const int size = BN_num_bytes(pub_key); + CHECK_GE(size, 0); + NoArrayBufferZeroFillScope no_zero_fill_scope(env->isolate_data()); + bs = ArrayBuffer::NewBackingStore(env->isolate(), size); + } + + CHECK_EQ(static_cast(bs->ByteLength()), + BN_bn2binpad(pub_key, + static_cast(bs->Data()), + bs->ByteLength())); + + Local ab = ArrayBuffer::New(env->isolate(), std::move(bs)); + Local buffer; + if (!Buffer::New(env, ab, 0, ab->ByteLength()).ToLocal(&buffer)) return; + args.GetReturnValue().Set(buffer); } @@ -297,13 +306,23 @@ void DiffieHellman::GetField(const FunctionCallbackInfo& args, if (num == nullptr) return THROW_ERR_CRYPTO_INVALID_STATE(env, err_if_null); - const int size = BN_num_bytes(num); - CHECK_GE(size, 0); - AllocatedBuffer data = AllocatedBuffer::AllocateManaged(env, size); - CHECK_EQ( - size, - BN_bn2binpad(num, reinterpret_cast(data.data()), size)); - args.GetReturnValue().Set(data.ToBuffer().FromMaybe(Local())); + std::unique_ptr bs; + { + const int size = BN_num_bytes(num); + CHECK_GE(size, 0); + NoArrayBufferZeroFillScope no_zero_fill_scope(env->isolate_data()); + bs = ArrayBuffer::NewBackingStore(env->isolate(), size); + } + + CHECK_EQ(static_cast(bs->ByteLength()), + BN_bn2binpad(num, + static_cast(bs->Data()), + bs->ByteLength())); + + Local ab = ArrayBuffer::New(env->isolate(), std::move(bs)); + Local buffer; + if (!Buffer::New(env, ab, 0, ab->ByteLength()).ToLocal(&buffer)) return; + args.GetReturnValue().Set(buffer); } void DiffieHellman::GetPrime(const FunctionCallbackInfo& args) { @@ -352,10 +371,14 @@ void DiffieHellman::ComputeSecret(const FunctionCallbackInfo& args) { return THROW_ERR_OUT_OF_RANGE(env, "secret is too big"); BignumPointer key(BN_bin2bn(key_buf.data(), key_buf.size(), nullptr)); - AllocatedBuffer ret = - AllocatedBuffer::AllocateManaged(env, DH_size(diffieHellman->dh_.get())); + std::unique_ptr bs; + { + NoArrayBufferZeroFillScope no_zero_fill_scope(env->isolate_data()); + bs = ArrayBuffer::NewBackingStore(env->isolate(), + DH_size(diffieHellman->dh_.get())); + } - int size = DH_compute_key(reinterpret_cast(ret.data()), + int size = DH_compute_key(static_cast(bs->Data()), key.get(), diffieHellman->dh_.get()); @@ -383,9 +406,14 @@ void DiffieHellman::ComputeSecret(const FunctionCallbackInfo& args) { } CHECK_GE(size, 0); - ZeroPadDiffieHellmanSecret(static_cast(size), &ret); - - args.GetReturnValue().Set(ret.ToBuffer().FromMaybe(Local())); + ZeroPadDiffieHellmanSecret(size, + static_cast(bs->Data()), + bs->ByteLength()); + + Local ab = ArrayBuffer::New(env->isolate(), std::move(bs)); + Local buffer; + if (!Buffer::New(env, ab, 0, ab->ByteLength()).ToLocal(&buffer)) return; + args.GetReturnValue().Set(buffer); } void DiffieHellman::SetKey(const FunctionCallbackInfo& args, From 7da39fae63ab341ed2fe74d52982c833d6c3b70c Mon Sep 17 00:00:00 2001 From: Darshan Sen Date: Sun, 27 Mar 2022 18:06:08 +0530 Subject: [PATCH 2/3] src: remove unnecessary static qualifier in crypto_dh.cc ZeroPadDiffieHellmanSecret() is in an anonymous namespace, so it has static linkage already. Signed-off-by: Darshan Sen --- src/crypto/crypto_dh.cc | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/crypto/crypto_dh.cc b/src/crypto/crypto_dh.cc index b6147995a24276..83f101e39b96a6 100644 --- a/src/crypto/crypto_dh.cc +++ b/src/crypto/crypto_dh.cc @@ -37,9 +37,9 @@ using v8::Value; namespace crypto { namespace { -static void ZeroPadDiffieHellmanSecret(size_t remainder_size, - char* data, - size_t length) { +void ZeroPadDiffieHellmanSecret(size_t remainder_size, + char* data, + size_t length) { // DH_size returns number of bytes in a prime number. // DH_compute_key returns number of bytes in a remainder of exponent, which // may have less bytes than a prime number. Therefore add 0-padding to the From c1ac41dce2ecd8de82bbd008a9db1d0d0a321335 Mon Sep 17 00:00:00 2001 From: Darshan Sen Date: Sun, 27 Mar 2022 18:20:20 +0530 Subject: [PATCH 3/3] src,crypto: handle empty maybe correctly in crypto_dh.cc Buffer::Length() dereferences the passed Local, so calling it when the underlying pointer is a nullptr would lead to a crash. This fixes that by returning early instead. Signed-off-by: Darshan Sen --- src/crypto/crypto_dh.cc | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/crypto/crypto_dh.cc b/src/crypto/crypto_dh.cc index 83f101e39b96a6..702c5e083f8f80 100644 --- a/src/crypto/crypto_dh.cc +++ b/src/crypto/crypto_dh.cc @@ -32,7 +32,6 @@ using v8::ReadOnly; using v8::SideEffectType; using v8::Signature; using v8::String; -using v8::Uint8Array; using v8::Value; namespace crypto { @@ -637,8 +636,10 @@ void DiffieHellman::Stateless(const FunctionCallbackInfo& args) { ManagedEVPPKey our_key = our_key_object->Data()->GetAsymmetricKey(); ManagedEVPPKey their_key = their_key_object->Data()->GetAsymmetricKey(); - Local out = StatelessDiffieHellmanThreadsafe(our_key, their_key) - .ToBuffer(env).FromMaybe(Local()); + Local out; + if (!StatelessDiffieHellmanThreadsafe(our_key, their_key) + .ToBuffer(env) + .ToLocal(&out)) return; if (Buffer::Length(out) == 0) return ThrowCryptoError(env, ERR_get_error(), "diffieHellman failed");