Repository navigation
Conversation
…tches its spellings Three groups of defects in check_url and allow_private_hosts from jordan-gibbs#53. Lookup name. check_url resolved the hostname as urlparse returns it, and socket.getaddrinfo encodes a non-ASCII name with the stdlib "idna" codec, which is IDNA 2003. httpx encodes the same host with IDNA 2008 before it connects. The two disagree on the IDNA 2008 deviation characters (ß, ς, ZWJ, ZWNJ): for "http://straße.attacker.example/" the gate asked the resolver for "strasse.attacker.example", while httpx connects to "xn--strae-oqa.attacker.example". Those are separate DNS names, so whoever controls the zone can answer the first with a public address and the second with a private one, and the gate passes a URL whose connection reaches the private address. The precondition is the one the module docstring already accepts for DNS rebinding (control of the zone), but nothing depends on timing. check_url now resolves httpx.URL(url).raw_host, the ASCII name httpx puts on the wire, and matches the allowlist against that name. The stdlib codec passes an ASCII name through unchanged, so the gate and the connection look up the same string. A URL httpx cannot parse or encode raises SafeHTTPError ("refusing unparseable URL") instead of httpx.InvalidURL or UnicodeEncodeError: an invalid IDNA label, for one, or a lone surrogate in the path, query or fragment, which is what a non-UTF-8 byte in a command-line argument becomes (the old gate let that one through to the fetch). Every other gate path goes through check_url: safe_get on each hop (it already rebuilt redirect targets through httpx.URL, so only the entry URL was affected there), the browser lane's entry and final-URL checks, the batch fetch, and check_oa_url. Hostname spellings. Allowlist hostnames were compared to the URL host as lowercased strings, so spellings of the same host missed their entry and the user's own mirror was refused as an SSRF target: - a trailing root dot on either side ("mirror.internal." in the URL, "mirror.internal" in the config, or the reverse), the same host whenever the resolver looks the name up as absolute; - an IDN host written as unicode on one side and punycode on the other ("münchen.example" vs "xn--mnchen-3ya.example"); - an entry containing '%' that is not a scoped IPv6 literal ("my%host") was accepted silently as a hostname that can never match; - so was an entry httpx refuses as a URL host (a fullwidth letter, a name not in NFC, a soft hyphen, a zero-width space). The URL's name (above) and each entry now go through one normalization: lowercase, a non-ASCII name through idna.encode, then strip the trailing dot. idna.encode is the same IDNA 2008 encoder httpx applies to a URL host (it also splits labels on the full stops U+3002, U+FF0E and U+FF61), so the gate encodes an entry into the name httpx would send. idna is now a declared dependency: pyproject.toml lists it unpinned, as httpx itself declares it. The stdlib "idna" codec is deliberately not used: it is IDNA 2003 and maps "straße" to "strasse", a different, separately registrable name from the "xn--strae-oqa" httpx connects to, so matching through it would let an entry for one admit the other. An entry that idna.encode rejects, or with a '%' and not parsed as an address, raises SafeHTTPError naming the entry. Scoped entries already parse as networks, "fe80::1%eth0" through ipaddress itself and "[fe80::1%eth0]" since jordan-gibbs#127 strips the brackets; tests here pin both, unchanged. Exception containment. The batch fetch path skips a refused URL by catching SafeHTTPError, so any other exception out of check_url fails every URL in the batch. Two inputs did that: - check_url("http://[::1/x") raised ValueError("Invalid IPv6 URL") from urlparse; - check_url("http://" + "a" * 300 + ".com/x") raised UnicodeError from getaddrinfo, which IDNA-encodes the name before any lookup and is not a gaierror. check_url now converts the first to SafeHTTPError ("refusing unparseable URL"), and _resolve catches UnicodeError and OSError (gaierror's base) around getaddrinfo. check_oa_url already caught an OSError escaping check_url; that error now arrives as SafeHTTPError, so its refusal reason reads "DNS lookup failed for ..." instead of "DNS resolution failed: ...", and test_rejects_unresolvable_host asserts the new text. Tests: 26 new cases in tests/test_web/test_safe_http.py plus one end-to-end batch test in tests/test_web/test_fetch_many_gate.py (a malformed URL and a URL with a lone surrogate are logged and skipped, the public URL after them is still fetched). The lookup-name and allowlist-spelling tests stub getaddrinfo: the first record the name it receives, the second answer with a private address, so a spelling that misses its entry is refused rather than resolved for real. With the safe_http.py change reverted, 22 fail: the three lookup-name cases (straße, ς, a label with a ZWJ), the safe_get test comparing each hop's lookup with the host its request goes to, all five spelling cases, the U+3002 entry, the '%' entry error, the four entries httpx refuses, the three unencodable URLs, both containment tests, the batch test, and the updated check_oa_url test. The two "straße"/"strasse" allowlist cases pass against the old exact-match code; routing the normalization through the stdlib IDNA 2003 codec instead fails one of them, entry "straße.example" with URL "strasse.example". The four scoped-IPv6 cases pass either way. Full suite: 1746 passed on 3.11, 3.12, 3.13 and 3.14; ruff check src/ tests/ is clean. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
urlparse drops leading C0 controls and spaces (the WHATWG rule); httpx reads " http://8.8.8.8/" as a relative URL with no host. Since the gate now checks httpx's host, such a URL was refused with "DNS lookup failed for ''", where it was accepted before. check_url strips the same characters before parsing with httpx, and refuses a URL httpx still finds no host in as "no hostname" rather than looking up an empty name. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TZrbJKxzwsFkWndioJB7q6
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What this changes
check_urlnow resolves the hostname httpx will connect to (httpx.URL(url).raw_host) instead of urlparse's spelling of it.allow_private_hostsmatches hostnames across spellings of the same name and rejects entries that can never match, andcheck_urlraises onlySafeHTTPError, so one bad URL no longer fails a whole batch.Why
Three groups of defects in the SSRF gate from #53.
Lookup name.
check_urlresolved urlparse's hostname, andsocket.getaddrinfoencodes a non-ASCII name with the stdlibidnacodec (IDNA 2003), while httpx connects to the IDNA 2008 name. The two differ for the deviation characters ß, ς, ZWJ and ZWNJ: forhttp://straße.attacker.example/the gate looked upstrasse.attacker.exampleand httpx connects toxn--strae-oqa.attacker.example, two separate DNS names. Whoever controls the zone can point the first at a public address and the second at a private one, and the gate passes the URL. That is the precondition the module docstring already accepts for DNS rebinding (control of the zone), but nothing depends on timing.The gate now resolves
raw_host, the ASCII name httpx puts on the wire. The stdlib codec passes an ASCII name through unchanged, so the gate and the connection look up the same string. A URL httpx cannot parse or encode (an invalid IDNA label, or a lone surrogate, which is what a non-UTF-8 byte in a command-line argument becomes) is refused as unparseable. Every other gate path (safe_gethops, the browser lane's entry and final-URL checks, the batch fetch,check_oa_url) goes throughcheck_url.Hostname spellings. Allowlist entries were compared to the URL host as lowercased strings, so a trailing root dot (
mirror.internal.vsmirror.internal, the same host whenever the resolver looks the name up as absolute) or unicode vs punycode (münchen.examplevsxn--mnchen-3ya.example) missed the entry, and the user's own mirror was refused. An entry with a stray%(my%host), or one httpx refuses as a URL host (a fullwidth letter, a name not in NFC, a soft hyphen, a zero-width space), was accepted silently and could never match.Both sides now go through one normalization: lowercase, a non-ASCII name through
idna.encode, then strip the trailing dot.idna.encodeis the same IDNA 2008 encoder httpx applies to a URL host, now a declared dependency (unpinned, as httpx declares it). The stdlib codec is not used because it mapsstraßetostrasse, a separately registrable name. An entryidna.encoderejects, or a%entry that is not an address, raisesSafeHTTPErrornaming the entry.Exception containment. The batch fetch skips a refused URL by catching
SafeHTTPError, so anything else out ofcheck_urlfails the whole batch.check_url("http://[::1/x")raisedValueErrorfrom urlparse, and a 300-character label raisedUnicodeErrorfromgetaddrinfo. Both now becomeSafeHTTPError.Side effect: for a resolver
OSErrorother thangaierror,check_oa_url's refusal reason now readsDNS lookup failed for ...instead ofDNS resolution failed: ...;test_rejects_unresolvable_hostasserts the new text.How it was verified
tests/test_web/test_safe_http.pyand one batch test intests/test_web/test_fetch_many_gate.py; no new test sends a DNS query.safe_http.pychange reverted, 22 tests fail; routing the normalization through the stdlib codec fails the case with entrystraße.exampleand URLstrasse.example.getaddrinfo("straße.attacker.example")asking forstrasse.attacker.example; a Playwright Chromium 153 build asks for the same IDNA 2008 names as httpx for all four deviation characters.pytest tests/: 1746 passed on 3.11, 3.12, 3.13 and 3.14.ruff check src/ tests/is clean.Scope check — the review is much faster when the diff contains only the change being proposed:
CHANGELOG.mdtouched only to add an entry under[Unreleased]