forked from bchanot/claude
feat(seo-data): safe_fetch — resolve-then-pin, close DNS-rebinding + SSRF
By-principle hardening. H1's url-guard validates the NAME; urlopen then resolved AND connected — two DNS lookups with a window a hostile authority uses to answer PUBLIC to validation and PRIVATE (169.254.169.254 metadata, 127.0.0.1, the LAN) to the connect. A name-level guard cannot see that rebind. safe_fetch collapses the two lookups into one: resolve ONCE, validate every IP (ipaddress, dual-stack v4+v6), refuse if ANY is non-public (the multi-A vector), connect to the exact validated IP with Host+SNI+cert for the real host — no second resolution to poison. Redirects re-validate each hop (urlopen followed them blind). One seam: sitemap._fetch, which linkgraph/render_check/drift all call, so every network verb inherits it. The load-bearing property (confirmed by the security review): classification is on the OS-resolved address (sockaddr[0]), never the URL text — so octal/hex/ decimal literals, IPv4-mapped IPv6, NAT64, 6to4 are all defeated structurally, not by enumeration. Better than the source idea (claude-seo url_safety.py, MIT): dual-stack (theirs IPv4-only), no global monkeypatch so thread-safe by construction (theirs locks a patched getaddrinfo), stdlib-only (no requests). Proven end-to-end before writing: pinned connect keeps SNI+cert for the real host. NOT covered, stated not silent: shell `curl` in the agent specs (separate process, unpinnable here). Smaller surface; `curl --resolve` is a separate change. REVIEW-SURFACED (fresh security-auditor, adversarial, VERDICT PASS) — two real holes it found while attacking the diff, both fixed here: - billion-laughs REOPENED in C1b: _refuse_dtd scanned only raw[:4096], so a >4KB leading comment pushed <!DOCTYPE past the window while ET parsed AND EXPANDED the entities. Proven (&lol2; → "lollollollollol"), now a full-doc case-insensitive scan. This is a genuine fix to already-merged C1b, not this feature — fixed here rather than filed, per root-cause discipline. - 192.88.99.0/24 (6to4-relay anycast) passed is_global as public — added to an extra special-use deny list. Verified: rebind-to-metadata refused BEFORE any connect (injected resolver), multi-A public+private refused, classifier fuzzed dual-stack incl. CGNAT/6to4, non-http scheme refused, both review fixes proven with no false positive; real fetch still works (zenquality 86 loc, lavageangels 24) through the pinned path; all 4 verbs work end-to-end via fetch.sh; seo-data 210 → 221 pass, 0 fail; full suite green; shellcheck + py_compile clean.
This commit is contained in:
+13
-6
@@ -29,10 +29,11 @@ def _mock(name):
|
||||
return f.read()
|
||||
|
||||
def _fetch(url):
|
||||
from urllib.request import urlopen, Request # stdlib, lazy
|
||||
req = Request(url, headers={"User-Agent": "claude-seo-data/1.0"})
|
||||
with urlopen(req, timeout=TIMEOUT) as r: # nosec: audited target
|
||||
raw = r.read(20 * 1024 * 1024) # 20 MB ceiling
|
||||
# SSRF + DNS-rebinding safe: resolve-then-pin, redirects re-validated.
|
||||
# This is the single seam for ALL network egress — linkgraph/render_check/
|
||||
# drift all call sitemap._fetch — so pinning here covers every verb.
|
||||
import safe_fetch # sibling, lazy
|
||||
raw = safe_fetch.safe_fetch(url, timeout=TIMEOUT, max_bytes=20 * 1024 * 1024)
|
||||
if raw[:2] == b"\x1f\x8b": # sitemap.xml.gz is common
|
||||
raw = gzip.decompress(raw)
|
||||
return raw
|
||||
@@ -53,8 +54,14 @@ def _refuse_dtd(raw):
|
||||
google_seo.py's mock/degrade paths. A sitemap with a DTD is not a sitemap
|
||||
we want anyway.
|
||||
"""
|
||||
head = raw[:4096].lstrip()[:2048].upper()
|
||||
if b"<!DOCTYPE" in head or b"<!ENTITY" in head:
|
||||
# Scan the WHOLE document, not a prefix. A security review (2026-07-17)
|
||||
# showed a >4 KB leading comment pushed <!DOCTYPE past the old raw[:4096]
|
||||
# window while ET.fromstring still parsed and EXPANDED the entities —
|
||||
# billion-laughs reopened. A legitimate sitemap contains neither construct
|
||||
# anywhere, so a full case-insensitive scan is correct; over ≤20 MB it is a
|
||||
# single re.search, microseconds, no 20 MB uppercased copy.
|
||||
import re # stdlib, lazy
|
||||
if re.search(rb"(?i)<!\s*(DOCTYPE|ENTITY)", raw):
|
||||
raise UnsafeXML("DTD in sitemap")
|
||||
|
||||
SITEMAP_NS = "{http://www.sitemaps.org/schemas/sitemap/0.9}"
|
||||
|
||||
Reference in New Issue
Block a user