Flow
The order I'd rebuild my fetcher in
1One lookup, both families
Ask once for IPv4 and IPv6, and reject the name if any answer is internal.
2Connect to the checked address
Use that exact address, but keep the real name for the certificate check.
3Redirects by hand
Run the full check again on every hop.
4One deadline, one size limit
Cover the whole response, body included.
5Tests with fake DNS
An answer that changes, a redirect inward, a body that never ends.
6Check the promised firewall
Confirm the egress rules exist, and remember they only limit ports.
A wrapper for outbound requests says it pins DNS. It doesn't. It looks up the hostname, checks that the answer is safe, and then hands the plain URL to fetch. Fetch looks the name up again, and it connects to whatever that second answer says.
I found this by reading the code from the day I moved my scheduling app into a new monorepo. I didn't attack it, and I haven't looked at the live boxes.
The door every request was supposed to use
The wrapper is meant to be the one way out for any request where a user picks the URL. Think webhooks, imports from a link, and sign-in callbacks. If those aren't guarded, a user can make my server call things inside my own network, like an admin port or the cloud metadata address. That attack is called server-side request forgery.
The wrapper checks the IPv4 addresses for the name, blocks private ones, blocks the metadata names, wants HTTPS in production, and sets a timer. Then it calls fetch with the original URL.
Why the check and the connection can disagree
Whoever owns a domain decides what it points to and for how long. They can give a safe public address the first time and an internal one the second time. That's DNS rebinding. Pinning means you connect to the exact address you checked. This code checks one answer and uses another. A cache might save me on some machine, but I'm not counting on luck.
The parts that didn't match the comment
Nothing in that commit calls the wrapper. I couldn't find the lint rule the comment talks about. The webhook code has its own weaker check that skips the metadata range, and then it uses a different library that looks the name up again. The sign-in code skips the wrapper too, though those URLs probably come from my own config. There are no tests.
IPv6, redirects and a timer that stops early
Only IPv4 gets checked, so a name with an IPv6 address could slip past. Redirects are followed and never checked. The timer stops once headers arrive, so a slow body can hang on as long as it likes.
One lookup, one address
If you're building something like this, these are the parts that matter:
- Look the name up once, for both IPv4 and IPv6, and reject it if any answer is internal.
- Connect to the address you checked, but still send the real name so the certificate check works.
- Follow redirects yourself and run the full check on every hop.
- Put one deadline and a size limit on the whole response, body included.
Test it with fake DNS that changes its answer, a redirect that points inward, and a body that never ends.
The backstop I haven't looked at
None of this is built yet. The design doc says a firewall only lets a few outbound ports through. I haven't checked that those rules exist, and a port rule wouldn't stop a request to an internal host on 443 anyway.