fix: run dummy bcrypt.compare on unknown user to prevent timing side-channel
This commit is contained in:
@@ -20,6 +20,21 @@ const BCRYPT_SALT_ROUNDS = 10;
|
|||||||
// grows into employee self-service.
|
// grows into employee self-service.
|
||||||
const MIN_PASSWORD_LENGTH = 8;
|
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 {
|
export interface UserRecord {
|
||||||
id: string;
|
id: string;
|
||||||
email: string;
|
email: string;
|
||||||
@@ -138,6 +153,11 @@ export async function verifyUserCredentials(
|
|||||||
|
|
||||||
const user = await prisma.user.findUnique({ where: { email } });
|
const user = await prisma.user.findUnique({ where: { email } });
|
||||||
if (!user) {
|
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;
|
return null;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
@@ -115,4 +115,20 @@ describe("verifyUserCredentials", () => {
|
|||||||
expect(await verifyUserCredentials(email, undefined)).toBeNull();
|
expect(await verifyUserCredentials(email, undefined)).toBeNull();
|
||||||
expect(await verifyUserCredentials(123, password)).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("[email protected]", password);
|
||||||
|
const wrongPasswordResult = await verifyUserCredentials(email, "wrong-password");
|
||||||
|
|
||||||
|
expect(unknownEmailResult).toBeNull();
|
||||||
|
expect(wrongPasswordResult).toBeNull();
|
||||||
|
expect(unknownEmailResult).toBe(wrongPasswordResult);
|
||||||
|
});
|
||||||
});
|
});
|
||||||
|
|||||||
Reference in New Issue
Block a user