crypto: fix argument validation in crypto.timingSafeEqual fast path

A regression introduced by
0136bb0
made it possible for the fast path to be hit with non-array-buffer
arguments despite that the fast paths could only deal with array
buffer arguments, so that it can crash with invalid arguments
once crypto.timingSafeEqual is optimized instead of throwing
validation errors as usual. This adds validation to the fast path
so that it throws correctly.

PR-URL: #60538
Fixes: #60537
Refs: nodejs-private/node-private#749
Reviewed-By: Rafael Gonzaga <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>

Author: joyeecheung

src/crypto/crypto_timing.cc

@@ -56,6 +56,20 @@ bool FastTimingSafeEqual(Local<Value> receiver,
                          // NOLINTNEXTLINE(runtime/references)
                          FastApiCallbackOptions& options) {
   HandleScope scope(options.isolate);
+  if (!IsAnyBufferSource(a_obj)) {
+    TRACK_V8_FAST_API_CALL("crypto.timingSafeEqual.error");
+    THROW_ERR_INVALID_ARG_TYPE(options.isolate,
+                               "The \"buf1\" argument must be an instance of "
+                               "ArrayBuffer, Buffer, TypedArray, or DataView.");
+    return false;
+  }
+  if (!IsAnyBufferSource(b_obj)) {
+    TRACK_V8_FAST_API_CALL("crypto.timingSafeEqual.error");
+    THROW_ERR_INVALID_ARG_TYPE(options.isolate,
+                               "The \"buf2\" argument must be an instance of "
+                               "ArrayBuffer, Buffer, TypedArray, or DataView.");
+    return false;
+  }
   ArrayBufferViewContents<uint8_t> a(a_obj);
   ArrayBufferViewContents<uint8_t> b(b_obj);
   if (a.length() != b.length()) {

test/sequential/test-crypto-timing-safe-equal-fast.js

@@ -0,0 +1,40 @@
+// Test v8 fast path for crypto.timingSafeEqual works correctly.
+// Flags: --expose-internals --allow-natives-syntax
+'use strict';
+
+const common = require('../common');
+
+if (!common.hasCrypto)
+  common.skip('missing crypto');
+
+const assert = require('assert');
+const crypto = require('crypto');
+
+// V8 Fast API
+const foo = Buffer.from('foo');
+const bar = Buffer.from('bar');
+const longer = Buffer.from('longer');
+function testFastPath(buf1, buf2) {
+  return crypto.timingSafeEqual(buf1, buf2);
+}
+eval('%PrepareFunctionForOptimization(testFastPath)');
+assert.strictEqual(testFastPath(foo, bar), false);
+eval('%OptimizeFunctionOnNextCall(testFastPath)');
+assert.strictEqual(testFastPath(foo, bar), false);
+assert.strictEqual(testFastPath(foo, foo), true);
+assert.throws(() => testFastPath(foo, longer), {
+  code: 'ERR_CRYPTO_TIMING_SAFE_EQUAL_LENGTH',
+});
+assert.throws(() => testFastPath(foo, ''), {
+  code: 'ERR_INVALID_ARG_TYPE',
+});
+assert.throws(() => testFastPath('', ''), {
+  code: 'ERR_INVALID_ARG_TYPE',
+});
+
+if (common.isDebug) {
+  const { internalBinding } = require('internal/test/binding');
+  const { getV8FastApiCallCount } = internalBinding('debug');
+  assert.strictEqual(getV8FastApiCallCount('crypto.timingSafeEqual.ok'), 2);
+  assert.strictEqual(getV8FastApiCallCount('crypto.timingSafeEqual.error'), 3);
+}

test/sequential/test-crypto-timing-safe-equal.js

@@ -1,4 +1,4 @@
-// Flags: --expose-internals --no-warnings --allow-natives-syntax
+// Flags: --no-warnings
 'use strict';
 const common = require('../common');
 if (!common.hasCrypto)
@@ -92,28 +92,3 @@ assert.throws(
     name: 'TypeError',
   }
 );
-
-{
-  // V8 Fast API
-  const foo = Buffer.from('foo');
-  const bar = Buffer.from('bar');
-  const longer = Buffer.from('longer');
-  function testFastPath(buf1, buf2) {
-    return crypto.timingSafeEqual(buf1, buf2);
-  }
-  eval('%PrepareFunctionForOptimization(testFastPath)');
-  assert.strictEqual(testFastPath(foo, bar), false);
-  eval('%OptimizeFunctionOnNextCall(testFastPath)');
-  assert.strictEqual(testFastPath(foo, bar), false);
-  assert.strictEqual(testFastPath(foo, foo), true);
-  assert.throws(() => testFastPath(foo, longer), {
-    code: 'ERR_CRYPTO_TIMING_SAFE_EQUAL_LENGTH',
-  });
-
-  if (common.isDebug) {
-    const { internalBinding } = require('internal/test/binding');
-    const { getV8FastApiCallCount } = internalBinding('debug');
-    assert.strictEqual(getV8FastApiCallCount('crypto.timingSafeEqual.ok'), 2);
-    assert.strictEqual(getV8FastApiCallCount('crypto.timingSafeEqual.error'), 1);
-  }
-}