Skip to content

Commit 98b011d

Browse files
dorlugasigalCopilot
andcommitted
fix(auth): pad to fixed length in safeCompare instead of self-compare
The previous safeCompare did a no-op timingSafeEqual(ab, ab) on length mismatch which doesn't actually equalize cost (same memory, trivially equal). Replace with the standard fixed-length padding pattern: copy both inputs into 256-byte zero-padded buffers, timingSafeEqual on those, and AND with a real length check. Inputs longer than 256 bytes are rejected outright (well beyond any reasonable password). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent 3af53a5 commit 98b011d

1 file changed

Lines changed: 13 additions & 8 deletions

File tree

src/server/auth.js

Lines changed: 13 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -293,18 +293,23 @@ const LOGIN_HTML = `<!DOCTYPE html>
293293
// Constant-time string compare using crypto.timingSafeEqual on raw bytes.
294294
// We intentionally do NOT hash the inputs: this is an equality check against
295295
// an in-memory secret (never stored), and hashing would trip CodeQL's
296-
// js/insufficient-password-hash rule which is aimed at password *storage*.
297-
// When lengths differ we still run timingSafeEqual on the longer buffer
298-
// against itself so the branch cost is roughly constant.
296+
// js/insufficient-password-hash rule which targets password *storage*.
297+
//
298+
// To avoid leaking length via early-return, both inputs are copied into
299+
// fixed-length zero-padded buffers before timingSafeEqual, and the final
300+
// result is AND-ed with a real length check.
301+
const SAFE_COMPARE_LEN = 256;
299302
function safeCompare(a, b) {
300303
if (typeof a !== 'string' || typeof b !== 'string') return false;
301304
const ab = Buffer.from(a, 'utf8');
302305
const bb = Buffer.from(b, 'utf8');
303-
if (ab.length !== bb.length) {
304-
crypto.timingSafeEqual(ab, ab);
305-
return false;
306-
}
307-
return crypto.timingSafeEqual(ab, bb);
306+
if (ab.length > SAFE_COMPARE_LEN || bb.length > SAFE_COMPARE_LEN) return false;
307+
const ap = Buffer.alloc(SAFE_COMPARE_LEN);
308+
const bp = Buffer.alloc(SAFE_COMPARE_LEN);
309+
ab.copy(ap);
310+
bb.copy(bp);
311+
const bytesEqual = crypto.timingSafeEqual(ap, bp);
312+
return bytesEqual && ab.length === bb.length;
308313
}
309314

310315
function createAuth(password) {

0 commit comments

Comments
 (0)