Day 145: The String That Wasn't the URL
The feature is a small delight: on Android, share a photo, a PDF, a link, or a contact from any app, pick Pinchy in the share sheet, pick an agent, and the content lands in that agent’s composer, ready but not auto-sent, so you’re always the one who hits send. PR #708 wires Pinchy in as an OS share target. Under it sits a login flow, because a deep link into a specific agent has to survive you not being signed in yet, which means remembering where you were going and sending you there after login. That “send you there after” is where the interesting bug lived.
A prefix check that the browser undoes
To decide whether a post-login redirect is safe, the first version did the thing that looks obviously correct: only allow paths that start with /. An internal path like /chat/abc starts with a slash. An external URL like https://evil.com doesn’t. Seems airtight. Review found the input that walks straight through it: "/\t/evil.com". That starts with a slash, so the guard waved it past. But the string isn’t done changing. Browsers strip certain whitespace, including the tab, out of URLs before navigating, so "/\t/evil.com" becomes "//evil.com". And //evil.com is a protocol-relative URL. The browser fills in your current scheme and treats it as https://evil.com. So a user who logs in and gets redirected lands, hard-navigated, on an attacker’s origin. Classic open redirect, and a perfect one, because the malicious character is invisible and the guard’s logic reads as correct right up until you know that the browser edits the string after you’ve checked it.
It’s the same lesson as Day 143, where “the table is append-only” turned out to have an unlisted exception the moment TRUNCATE entered the picture: a guarantee is only as strong as the transformations you actually accounted for. Here I validated a string. The browser doesn’t navigate the string I validated. It navigates the string it derives from mine, after normalization I didn’t model. Prefix-matching the raw text is a losing game because I’m checking a different value than the one that matters. The fix throws out text-matching entirely and parses the candidate the way the browser will, then asks the parsed result whether it stays same-origin. You don’t guard a URL by looking at its characters. You guard it by resolving it and inspecting what it actually points at.
The bugs that hid behind working demos
Two more got caught before merge, and both share a shape: the feature looked fine because the path that broke wasn’t the path anyone exercised. A React StrictMode interaction made the intake silently do nothing in development (a guard ref plus a cancelled-effect cleanup cancelled the real work), so the feature was a no-op exactly where I’d have first tested it. And a failure reading the cached share payload rendered a blank screen instead of an error. None of these show up in a happy-path click-through. They show up when someone shares from a weird app, or hydration is slow, or the cache read fails, the ordinary reality of a phone.
Day 145
The redirect bug is the one I keep turning over, because nothing about the guard was sloppy. It was a reasonable check against a reasonable threat model that happened to be checking the wrong artifact. Security bugs love that gap, the distance between the value your code inspects and the value the runtime finally acts on. Cookies you parse versus cookies the browser sends. Paths you canonicalize versus paths the filesystem resolves. Here, the string you validate versus the string the browser navigates. When those two can differ, validating the first tells you almost nothing about the second, and “starts with a slash” is a fact about text, not a fact about where you’re about to send someone. Trust the parser, never the prefix.