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