From e20007e4e7c0374efc4ddd3b16c4a45bdf637acd Mon Sep 17 00:00:00 2001 From: kshitijk4poor <82637225+kshitijk4poor@users.noreply.github.com> Date: Thu, 29 Oct 2026 11:33:06 +0530 Subject: [PATCH] fix(browser): raw CDP URL scan reads bracketed IPv6 and scheme-relative hosts The URL literal scan behind raw CDP's private guard and metadata floor now reads a bracketed host such as https://[2606:4700:4700::1111]/ as one literal, so a public IPv6 URL is no longer refused on guarded backends and an IPv4-mapped metadata address (http://[::ffff:a9fe:a9fe]/) is refused with the guard off. The metadata floor also reads scheme-relative //host, ///host and \\host literals, which browsers load with the page's scheme. They are judged by their literal host without a DNS lookup, decimal, hex and octal IPv4 spellings (//2852039166/, //0xa9fea9fe/) included, so a //TODO comment costs no resolver call. A // comment, an a//b path or a // inside another URL's path or query is not taken for a second target, and the private guard keeps screening http(s) literals only. A scheme-relative literal carrying a control character (//169.254.169.254 followed by U+0001, which a browser strips) is refused like its http(s) spelling. Root cause: the literal regex stopped at the first ], cutting a bracketed host in two (the head failed to parse, the tail was never checked), and it only knew http(s):// prefixes. The public IPv6 refusal is a regression from f4aa77a467 (#146100, @teknium1, 2026-10-28); the mapped-address and scheme-relative gaps are older bugs. --- tests/tools/test_browser_eval_ssrf.py | 42 ++++++++++++++++++++++ tools/browser_cdp_tool.py | 13 ++++--- tools/browser_url_policy.py | 50 ++++++++++++++++++++++++--- 3 files changed, 93 insertions(+), 12 deletions(-) diff --git a/tests/tools/test_browser_eval_ssrf.py b/tests/tools/test_browser_eval_ssrf.py index 0b1f0368b8..47bc4a6b10 100644 --- a/tests/tools/test_browser_eval_ssrf.py +++ b/tests/tools/test_browser_eval_ssrf.py @@ -147,3 +147,45 @@ def test_raw_cdp_params_the_call_never_fetches_are_not_screened(cdp_guard): # An attribute the page then loads is still a fetch. src = {"nodeId": 1, "name": "src", "value": "http://169.254.169.254/latest/meta-data/"} assert "cloud metadata" in cdp_guard(method="DOM.setAttributeValue", params=src) + + +@pytest.mark.parametrize("local_backend", [False, True]) +def test_raw_cdp_reads_bracketed_ipv6_hosts_whole(cdp_guard): + """``[...]`` hosts are one literal: a public IPv6 URL passes, an IPv4-mapped metadata address does not.""" + public = {"frameId": "F", "url": "https://[2606:4700:4700::1111]/dns", "options": {}} + assert cdp_guard(method="Network.loadNetworkResource", params=public) is None + mapped = {"functionDeclaration": "() => fetch('http://[::ffff:a9fe:a9fe]/latest/meta-data/')"} + assert "cloud metadata" in cdp_guard(method="Runtime.callFunctionOn", params=mapped) + + +@pytest.mark.parametrize("local_backend", [False, True]) +def test_raw_cdp_metadata_floor_reads_scheme_relative_urls(cdp_guard): + """``//host``, ``///host`` and ``\\\\host`` load with the page's scheme; a ``//`` comment, a path, or a + ``//`` inside another URL's query is not a second target.""" + for target in ("//169.254.169.254/latest/meta-data/", "///169.254.169.254/latest/", "\\\\169.254.169.254\\latest", + "//metadata.google.internal/computeMetadata/v1/", "//2852039166/", "//0xA9FEA9FE/latest/", + "//0251.0376.0251.0376/", "//169.254.43518/", "//169.254.169.254\x01"): + verdict = cdp_guard(method="Runtime.evaluate", params={"expression": f"location.href='{target}'"}) + assert "cloud metadata" in verdict, target + html = {"expression": "document.body.innerHTML = ''"} + assert "cloud metadata" in cdp_guard(method="Runtime.evaluate", params=html) + src = {"nodeId": 1, "name": "src", "value": "//0xa9fea9fe/latest/"} + assert "cloud metadata" in cdp_guard(method="DOM.setAttributeValue", params=src) + harmless = {"expression": "// note: see docs\nconst p = 'a//b'; fetch('https://93.184.216.34//x')"} + assert cdp_guard(method="Runtime.evaluate", params=harmless) is None + # A //TODO comment or a scheme-relative public script is not an http(s) literal for the private check. + script = {"functionDeclaration": "function(){ //TODO\n import('//cdn.jsdelivr.net/npm/a.js') }"} + assert cdp_guard(method="Runtime.callFunctionOn", params=script) is None + for url in ("https://1.1.1.1/?next=//169.254.169.254/", "https://[2606:4700:4700::1111]//169.254.169.254/x"): + assert cdp_guard(method="Network.loadNetworkResource", params={"frameId": "F", "url": url}) is None, url + + +@pytest.mark.parametrize("local_backend", [False, True]) +def test_raw_cdp_scheme_relative_literals_cost_no_dns_lookup(cdp_guard, monkeypatch): + """``//word`` comments are judged by their literal host: no resolver call per comment.""" + from tools import url_safety + lookups = [] + monkeypatch.setattr(url_safety, "_getaddrinfo", lambda host, port=None: lookups.append(host) or []) + script = "\n".join(f"//step{i} fill the form" for i in range(20)) + "\n//eslint-disable-next-line\nrun()" + assert cdp_guard(method="Runtime.evaluate", params={"expression": script}) is None + assert lookups == [] diff --git a/tools/browser_cdp_tool.py b/tools/browser_cdp_tool.py index e5b788c8dc..32e1c68c3c 100644 --- a/tools/browser_cdp_tool.py +++ b/tools/browser_cdp_tool.py @@ -209,12 +209,12 @@ _CDP_NON_FETCHING_METHODS = frozenset({ def _param_url_literals(value: Any, key: str = "") -> list[str]: - """Every URL a CDP call's params name: each ``url`` value whole, plus the ``http(s)://`` literals in - every string at any depth (``Network.loadNetworkResource``, ``Runtime.callFunctionOn`` bodies, - ``Page.addScriptToEvaluateOnNewDocument`` sources, injected HTML).""" + """Every URL a CDP call's params name: each ``url`` value whole, plus the ``http(s)://`` and + protocol-relative ``//host`` literals in every string at any depth (``Network.loadNetworkResource``, + ``Runtime.callFunctionOn`` bodies, ``Page.addScriptToEvaluateOnNewDocument`` sources, injected HTML).""" from tools.browser_url_policy import url_literals if isinstance(value, str): - return ([value.strip()] if key == "url" and value.strip() else []) + url_literals(value) + return ([value.strip()] if key == "url" and value.strip() else []) + url_literals(value, protocol_relative=True) if isinstance(value, dict): return [u for k, v in value.items() for u in _param_url_literals(v, str(k))] if isinstance(value, (list, tuple)): @@ -237,8 +237,7 @@ def _browser_cdp_private_guard(*, method: str, params: dict[str, Any], endpoint: Raw CDP shares the cloud/private-network boundary of ``browser_exec``'s URL policy and must not become its bypass, so a page whose URL cannot be verified as public refuses page-reading methods. """ - from tools.browser_url_policy import eval_ssrf_guard_active, url_blocked - from tools.url_safety import is_always_blocked_url + from tools.browser_url_policy import eval_ssrf_guard_active, literal_targets_metadata, url_blocked guard_active = eval_ssrf_guard_active() guard = _METHOD_PARAM_GUARDS.get(method) if guard_active and guard is not None: @@ -249,7 +248,7 @@ def _browser_cdp_private_guard(*, method: str, params: dict[str, Any], endpoint: # The cloud-metadata floor applies with the private guard on or off, sidecar and local Chromium included, # as in browser_exec's _url_policy_error (#16234), to every URL the call could fetch. literals = _fetched_param_urls(method, params or {}) - metadata = next((u for u in literals if is_always_blocked_url(u)), None) + metadata = next((u for u in literals if literal_targets_metadata(u)), None) if metadata: return _blocked(f"Blocked: CDP {method} targets a cloud metadata endpoint ({metadata}).", method) if not guard_active: diff --git a/tools/browser_url_policy.py b/tools/browser_url_policy.py index dfd3614ae1..d436547bab 100644 --- a/tools/browser_url_policy.py +++ b/tools/browser_url_policy.py @@ -16,7 +16,12 @@ import urllib.parse from typing import Callable, Optional from tools.url_safety import ( + _BLOCKED_HOSTNAMES, + _authority_ambiguity, + _is_always_blocked_ip, _is_declared_fake_ip, + _parse_ip, + _url_hostname, is_always_blocked_url, is_safe_url, normalize_url_for_request, @@ -28,8 +33,17 @@ logger = logging.getLogger(__name__) _PRIVATE_HOST_SUFFIXES = (".localhost", ".local", ".lan", ".internal") _CGNAT = ipaddress.ip_network("100.64.0.0/10") # URL-shaped literals in a JS expression (http/https only). fetch/XHR to a private host never updates -# ``location.href``, so the literals are pre-screened instead. -_JS_URL_LITERAL_RE = re.compile(r"""https?://[^\s'"`)\]<>]+""", re.IGNORECASE) +# ``location.href``, so the literals are pre-screened instead. A literal ends at a quote, whitespace, +# ``)``, ``]`` or ``<>``, except that one ``[...]`` segment (a bracketed IPv6 host such as +# ``http://[::1]:8080/``) is read whole. +_URL_BODY = r"""(?=[^\s'"`)\]<>])(?:[^\s'"`)\[\]<>]*\[[^\s'"`)\[\]<>]*\])?[^\s'"`)\]<>]*""" +_JS_URL_LITERAL_RE = re.compile(r"https?://" + _URL_BODY, re.IGNORECASE) +# Plus scheme-relative ``//host``, ``///host`` or ``\\host`` (browsers fetch each with the page's scheme and +# read ``\`` as ``/``); not ``a//b`` or ``// note``. One scan, so a ``//`` inside an http(s) literal's path or +# query (``https://x/?next=//y``) is part of that literal, not a second target. +_URL_OR_SCHEME_RELATIVE_RE = re.compile(r"""(?:https?://|(? dict: @@ -136,9 +150,35 @@ def url_blocked(url: str) -> bool: return is_always_blocked_url(url) or not is_safe_url(url) -def url_literals(text: str) -> list[str]: - """Every ``http(s)://`` literal in free text (a JS expression, an HTML snippet, a CDP param).""" - return [m.rstrip(".,;") for m in _JS_URL_LITERAL_RE.findall(text)] if isinstance(text, str) else [] +def url_literals(text: str, *, protocol_relative: bool = False) -> list[str]: + """Every ``http(s)://`` literal in free text (a JS expression, an HTML snippet, a CDP param); with + ``protocol_relative``, each scheme-relative ``//host/...`` literal too, kept scheme-less.""" + if not isinstance(text, str): + return [] + found = (_URL_OR_SCHEME_RELATIVE_RE if protocol_relative else _JS_URL_LITERAL_RE).findall(text) + return [m.rstrip(".,;") for m in found] + + +def literal_targets_metadata(literal: str) -> bool: + """True when a URL literal hits the cloud-metadata floor. A scheme-relative one (``//host``, + ``\\\\host``) is judged by its literal host only (a metadata hostname or IP, decimal/hex/octal IPv4 + spellings included), never resolved, so a ``//TODO`` comment costs no DNS lookup.""" + if not literal.startswith(("/", "\\")): + return is_always_blocked_url(literal) + url = "http://" + literal.lstrip("/\\").replace("\\", "/") + try: + if _authority_ambiguity(url, floor=True): # a control character can move the host, as for http(s) + return True + host = _url_hostname(url, percent_decode=True) + except ValueError: + return False + ip = _parse_ip(host) + if ip is None and _NUMERIC_HOST_RE.fullmatch(host): + try: # decimal, hex, octal or short IPv4 (``//0xa9fea9fe``), read as browsers do; still no DNS + ip = ipaddress.IPv4Address(socket.inet_aton(host)) + except (OSError, ValueError): + pass + return host in _BLOCKED_HOSTNAMES or (ip is not None and _is_always_blocked_ip(ip)) def expression_targets_private_url(expression: str, *, blocked: Callable[[str], bool] = url_blocked) -> Optional[str]: