diff --git a/apps/server/src/routes/login.ts b/apps/server/src/routes/login.ts index 1126d9a7f..2a505a993 100644 --- a/apps/server/src/routes/login.ts +++ b/apps/server/src/routes/login.ts @@ -1,3 +1,4 @@ +import crypto from "crypto"; import utils from "../services/utils.js"; import optionService from "../services/options.js"; import myScryptService from "../services/encryption/my_scrypt.js"; @@ -160,7 +161,11 @@ function verifyPassword(submittedPassword: string) { const guess_hashed = myScryptService.getVerificationHash(submittedPassword); - return guess_hashed.equals(hashed_password); + // Use constant-time comparison to prevent timing attacks + if (hashed_password.length !== guess_hashed.length) { + return false; + } + return crypto.timingSafeEqual(guess_hashed, hashed_password); } function sendLoginError(req: Request, res: Response, errorType: 'password' | 'totp' = 'password') { diff --git a/apps/server/src/services/encryption/recovery_codes.ts b/apps/server/src/services/encryption/recovery_codes.ts index 898a4cbb4..796290496 100644 --- a/apps/server/src/services/encryption/recovery_codes.ts +++ b/apps/server/src/services/encryption/recovery_codes.ts @@ -56,13 +56,22 @@ function verifyRecoveryCode(recoveryCodeGuess: string) { const recoveryCodes = getRecoveryCodes(); let loginSuccess = false; - recoveryCodes.forEach((recoveryCode) => { + let matchedCode: string | null = null; + + // Check ALL codes to prevent timing attacks - do not short-circuit + for (const recoveryCode of recoveryCodes) { if (constantTimeCompare(recoveryCodeGuess, recoveryCode)) { - removeRecoveryCode(recoveryCode); + matchedCode = recoveryCode; loginSuccess = true; - return; + // Continue checking all codes to maintain constant time } - }); + } + + // Remove the matched code only after checking all codes + if (matchedCode) { + removeRecoveryCode(matchedCode); + } + return loginSuccess; } diff --git a/apps/server/src/services/utils.ts b/apps/server/src/services/utils.ts index 370be6724..370f9297f 100644 --- a/apps/server/src/services/utils.ts +++ b/apps/server/src/services/utils.ts @@ -82,8 +82,14 @@ export function hmac(secret: any, value: any) { * @param a First string to compare * @param b Second string to compare * @returns true if strings are equal, false otherwise + * @note Returns false for null/undefined/non-string inputs. Empty strings are considered equal. */ -export function constantTimeCompare(a: string, b: string): boolean { +export function constantTimeCompare(a: string | null | undefined, b: string | null | undefined): boolean { + // Handle null/undefined/non-string cases safely + if (typeof a !== "string" || typeof b !== "string") { + return false; + } + const bufA = Buffer.from(a, "utf-8"); const bufB = Buffer.from(b, "utf-8");