fix: validate the trusted X-Forwarded-For entry as client_ip

docs/advanced_onboarding/configuration.md

@@ -143,7 +143,7 @@ config = rx.Config(
 )
 ```
 
-For example, `trusted_proxy_hops=1` uses the rightmost `X-Forwarded-For` entry; `2` uses the second entry from the right. The same entry of `X-Forwarded-Host` and `X-Forwarded-Proto` is used when the backend checks the origin of a WebSocket connection (see [CORS](#cors)). Proxies usually set these two headers instead of appending to them, so when one has fewer entries than `trusted_proxy_hops`, its leftmost entry is used. The default is `0`. Do not enable forwarded-header trust if untrusted clients can connect directly to the backend or bypass the trusted proxy chain. The setting can also be provided as `REFLEX_TRUSTED_PROXY_HOPS`.
+For example, `trusted_proxy_hops=1` uses the rightmost `X-Forwarded-For` entry; `2` uses the second entry from the right. The entry is normalized to an IP address after dropping any port and IPv6 brackets; if it is not a valid address, it is ignored and the connection's peer address is used. The same entry of `X-Forwarded-Host` and `X-Forwarded-Proto` is used when the backend checks the origin of a WebSocket connection (see [CORS](#cors)). Proxies usually set these two headers instead of appending to them, so when one has fewer entries than `trusted_proxy_hops`, its leftmost entry is used. The default is `0`. Do not enable forwarded-header trust if untrusted clients can connect directly to the backend or bypass the trusted proxy chain. The setting can also be provided as `REFLEX_TRUSTED_PROXY_HOPS`.
 
 ## Plugins
 

news/+forwarded-client-ip-validation.bugfix.md

@@ -0,0 +1 @@
+`router.session.client_ip` now normalizes the `X-Forwarded-For` entry selected by `trusted_proxy_hops` to an IP address, dropping any port and IPv6 brackets, and falls back to the connection's peer address when the entry is not a valid address.

reflex/app.py

@@ -9,6 +9,7 @@
 import functools
 import importlib
 import inspect
+import ipaddress
 import json
 import logging
 import operator
@@ -600,6 +601,42 @@ def _trusted_forwarded_value(
     return None if appended or not entries else entries[0]
 
 
+def _is_port(text: str) -> bool:
+    """Return whether text is a port number.
+
+    Args:
+        text: The text after the ``:`` of an address.
+
+    Returns:
+        Whether the text is made of ASCII digits only.
+    """
+    return text.isascii() and text.isdigit()
+
+
+def _forwarded_client_ip(entry: str) -> str | None:
+    """Normalize an ``X-Forwarded-For`` entry to an IP address.
+
+    Args:
+        entry: The entry, an IP address with an optional port and IPv6 brackets.
+
+    Returns:
+        The normalized address, or None when the entry is not an IP address.
+    """
+    if entry.startswith("["):
+        entry, closed, rest = entry[1:].partition("]")
+        port_ok = not rest or (rest.startswith(":") and _is_port(rest[1:]))
+        if not closed or not port_ok:
+            return None
+    elif entry.count(":") == 1:
+        entry, _, port = entry.partition(":")
+        if not _is_port(port):
+            return None
+    try:
+        return str(ipaddress.ip_address(entry))
+    except ValueError:
+        return None
+
+
 def _origin_matches_request(
     origin: str, hosts: Iterable[str | None], forwarded_proto: str | None
 ) -> bool:
@@ -2708,7 +2745,13 @@ def _build_static_router_data(self, sid: str, environ: dict) -> dict[str, Any]:
             headers.get("x-forwarded-for", ""), get_config().trusted_proxy_hops
         )
         if forwarded_ip is not None:
-            client_ip = forwarded_ip
+            if (normalized_ip := _forwarded_client_ip(forwarded_ip)) is not None:
+                client_ip = normalized_ip
+            else:
+                logger.debug(
+                    f"Ignoring X-Forwarded-For entry {_clip(forwarded_ip)}: "
+                    "not an IP address."
+                )
         return {
             constants.RouteVar.SESSION_ID: sid,
             constants.RouteVar.HEADERS: headers,

tests/units/test_app.py

@@ -4782,6 +4782,16 @@ def test_static_router_data_keeps_every_repeated_header_field(
         (1, "203.0.113.7", "10.0.0.2", "203.0.113.7"),
         (2, "6.6.6.6, 203.0.113.7, 10.0.0.2", "127.0.0.1", "203.0.113.7"),
         (2, "203.0.113.7", "10.0.0.2", "10.0.0.2"),
+        (1, "203.0.113.7:51234", "10.0.0.2", "203.0.113.7"),
+        (1, "[2001:db8::1]:443", "10.0.0.2", "2001:db8::1"),
+        (1, "[2001:db8::1]", "10.0.0.2", "2001:db8::1"),
+        (1, "2001:db8:0:0:0:0:0:1", "10.0.0.2", "2001:db8::1"),
+        (1, "6.6.6.6, not an ip <script>", "10.0.0.2", "10.0.0.2"),
+        (1, "[2001:db8::1", "10.0.0.2", "10.0.0.2"),
+        (1, "[2001:db8::1]junk", "10.0.0.2", "10.0.0.2"),
+        (1, "[2001:db8::1]:", "10.0.0.2", "10.0.0.2"),
+        (1, "203.0.113.7:garbage", "10.0.0.2", "10.0.0.2"),
+        (1, "203.0.113.7:", "10.0.0.2", "10.0.0.2"),
     ],
 )
 def test_static_router_data_resolves_client_ip_from_trusted_proxy_hops(
@@ -4809,6 +4819,30 @@ def test_static_router_data_resolves_client_ip_from_trusted_proxy_hops(
     assert router_data[constants.RouteVar.CLIENT_IP] == expected_ip
 
 
+def test_static_router_data_logs_ignored_forwarded_client_ip(
+    monkeypatch: pytest.MonkeyPatch, caplog: pytest.LogCaptureFixture
+):
+    """A forwarded entry that is not an IP address is logged and ignored."""
+    monkeypatch.setattr("reflex.app.get_config", lambda: Mock(trusted_proxy_hops=1))
+    with caplog.at_level(logging.DEBUG, logger="reflex.app"):
+        router_data = EventNamespace("/event", Mock())._build_static_router_data(
+            "sid1",
+            {
+                "asgi.scope": {
+                    "headers": [(b"x-forwarded-for", b"not an ip <script>")],
+                    "client": ("10.0.0.2", 1234),
+                }
+            },
+        )
+
+    assert router_data[constants.RouteVar.CLIENT_IP] == "10.0.0.2"
+    assert [
+        record.getMessage()
+        for record in caplog.records
+        if record.name == "reflex.app" and record.levelno == logging.DEBUG
+    ] == ["Ignoring X-Forwarded-For entry 'not an ip <script>': not an IP address."]
+
+
 def test_call_ignores_stale_marker_without_dev_backend_reload(
     compilable_app: tuple[App, Path],
     mocker: MockerFixture,