Forged cookies should fail quietly
A forged admin cookie could crash the request instead of failing it. The bug lived in the difference between string length and byte length.
A tampered admin cookie is supposed to be the most boring input my server sees: verify the signature, get null, serve the login page, done. Yesterday, rereading the session code, I found a class of forgery that would skip boring and go straight to a 500.
The session cookie is a signed token: a base64url payload plus an HMAC signature, and verification recomputes the signature and compares with crypto.timingSafeEqual. That function has a sharp edge: handed two buffers of different lengths, it does not return false. It throws a RangeError. So you guard the lengths first, and my guard checked sig.length against expected.length, string against string.
JavaScript string length counts UTF-16 characters, not bytes. The expected signature is base64url, one byte per character. A forged cookie is attacker-controlled text, and a signature containing multi-byte characters can match the expected string character-for-character in length while encoding to a longer buffer. Equal string lengths, the guard passes, Buffer.from produces unequal buffers, timingSafeEqual throws, nothing catches it, and a garbage cookie takes down the request.
The fix is four honest lines: build both buffers first, compare their byte lengths, then run the constant-time comparison.
const sigBuf = Buffer.from(sig);
const expectedBuf = Buffer.from(expected);
if (sigBuf.length !== expectedBuf.length) return null;
if (!crypto.timingSafeEqual(sigBuf, expectedBuf)) return null;Severity, honestly: this was not a bypass. The forged request still failed. But there is a real difference between a rejection and an error, and auth code should never let hostile input cross from one to the other. An error is a free noise generator for whoever sends the cookie: monitoring reads crash where it should read forgery, error paths get retried and investigated, and a 500 tells an attacker their input reached deeper than a 401 would. Rejections should be cheap, silent, and identical. Errors are for my bugs, not their inputs.
The reason this class of bug survives is that correct traffic cannot find it. Every legitimate cookie, and every crude forgery that stays in ASCII, behaves identically under the broken guard and the fixed one. The divergence only exists for input a well-meaning test would never think to send, which means test suites built from real sessions are structurally blind to it. The tests that catch it are the rude ones: feed the verifier garbage, multi-byte garbage, truncated garbage, and assert that every response is the same quiet null.
The embarrassing detail is that the correct pattern already existed in this codebase. The share password gate verifies its grant cookies byte-first; the session path predates it and never got revisited when the better version appeared. That is the general failure: the second implementation of a pattern is where the first one's bug becomes visible, and nothing forces you to look back. The whole audit, once I knew what to look for, was a grep for timingSafeEqual.
One anticipated objection: short-circuiting on length before a constant-time comparison is fine here. Constant-time comparison exists to keep the content of the secret from leaking through timing. The length of an HMAC-SHA256 signature is public knowledge, every well-formed cookie has the same one, and only forgeries differ. Guarding it early leaks nothing an attacker does not already have.
Hostile input deserves the most boring response available: null. Anything more expressive is a donation.