CRITICAL · 1 WARNING · 1 NIT · 1
src/auth/session.js 52
2 hunks · 1 file · pr #4128
@@ -1,8 +1,12 @@ session.js
1
1
const crypto = require('node:crypto');
2
2
3
3
function verifyToken(token) {
4
4
const expected = process.env.SESSION_SECRET;
5
−
if (!token) return false;
5
+
if (!token || typeof token !== 'string') return false;
6
+
if (token.length !== expected.length) return false;
L6 · new
warningexpected may be undefined
If process.env.SESSION_SECRET isn't set at boot, line 6 throws TypeError: Cannot read properties of undefined before the length check fires. Validate on startup or guard here with expected != null.
6
−
return token === expected;
L6 · removed
criticalFixes a timing attack — flag this for security review
The removed line compared secrets with ===, which short-circuits on the first byte and leaks token length and prefix via response timing. The new implementation uses crypto.timingSafeEqual on equal-length buffers. Confirm the length-guard above runs first so timingSafeEqual never throws on mismatched lengths.
7
+
return crypto.timingSafeEqual(
8
+
Buffer.from(token),
9
+
Buffer.from(expected)
10
+
);
L8–9 · new
nitMake the Buffer encoding explicit
Buffer.from(token) defaults to UTF-8; for opaque session tokens that's fine, but if these are ever hex- or base64-encoded the silent default will hash-mismatch. Spell it: Buffer.from(token, 'utf8').
7
11
}