From d23a10c1dc3d299d9176f1a9c4c5512de39a3051 Mon Sep 17 00:00:00 2001
From: Joyee Cheung <joyeec9h3@gmail.com>
Date: Fri, 24 Oct 2025 16:39:53 +0200
Subject: [PATCH] crypto: fix argument validation in crypto.timingSafeEqual
 fast path

A regression introduced by
https://github.com/nodejs/node/commit/0136bb0ee807b9789bf69e8d87e7a98e1b15f217
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.
---
 src/crypto/crypto_timing.cc                   | 14 +++++++
 .../test-crypto-timing-safe-equal-fast.js     | 40 +++++++++++++++++++
 .../test-crypto-timing-safe-equal.js          | 27 +------------
 3 files changed, 55 insertions(+), 26 deletions(-)
 create mode 100644 test/sequential/test-crypto-timing-safe-equal-fast.js

diff --git a/src/crypto/crypto_timing.cc b/src/crypto/crypto_timing.cc
index a0194f22cfd51e..0601f70926b245 100644
--- a/src/crypto/crypto_timing.cc
+++ b/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()) {
diff --git a/test/sequential/test-crypto-timing-safe-equal-fast.js b/test/sequential/test-crypto-timing-safe-equal-fast.js
new file mode 100644
index 00000000000000..0774eeae561fbb
--- /dev/null
+++ b/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);
+}
diff --git a/test/sequential/test-crypto-timing-safe-equal.js b/test/sequential/test-crypto-timing-safe-equal.js
index 80b3a69e59cb2d..74ae39fbafcd0c 100644
--- a/test/sequential/test-crypto-timing-safe-equal.js
+++ b/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);
-  }
-}