From 2837d25259a69f02295b8aab15c298ddbdd7aaa4 Mon Sep 17 00:00:00 2001 From: teknium1 <127238744+teknium1@users.noreply.github.com> Date: Wed, 16 Sep 2026 12:13:54 -0700 Subject: [PATCH] fix(url_safety): refuse fake-ip declarations that overlap reserved address classes A declared `security.fake_ip_ranges` block is trusted like `allow_private_urls` for whatever it names, and `_resolve_fake_ip_ranges` accepted the entries verbatim. Declaring 10.0.0.0/8, 127.0.0.0/8, 100.64.0.0/10, fc00::/7, or a catch-all 0.0.0.0/0 / ::/0 therefore made loopback, RFC 1918, CGNAT and ULA hosts dialable at pre-flight and at TCP connect, while the docs promised those classes "stay blocked". Only the link-local/metadata floor actually held. A local proxy owns none of those classes, so an entry overlapping one of them (or the unspecified address, which is how the catch-alls are caught) is now dropped with a warning instead of widening the guard; the other declared entries keep working. The docs sentence now says so. The browser hybrid-routing oracle `_url_is_private` classified the declared sentinel as private via `ipaddress.is_private` without consulting the declaration, so with a cloud browser provider and `browser.auto_local_for_private_urls` (default on) every URL on a fake-ip host was routed to the local sidecar. The sentinel is not private for routing either: the name is public and the cloud browser resolves it itself. --- tests/tools/test_url_safety.py | 28 ++++++++++++++++++++++++++++ tools/browser_tool.py | 8 +++++++- tools/url_safety.py | 18 +++++++++++++++++- website/docs/user-guide/security.md | 5 ++++- 4 files changed, 56 insertions(+), 3 deletions(-) diff --git a/tests/tools/test_url_safety.py b/tests/tools/test_url_safety.py index 1630452c01..62b684d535 100644 --- a/tests/tools/test_url_safety.py +++ b/tests/tools/test_url_safety.py @@ -534,3 +534,31 @@ class TestDeclaredFakeIpSentinelRanges: _reset_allow_private_cache() with patch("hermes_cli.config.read_raw_config", lambda: {}), _resolves_to("198.18.0.23"): assert is_safe_url("https://example.com/file.jpg") is False + + @pytest.mark.parametrize( + ("declared", "ip"), + [ + (["10.0.0.0/8"], "10.0.0.5"), # RFC 1918 + (["127.0.0.0/8"], "127.0.0.1"), # loopback + (["100.64.0.0/10"], "100.64.0.1"), # CGNAT + (["fc00::/7"], "fd00::1"), # ULA + (["0.0.0.0/0"], "192.168.1.1"), # catch-all overlaps the unspecified address + (["::/0"], "::1"), # v6 catch-all overlaps the unspecified address + ], + ) + def test_declaration_cannot_excuse_reserved_classes(self, monkeypatch, declared, ip): + # A declared block is trusted like allow_private_urls, so it must not be able to name + # loopback/RFC 1918/CGNAT/ULA/unspecified space — those classes stay blocked no matter + # what the config says; the overlapping entry is dropped, it does not widen the guard. + monkeypatch.setattr( + "hermes_cli.config.read_raw_config", + lambda: {"security": {"fake_ip_ranges": declared + ["198.18.0.0/15"]}}, + ) + _reset_allow_private_cache() + try: + with _resolves_to(ip): + assert is_safe_url("https://example.com/") is False + with _resolves_to("198.18.1.125"): # the legitimate sibling declaration still works + assert is_safe_url("https://example.com/") is True + finally: + _reset_allow_private_cache() diff --git a/tools/browser_tool.py b/tools/browser_tool.py index b8576e4f61..16c4048600 100644 --- a/tools/browser_tool.py +++ b/tools/browser_tool.py @@ -67,11 +67,13 @@ except Exception: try: from tools.url_safety import ( + _is_declared_fake_ip, is_safe_url as _is_safe_url, is_always_blocked_url as _is_always_blocked_url, normalize_url_for_request as _normalize_url_for_request, ) except Exception: + _is_declared_fake_ip = lambda ip: False # noqa: E731 — no declaration known: keep the private verdict _is_safe_url = lambda url: False # noqa: E731 — fail-closed: block all if safety module unavailable _is_always_blocked_url = lambda url: True # noqa: E731 — fail-closed on the floor too _normalize_url_for_request = lambda url: url # noqa: E731 — best-effort fallback @@ -263,7 +265,9 @@ _PRIVATE_HOST_SUFFIXES = (".localhost", ".local", ".lan", ".internal") def _url_is_private(url: str) -> bool: """True when the URL's host is (or resolves to) a private/LAN/loopback/CGNAT address. Routing oracle only: DNS failures are NOT private (the configured backend surfaces the - error); obvious names short-circuit the DNS hop.""" + error); obvious names short-circuit the DNS hop. A local proxy's declared fake-ip sentinel + (``security.fake_ip_ranges``) is not private: the name is public, the cloud browser resolves + it itself, so routing it to the local sidecar would send every URL local on such a host.""" import ipaddress import socket from urllib.parse import urlparse @@ -273,6 +277,8 @@ def _url_is_private(url: str) -> bool: ip = ipaddress.ip_address(host) except ValueError: return None + if _is_declared_fake_ip(ip): + return False return ip.is_private or ip.is_loopback or ip.is_link_local or ip in ipaddress.ip_network("100.64.0.0/10") try: diff --git a/tools/url_safety.py b/tools/url_safety.py index c54326b2b2..e4972fada7 100644 --- a/tools/url_safety.py +++ b/tools/url_safety.py @@ -121,6 +121,16 @@ _MAX_SSRF_CONNECT_IPS = 8 # ipaddress — must be blocked explicitly (Tailscale/WireGuard, cloud internal nets). _CGNAT_NETWORK = ipaddress.ip_network("100.64.0.0/10") +# Address classes a ``security.fake_ip_ranges`` declaration can never excuse: a local proxy owns +# none of them, and a declaration is trusted like ``allow_private_urls`` for whatever it names, +# so an entry overlapping one of these (including 0.0.0.0/0 and ::/0) would make real internal +# hosts dialable. Such entries are dropped with a warning instead. +_FAKE_IP_UNDECLARABLE_NETWORKS = tuple(ipaddress.ip_network(n) for n in ( + "0.0.0.0/32", "127.0.0.0/8", "10.0.0.0/8", "172.16.0.0/12", "192.168.0.0/16", # unspecified/loopback/RFC 1918 + "169.254.0.0/16", "100.64.0.0/10", # link-local, CGNAT + "::/128", "::1/128", "fc00::/7", "fe80::/10", # unspecified, loopback, ULA, link-local +)) + # Global toggle cache (process lifetime; see _global_allow_private_urls). _allow_private_resolved, _cached_allow_private = False, False _fake_ip_resolved, _cached_fake_ip_ranges = False, () @@ -184,9 +194,15 @@ def _resolve_fake_ip_ranges() -> tuple: networks = [] for entry in entries: try: - networks.append(ipaddress.ip_network(str(entry).strip(), strict=False)) + net = ipaddress.ip_network(str(entry).strip(), strict=False) except ValueError: logger.warning("Ignoring unparseable security.fake_ip_ranges entry: %r", entry) + continue + clash = next((r for r in _FAKE_IP_UNDECLARABLE_NETWORKS if r.version == net.version and net.overlaps(r)), None) + if clash is not None: + logger.warning("Ignoring security.fake_ip_ranges entry %r: it overlaps %s, which stays blocked", entry, clash) + continue + networks.append(net) return tuple(networks) diff --git a/website/docs/user-guide/security.md b/website/docs/user-guide/security.md index 5e1b0eacf5..4f346a78e1 100644 --- a/website/docs/user-guide/security.md +++ b/website/docs/user-guide/security.md @@ -765,7 +765,10 @@ security: Empty by default, and narrower than `allow_private_urls`: only the declared blocks get the exemption, they should be ranges the local proxy owns (the dial still goes to the proxy, which resolves the real target itself), and loopback, RFC 1918, link-local, CGNAT and cloud-metadata -destinations stay blocked. +destinations stay blocked — an entry that overlaps one of those classes (including `0.0.0.0/0` +or `::/0`) is ignored with a warning rather than widening the guard. On a host with a cloud +browser provider, the declared sentinel also stops counting as private for +`browser.auto_local_for_private_urls`, so those pages keep going to the cloud browser. ### Tirith Pre-Exec Security Scanning