fix(otp): compare OTP codes in constant time

src/otp.test.ts

@@ -1,5 +1,5 @@
 import { describe, it, expect, vi } from "vitest";
-import { createOTP } from "./otp";
+import { constantTimeEqual, createOTP } from "./otp";
 
 describe("HOTP and TOTP Generation Tests", () => {
 	it("should generate a valid HOTP for a given counter", async () => {
@@ -88,6 +88,13 @@ describe("HOTP and TOTP Generation Tests", () => {
 		expect(isValid).toBe(false);
 	});
 
+	it("should verify with a window greater than 1", async () => {
+		const secret = "1234567890";
+		const totp = await createOTP(secret).totp();
+		const isValid = await createOTP(secret).verify(totp, { window: 2 });
+		expect(isValid).toBe(true);
+	});
+
 	it("should generate a valid QR code URL", () => {
 		const secret = "1234567890";
 		const issuer = "my-site.com";
@@ -97,3 +104,57 @@ describe("HOTP and TOTP Generation Tests", () => {
 		expect(url).toContain("otpauth://totp");
 	});
 });
+
+describe("constant-time OTP comparison", () => {
+	it("returns true for equal strings and false for differing ones", () => {
+		expect(constantTimeEqual("123456", "123456")).toBe(true);
+		expect(constantTimeEqual("123456", "123457")).toBe(false);
+		expect(constantTimeEqual("123456", "923456")).toBe(false);
+		expect(constantTimeEqual("", "")).toBe(true);
+	});
+
+	it("returns false for strings of different lengths", () => {
+		expect(constantTimeEqual("123456", "1234567")).toBe(false);
+		expect(constantTimeEqual("1234567", "123456")).toBe(false);
+		expect(constantTimeEqual("123456", "")).toBe(false);
+		expect(constantTimeEqual("", "123456")).toBe(false);
+		// A correct prefix that is shorter must not be accepted, which would
+		// be the failure mode of a length-leaking early return.
+		expect(constantTimeEqual("123", "123456")).toBe(false);
+	});
+
+	it("handles multi-byte unicode without early-return mismatches", () => {
+		expect(constantTimeEqual("café", "café")).toBe(true);
+		expect(constantTimeEqual("café", "cafe")).toBe(false);
+	});
+
+	it("does not short-circuit on the matching-prefix length", () => {
+		// Regression for the timing side channel: comparison time must not
+		// correlate with how many leading characters match. We measure the
+		// mean comparison time for a candidate that mismatches at the first
+		// character versus one that matches all but the last character; a
+		// short-circuiting `===` makes the late-mismatch case measurably
+		// slower, while a constant-time compare keeps them indistinguishable.
+		const target = "9".repeat(4096);
+		const earlyMismatch = "0" + "9".repeat(4095);
+		const lateMismatch = "9".repeat(4095) + "0";
+
+		const time = (candidate: string) => {
+			const iterations = 2000;
+			// Warm up to reduce JIT noise before timing.
+			for (let i = 0; i < 200; i++) constantTimeEqual(candidate, target);
+			const start = performance.now();
+			for (let i = 0; i < iterations; i++) {
+				constantTimeEqual(candidate, target);
+			}
+			return (performance.now() - start) / iterations;
+		};
+
+		const early = time(earlyMismatch);
+		const late = time(lateMismatch);
+		// Both inputs are full length and fully scanned, so timings should be
+		// within the same order of magnitude regardless of mismatch position.
+		const ratio = Math.max(early, late) / Math.max(Math.min(early, late), 1e-9);
+		expect(ratio).toBeLessThan(5);
+	});
+});

src/otp.ts

@@ -5,6 +5,29 @@ import type { SHAFamily } from "./type";
 const defaultPeriod = 30;
 const defaultDigits = 6;
 
+/**
+ * Compares two strings in constant time relative to the length of `b`.
+ *
+ * Unlike `===`, this does not short-circuit on the first differing byte, so
+ * the comparison time does not leak the length of the matching prefix. This
+ * matters because one operand is a secret OTP derived from the shared secret
+ * and the other is attacker-controlled.
+ */
+export function constantTimeEqual(a: string, b: string) {
+	const aBytes = new TextEncoder().encode(a);
+	const bBytes = new TextEncoder().encode(b);
+	// Fold the length difference into the accumulator so a mismatched length
+	// can never produce a `0` result, while still iterating over every byte.
+	let result = aBytes.length ^ bBytes.length;
+	for (let i = 0; i < bBytes.length; i++) {
+		// `aBytes[i]` reads `undefined` past the end of `a`; `undefined ^ x`
+		// coerces to `0 ^ x = x`, so a length mismatch is still recorded by
+		// the accumulator above without an early return.
+		result |= (aBytes[i] as number) ^ bBytes[i];
+	}
+	return result === 0;
+}
+
 async function generateHOTP(
 	secret: string,
 	{
@@ -66,16 +89,17 @@ async function verifyTOTP(
 ) {
 	const milliseconds = period * 1000;
 	const counter = Math.floor(Date.now() / milliseconds);
+	let valid = false;
 	for (let i = -window; i <= window; i++) {
 		const generatedOTP = await generateHOTP(secret, {
 			counter: counter + i,
 			digits,
 		});
-		if (otp === generatedOTP) {
-			return true;
-		}
+		// Compare in constant time and avoid an early return so neither the
+		// matching-prefix length nor which window candidate matched is leaked.
+		valid = constantTimeEqual(otp, generatedOTP) || valid;
 	}
-	return false;
+	return valid;
 }
 
 /**