crypto: move typechecking for timingSafeEqual into C++

This makes the function more robust against V8 inlining.

Fixes: https://github.com/nodejs/node/issues/34073

PR-URL: https://github.com/nodejs/node/pull/34141
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: Ujjwal Sharma <ryzokuken@disroot.org>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Zeyu Yang <himself65@outlook.com>

1 files changed, 23 insertions, 3 deletions

diff --git a/src/node_crypto.cc b/src/node_crypto.cc
index e34b27b166c..aace8e2c05e 100644
--- a/src/node_crypto.cc
+++ b/src/node_crypto.cc
@@ -6831,10 +6831,30 @@ void StatelessDiffieHellman(const FunctionCallbackInfo<Value>& args) {
 
 
 void TimingSafeEqual(const FunctionCallbackInfo<Value>& args) {
-  ArrayBufferViewContents<char> buf1(args[0]);
-  ArrayBufferViewContents<char> buf2(args[1]);
+  // Moving the type checking into JS leads to test failures, most likely due
+  // to V8 inlining certain parts of the wrapper. Therefore, keep them in C++.
+  // Refs: https://github.com/nodejs/node/issues/34073.
+  Environment* env = Environment::GetCurrent(args);
+  if (!args[0]->IsArrayBufferView()) {
+    THROW_ERR_INVALID_ARG_TYPE(
+      env, "The \"buf1\" argument must be an instance of "
+      "Buffer, TypedArray, or DataView.");
+    return;
+  }
+  if (!args[1]->IsArrayBufferView()) {
+    THROW_ERR_INVALID_ARG_TYPE(
+      env, "The \"buf2\" argument must be an instance of "
+      "Buffer, TypedArray, or DataView.");
+    return;
+  }
+
+  ArrayBufferViewContents<char> buf1(args[0].As<ArrayBufferView>());
+  ArrayBufferViewContents<char> buf2(args[1].As<ArrayBufferView>());
 
-  CHECK_EQ(buf1.length(), buf2.length());
+  if (buf1.length() != buf2.length()) {
+    THROW_ERR_CRYPTO_TIMING_SAFE_EQUAL_LENGTH(env);
+    return;
+  }
 
   return args.GetReturnValue().Set(
       CRYPTO_memcmp(buf1.data(), buf2.data(), buf1.length()) == 0);