diff --git a/docs/advanced_onboarding/configuration.md b/docs/advanced_onboarding/configuration.md index 0cc8831c50e..a13376bbce6 100644 --- a/docs/advanced_onboarding/configuration.md +++ b/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 diff --git a/news/+forwarded-client-ip-validation.bugfix.md b/news/+forwarded-client-ip-validation.bugfix.md new file mode 100644 index 00000000000..659cd5560ee --- /dev/null +++ b/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. diff --git a/reflex/app.py b/reflex/app.py index 718b7e0c4ae..91612735da4 100644 --- a/reflex/app.py +++ b/reflex/app.py @@ -9,6 +9,7 @@ import functools import importlib import inspect +import ipaddress import json import logging import operator @@ -600,6 +601,46 @@ 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 + if "%" in entry: + # A zone index only means something on the proxy's own host, and + # ``ipaddress`` keeps it without validating it. + 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 +2749,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, diff --git a/tests/units/test_app.py b/tests/units/test_app.py index 121df59ad75..8a5fee8f673 100644 --- a/tests/units/test_app.py +++ b/tests/units/test_app.py @@ -4782,6 +4782,18 @@ 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