Security

Fetching a URL the user gave you

One of our services accepts a link, downloads it and extracts the content. It is a small feature with a large attack surface: you are asking a server inside your network to make a request chosen by a stranger. Here are three problems we found in our own implementation, in the order they would have bitten us.

1. The redirect chain is the vulnerability

Our first version validated the submitted URL properly – scheme, hostname, resolved address, private ranges rejected – and then handed it to the HTTP client, which followed redirects by itself. That is the bug. Validation applied to hop zero; hops one through five were whatever the remote server felt like returning. A public URL that 302s to http://169.254.169.254/ or http://127.0.0.1:8000/ walks straight past a check that only ever looked at the first address.

The fix is to stop delegating: disable automatic redirects, follow them yourself, and run the full validation on every hop.

for hop in range(MAX_REDIRECTS):
    validate(url)                    # scheme, host, resolved IP, ranges
    resp = await client.get(url, follow_redirects=False)
    if not resp.is_redirect:
        return resp
    url = urljoin(url, resp.headers["location"])
raise TooManyRedirects

Two details matter beyond the loop. Validate the resolved address, not the hostname – a name you do not control can point anywhere, and blocklisting strings like localhost catches nobody serious. And be honest that a resolve-then-connect gap leaves a DNS rebinding window; closing it properly means pinning the connection to the address you checked.

VALIDATE ONCE – THE CHAIN ESCAPES https://example.com/a 302 → short.link/b 302 → 169.254.169.254 checked ✓ not checked not checked – metadata service reached FOLLOW MANUALLY – VALIDATE EVERY HOP https://example.com/a 302 → short.link/b 302 → 169.254.169.254 resolve + check ✓ resolve + check ✓ link-local range → refused, chain stops
The check has to run on the address you are about to connect to – every time, not once.

2. Comparing a webhook secret with ==

Unrelated code path, same review. We authenticated an inbound webhook by comparing the received secret to the expected one with an ordinary string comparison, which returns as soon as it finds a differing byte. Timing differences leak the prefix, and an attacker with patience recovers the secret one byte at a time.

The correct version compares in constant time, over bytes rather than text so that Unicode normalisation cannot produce a surprising match:

hmac.compare_digest(received.encode(), expected.encode())

This is a two-line fix that nobody notices until someone looks. Grep your codebase for == secret and == token; there is usually one.

3. A timeout that was not a deadline

Our client had a timeout, so we assumed we were covered. We were not: the timeout applied per socket operation, not to the request as a whole. A server that trickles one byte every few seconds keeps a worker occupied indefinitely without ever tripping it. Add a redirect chain and you have a cheap way to exhaust a connection pool.

What we needed was a total budget for the whole operation – every hop, every read – plus a cap on the response size, enforced while streaming rather than after. Related, and found in the same pass: the parsing step was synchronous and ran on the event loop, so a large document blocked every other request in the process. It now runs in a worker thread.

The takeaway

Every one of these was a case of trusting a library's default to mean what we assumed. Redirects are followed unless you say otherwise; comparison is fast unless you ask for constant time; a timeout is per-operation unless you make it a deadline. None of these defaults are wrong – they are just not security decisions, and we were treating them as if they were.
← All notes