From 2a0acd11a964fde8d572a8d4668e850815cd8b01 Mon Sep 17 00:00:00 2001
From: toanhoang <toanhoang@appsrv01.toanhoang.local>
Date: Tue, 4 Aug 2026 06:39:08 -0400
Subject: [PATCH] fix: run dummy bcrypt.compare on unknown user to prevent
 timing side-channel

---
 src/lib/users.ts   | 20 ++++++++++++++++++++
 tests/auth.test.ts | 16 ++++++++++++++++
 2 files changed, 36 insertions(+)

diff --git a/src/lib/users.ts b/src/lib/users.ts
index ec1da71..014467f 100644
--- a/src/lib/users.ts
+++ b/src/lib/users.ts
@@ -20,6 +20,21 @@ const BCRYPT_SALT_ROUNDS = 10;
 // grows into employee self-service.
 const MIN_PASSWORD_LENGTH = 8;
 
+// Precomputed bcrypt hash of a fixed dummy password, hashed with the same
+// cost factor (BCRYPT_SALT_ROUNDS) used for real user passwords. Used as a
+// timing-safety decoy in verifyUserCredentials() below: when the looked-up
+// email doesn't exist, we still run a bcrypt.compare() against this hash
+// (whose result is always discarded) so that the "unknown email" code path
+// takes roughly the same time as the "wrong password" path — otherwise the
+// early return would let an attacker distinguish "email doesn't exist" from
+// "email exists, password wrong" purely by response latency (bcrypt.compare
+// dominates request time). Computed once at module load via hashSync (not
+// per-request) since hashing itself is deliberately expensive.
+const DUMMY_PASSWORD_HASH = bcrypt.hashSync(
+  "dummy-password-for-timing-safety",
+  BCRYPT_SALT_ROUNDS
+);
+
 export interface UserRecord {
   id: string;
   email: string;
@@ -138,6 +153,11 @@ export async function verifyUserCredentials(
 
   const user = await prisma.user.findUnique({ where: { email } });
   if (!user) {
+    // Run a dummy compare against a fixed decoy hash and discard the result
+    // — see DUMMY_PASSWORD_HASH comment above. This keeps timing consistent
+    // with the "email exists, password wrong" branch below instead of
+    // returning early.
+    await bcrypt.compare(password, DUMMY_PASSWORD_HASH);
     return null;
   }
 
diff --git a/tests/auth.test.ts b/tests/auth.test.ts
index 177bbd2..d4b309f 100644
--- a/tests/auth.test.ts
+++ b/tests/auth.test.ts
@@ -115,4 +115,20 @@ describe("verifyUserCredentials", () => {
     expect(await verifyUserCredentials(email, undefined)).toBeNull();
     expect(await verifyUserCredentials(123, password)).toBeNull();
   });
+
+  // Timing side-channel fix: an unknown email must not short-circuit to a
+  // different outcome shape than a known email with the wrong password —
+  // both must be indistinguishable (null) so a caller/attacker can't infer
+  // which case occurred from the result alone. (The actual timing-safety
+  // property — running a dummy bcrypt.compare() on the unknown-email path —
+  // isn't asserted here since timing isn't reliably testable in a unit
+  // test; this only pins down the observable behavior staying identical.)
+  it("returns the same result (null) for an unknown email as for a known email with the wrong password", async () => {
+    const unknownEmailResult = await verifyUserCredentials("nobody@example.com", password);
+    const wrongPasswordResult = await verifyUserCredentials(email, "wrong-password");
+
+    expect(unknownEmailResult).toBeNull();
+    expect(wrongPasswordResult).toBeNull();
+    expect(unknownEmailResult).toBe(wrongPasswordResult);
+  });
 });