I run Shorty, an open-source URL shortener I built and self-host. It does the usual things — short links, QR codes, click analytics — plus a moderation console with role-based access, an audit log and abuse triage. The whole thing is TypeScript: Next.js 16 on the front, Express 5 and Drizzle over MySQL/TiDB on the back.
The README claimed, among other things, that "identical error messages prevent account enumeration."
I sat down to audit my own code and found out that sentence was false. Here's what a careful pass over a codebase you wrote yourself actually turns up.
The bug that started it
Here is the entire defect. It is one line, in the admin login handler:
if (!account) {
// Spend comparable time so a missing account is not detectably faster.
await verifyPassword(password, '$2b$12$invalidinvalidinvalidinvalidinvalidinvalidinvalidinvalidin');
throw genericFailure;
}The intent is textbook. When someone tries to log in with an email that doesn't exist, you don't want to return instantly — that reveals the address isn't registered. So you burn the same CPU you would have burned verifying a real password, then return the same generic error.
The comment is right. The code does not do what the comment says.
A bcrypt hash is exactly 60 characters. That literal is 65. bcryptjs validates the shape of the hash before it does any work, and bails immediately when it doesn't parse.
So I measured it:
unknown account (malformed hash): 0.04 ms
real account (genuine hash): 264.73 msThat's a 6,000x difference, from a single unauthenticated request, with no rate limit worth speaking of and no lockout on that path. You don't need statistics or timing analysis. You need a stopwatch. Feed it a list of candidate emails and the ones that come back slowly are real administrators.
The fix is to stop hand-writing the hash and generate one, then assert the thing that actually matters:
export function dummyPasswordHash(): string {
if (dummyHash === undefined) {
dummyHash = bcrypt.hashSync(generateOpaqueToken(32), BCRYPT_ROUNDS);
if (dummyHash.length !== 60) {
throw new Error(`bcrypt returned a ${dummyHash.length}-character hash; the login timing guard is broken`);
}
}
return dummyHash;
}After: 0.98x. Indistinguishable.
There's a second-order lesson in the lazy initialisation. Deriving a cost-12 hash takes ~250 ms, and the module is imported by the redirect path too — so computing it at import time would have added a quarter second to every cold start on a serverless deploy, in order to protect a login endpoint. Security fixes have a blast radius.
The one I'm most annoyed about
Same login handler, a few lines down:
if (account.lockedUntil && account.lockedUntil.getTime() > Date.now()) {
throw new AppError(423, 'ACCOUNT_LOCKED', `Too many failed attempts. Try again in ${minutes} minutes.`);
}Account lockout is a good control. But look at what it returns: a 423 with a countdown, where every other failure returns a generic 401.
I built a careful anti-enumeration story on one path and then handed the answer away on another. Five failed logins against any address, and the status code tells you whether that address belongs to a real admin. The locked branch also returned before doing any bcrypt work, so it was fast as well as differently-shaped.
It now returns the same 401, after the same bcrypt spend, and writes the audit entry that path was silently missing. Legitimate admins lose the "try again in 12 minutes" message. That's the trade, and it's the right one.
When your defence has a formatting dependency
The link-safety guard refuses destinations that point at private networks. It handled
IPv4-mapped IPv6 addresses — the trick where you smuggle 127.0.0.1 inside a v6 literal:
const mapped = normalised.match(/::ffff:(\d+\.\d+\.\d+\.\d+)$/);
if (mapped?.[1]) return isPrivateIpv4(mapped[1]);Reasonable. Except the check runs on a hostname that has already been through the WHATWG URL parser, and that parser rewrites the address:
new URL('https://[::ffff:127.0.0.1]/').hostname
// '[::ffff:7f00:1]'The dotted quad becomes hex. The regex requires dots. It never matched a single real request.
So [::ffff:a9fe:a9fe] — the cloud metadata endpoint, 169.254.169.254, wearing an IPv6
hat — sailed straight through. Along with five other forms.
The fix was to stop parsing addresses by hand:
const PRIVATE_RANGES = new BlockList();
PRIVATE_RANGES.addSubnet('127.0.0.0', 8, 'ipv4');
PRIVATE_RANGES.addSubnet('169.254.0.0', 16, 'ipv4');
// ...
// 'ipv6' is deliberate even for an IPv4-mapped address: BlockList maps it
// back onto the IPv4 entries above, which is what closes the bypass.
if (ipVersion === 6) return PRIVATE_RANGES.check(host, 'ipv6');Node's built-in net.BlockList resolves IPv4-mapped addresses against the IPv4 rules
natively. Six bypasses closed, and — verified — nothing legitimate over-blocked.
The pattern worth taking away: my check ran on a different string than the one I wrote it against. Anywhere a value passes through a normaliser between validation and use, that's where the bug lives.
Controls that report success while doing nothing
Three findings shared a shape, and it's the shape I now look for first.
"Revoke sessions" didn't revoke sessions. The emergency force-sign-out button
incremented a tokenVersion column, which kills outstanding access tokens. But the refresh
endpoint never compared sessions against that version — so a stolen refresh token could be
exchanged straight back for a fresh access token carrying the new version. The button
returned {revoked: true}, wrote a satisfying audit entry, and changed nothing an attacker
would notice. For up to seven days.
Blocking a domain didn't block existing links. The blocklist had exactly one reader:
the create-link path. Block evil.com and every link already pointing there kept
redirecting, indefinitely, while the console displayed the domain as blocked. The
moderation control worked perfectly for the case where nobody had attacked you yet.
Rate limiting three routes that had none. In Express, router.use() only runs for
requests that reach it in stack order:
adminRouter.post('/auth/password', requireAdmin, asyncHandler(changePassword));
// ... more routes ...
adminRouter.use(requireAdmin, adminApiLimiter); // never runs for anything above/auth/password runs two bcrypt operations per call — roughly 600 ms of single-threaded
CPU — and any token holder, down to the lowest role, could loop it with a wrong password
and stall the process that also serves every public redirect.
A control that fails loudly gets fixed. A control that succeeds quietly while doing nothing can sit there for a year.
The rest, briefly
- The login rate limiter was keyed on attacker-controlled input (
ip:email). Vary the email, get unlimited fresh buckets. It also never trimmed the email while the database lookup did — so"admin@x.com"and"admin@x.com "were separate budgets hitting the same row, which is enough to hold any known admin in a permanent lockout. - A legacy compatibility endpoint bypassed every per-action limiter, allowing ~80x the intended abuse-report rate. Enough for a handful of hosts to auto-block arbitrary links with no human involved.
/healthechoed raw driver errors to anonymous callers — database hostname, port, andAccess denied for user 'x'@'y'.- The admin console had no CSRF protection. The session cookies were
SameSite=Lax, which I had filed as "handled". Lax is a same-site control, and site means registrable domain — which my API subdomain shares with the web app. Any page on any sibling subdomain could drive every admin write with the operator's cookies attached. - The blocklist failed open. A database error returned an empty list, so on a cold serverless process whose first round-trip timed out, every blocked domain was accepted — and permanently minted, because nothing re-checked existing links.
Ten findings. None of them were exotic. All of them were in code I wrote, reviewed, and shipped.
What I actually changed about how I work
Every security claim in a README is a test that doesn't exist yet. I wrote "prevents account enumeration" and then never wrote the assertion. The claim had been false since the day it was committed. There are now 27 tests, and every bypass above is pinned as a regression case — because the SSRF guard in particular fails silently. A hole in it looks exactly like a working shortener right up until someone points a link at your metadata endpoint.
Comments describe intent, not behaviour. // Spend comparable time was accurate about
what I meant and wrong about what ran. When reviewing, read the code as if the comment
isn't there — then check whether they agree.
Prefer the boring standard-library primitive. My hand-rolled IP range checks had a
subtle formatting dependency. net.BlockList has been correct for years and handles a
case I didn't know existed.
Check whether your control has ever fired. "Does this code run?" caught three findings. For any security control, trace one real request through it and confirm the guard is on the path. Middleware ordering, cache population, retroactive application — these are where things quietly do nothing.
Audit the seams. Nearly every finding lived at a boundary: between a parser and a validator, between route registration and middleware, between two subdomains that a browser considers the same site. The code inside each module was fine. The bugs were in the joins.
Closing
Everything above is fixed and shipped in v3.0.1. The full changelog has the per-file detail, and the code is on GitHub if you want to check my work — genuinely, please do.
Two things I'd suggest if you're about to do this to your own project. Run your git history through a secret scanner before anything else; rotating a credential is cheap, and finding out later that it's been public for two years is not. And write the audit down as you go, findings and non-findings both. Half the value turned out to be the list of things I checked and didn't find, because that's the list I don't have to check again.
The most uncomfortable part wasn't any individual bug. It was that I'd written a security section in the README describing the system I intended to build, and never gone back to verify I'd built it.
