Skip to content

Commit 425c4cb

Browse files
authored
fix(security): join repeated X-Forwarded-For lines before taking the last hop (#14425)
Both client-IP resolvers read the forwarded chain with `Headers.get`, which returns only the first matching header line. Proxies that append their own `X-Forwarded-For` line rather than extending the client's (HAProxy's `option forwardfor` among them) therefore leave the caller's line as the one we parse, so the "rightmost entry is the trusted proxy's last hop" invariant resolves to an attacker-chosen value. Under `rate_limit_trust_proxy=True` this lets a remote caller send `X-Forwarded-For: 127.0.0.1` and pass the local-only gate on `POST /api/v1/mcp/project/{id}/install`, which writes MCP client config to the host filesystem - the same bypass GHSA-4f6c-2vvp-gw82 reported, reachable again through a differently-shaped header. It also lets a caller pin or rotate their own rate-limit bucket on login and public-flow endpoints. Join every occurrence in order before taking the last hop, and drop empty entries so a blank header falls back to the TCP peer instead of resolving to an empty client IP. Default deployments (`rate_limit_trust_proxy=False`) never read the header and were not affected.
1 parent 63d4b97 commit 425c4cb

4 files changed

Lines changed: 123 additions & 18 deletions

File tree

src/backend/base/langflow/api/v1/mcp_projects.py

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -83,6 +83,7 @@
8383
from langflow.services.database.models.user.crud import get_user_by_username
8484
from langflow.services.database.models.user.model import User
8585
from langflow.services.deps import get_service
86+
from langflow.services.rate_limit.service import get_last_forwarded_for_hop
8687

8788
# Constants
8889
ALL_INTERFACES_HOST = "0.0.0.0" # noqa: S104
@@ -792,6 +793,9 @@ def get_client_ip(request: Request) -> str:
792793
(``rate_limit_trust_proxy``) do we consult ``X-Forwarded-For``, and then we
793794
take the rightmost entry — the last hop added by the trusted proxy, which a
794795
client cannot forge — mirroring ``langflow.services.rate_limit.service.get_client_ip``.
796+
Every occurrence of the header is joined first, so a proxy that appends its
797+
own line rather than extending the client's cannot leave the attacker's line
798+
as the one we read.
795799
796800
Args:
797801
request: FastAPI Request object
@@ -802,11 +806,9 @@ def get_client_ip(request: Request) -> str:
802806
# Only consult X-Forwarded-For when an operator has explicitly declared a
803807
# trusted proxy; otherwise the header is attacker-controlled.
804808
if get_settings_service().settings.rate_limit_trust_proxy:
805-
forwarded_for = request.headers.get("X-Forwarded-For")
806-
if forwarded_for:
807-
# Rightmost entry = last hop added by the trusted proxy (unspoofable);
808-
# the leftmost entry is client-supplied and must never be trusted.
809-
return forwarded_for.split(",")[-1].strip()
809+
last_hop = get_last_forwarded_for_hop(request)
810+
if last_hop:
811+
return last_hop
810812

811813
# Default: trust only the real TCP peer.
812814
if request.client:

src/backend/base/langflow/services/rate_limit/service.py

Lines changed: 24 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -104,6 +104,27 @@ def check_rate_limit(
104104
raise RateLimitExceeded(limit_wrapper)
105105

106106

107+
def get_last_forwarded_for_hop(request: Request) -> str | None:
108+
"""Return the rightmost ``X-Forwarded-For`` entry, or None when the header is absent.
109+
110+
The header may legitimately arrive as several separate lines: some proxies
111+
(HAProxy's ``option forwardfor``, for one) append their own line instead of
112+
extending the client's. ``Headers.get`` returns only the first line, so
113+
reading it alone would yield the rightmost entry of the *client-supplied*
114+
line and hand an attacker control of a value the caller assumes is
115+
unspoofable. Join every line in order, per RFC 9110 field-order semantics,
116+
before taking the last hop.
117+
118+
Callers must gate this on an explicit trusted-proxy opt-in; the header is
119+
attacker-controlled otherwise.
120+
"""
121+
# X-Forwarded-For format: "client, proxy1, proxy2", possibly split across lines.
122+
chain = [ip.strip() for line in request.headers.getlist("x-forwarded-for") for ip in line.split(",")]
123+
entries = [ip for ip in chain if ip]
124+
# Rightmost entry = last hop added by the trusted proxy; the leftmost is client-supplied.
125+
return entries[-1] if entries else None
126+
127+
107128
def get_client_ip(request: Request) -> str:
108129
"""Extract client IP address from request, using rightmost X-Forwarded-For entry.
109130
@@ -118,12 +139,9 @@ def get_client_ip(request: Request) -> str:
118139
str: Client IP address
119140
"""
120141
# Check X-Forwarded-For header first (for proxied requests)
121-
forwarded_for = request.headers.get("X-Forwarded-For")
122-
if forwarded_for:
123-
# X-Forwarded-For format: "client, proxy1, proxy2"
124-
# Take the rightmost IP (last proxy before us)
125-
ips = [ip.strip() for ip in forwarded_for.split(",")]
126-
return ips[-1]
142+
last_hop = get_last_forwarded_for_hop(request)
143+
if last_hop:
144+
return last_hop
127145

128146
# Fall back to direct client IP
129147
if request.client:

src/backend/tests/unit/api/v1/test_mcp_install_xff_trust.py

Lines changed: 51 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,11 +12,18 @@
1212

1313
from langflow.api.v1 import mcp_projects
1414
from langflow.api.v1.mcp_projects import get_client_ip, is_local_ip
15+
from starlette.datastructures import Headers
1516

1617

1718
def _request(headers: dict | None = None, host: str | None = "203.0.113.7"):
1819
client = SimpleNamespace(host=host) if host is not None else None
19-
return SimpleNamespace(headers=headers or {}, client=client)
20+
return SimpleNamespace(headers=Headers(headers or {}), client=client)
21+
22+
23+
def _request_raw(raw_headers: list[tuple[bytes, bytes]], host: str | None = "203.0.113.7"):
24+
"""Build a request whose headers may repeat, which ``Headers(dict)`` cannot express."""
25+
client = SimpleNamespace(host=host) if host is not None else None
26+
return SimpleNamespace(headers=Headers(raw=raw_headers), client=client)
2027

2128

2229
def _set_trust_proxy(monkeypatch, *, value: bool) -> None:
@@ -53,6 +60,49 @@ def test_trusted_proxy_uses_rightmost_xff(monkeypatch):
5360
assert is_local_ip(ip) is False
5461

5562

63+
def test_trusted_proxy_joins_repeated_xff_lines(monkeypatch):
64+
"""A proxy that appends its own header line must not leave the client's line as the one we read.
65+
66+
HAProxy's ``option forwardfor`` adds a second ``X-Forwarded-For`` line rather than extending the
67+
client's. ``Headers.get`` returns only the first line, so reading it alone would take the
68+
rightmost entry of the attacker-supplied line — handing the attacker the value the locality gate
69+
assumes is unspoofable.
70+
"""
71+
_set_trust_proxy(monkeypatch, value=True)
72+
req = _request_raw(
73+
[
74+
(b"host", b"target:7860"),
75+
(b"x-forwarded-for", b"127.0.0.1"), # attacker's own line, sent first
76+
(b"x-forwarded-for", b"203.0.113.7"), # line appended by the trusted proxy
77+
],
78+
host="10.0.0.5", # TCP peer is the proxy
79+
)
80+
ip = get_client_ip(req)
81+
assert ip == "203.0.113.7"
82+
assert is_local_ip(ip) is False
83+
84+
85+
def test_repeated_xff_lines_ignored_without_trusted_proxy(monkeypatch):
86+
"""Without the trusted-proxy opt-in, repeated lines are ignored entirely."""
87+
_set_trust_proxy(monkeypatch, value=False)
88+
req = _request_raw(
89+
[(b"x-forwarded-for", b"127.0.0.1"), (b"x-forwarded-for", b"127.0.0.1")],
90+
host="203.0.113.7",
91+
)
92+
ip = get_client_ip(req)
93+
assert ip == "203.0.113.7"
94+
assert is_local_ip(ip) is False
95+
96+
97+
def test_empty_xff_falls_back_to_peer(monkeypatch):
98+
"""A blank header must not resolve to an empty client IP."""
99+
_set_trust_proxy(monkeypatch, value=True)
100+
req = _request(headers={"X-Forwarded-For": " "}, host="203.0.113.7")
101+
ip = get_client_ip(req)
102+
assert ip == "203.0.113.7"
103+
assert is_local_ip(ip) is False
104+
105+
56106
def test_no_client_falls_back_to_non_local(monkeypatch):
57107
"""When the peer cannot be determined, fall back to a non-routable, non-local IP."""
58108
_set_trust_proxy(monkeypatch, value=False)

src/backend/tests/unit/test_login_rate_limiting.py

Lines changed: 41 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
from unittest.mock import Mock
66

77
import pytest
8+
from starlette.datastructures import Headers
89

910

1011
@pytest.fixture
@@ -103,7 +104,7 @@ def test_get_client_ip_from_x_forwarded_for_single(self):
103104
from langflow.services.rate_limit.service import get_client_ip
104105

105106
request = Mock()
106-
request.headers = {"X-Forwarded-For": "203.0.113.1"}
107+
request.headers = Headers({"X-Forwarded-For": "203.0.113.1"})
107108
request.client = Mock(host="127.0.0.1")
108109

109110
ip = get_client_ip(request)
@@ -115,7 +116,7 @@ def test_get_client_ip_from_x_forwarded_for_chain_uses_rightmost(self):
115116
from langflow.services.rate_limit.service import get_client_ip
116117

117118
request = Mock()
118-
request.headers = {"X-Forwarded-For": "203.0.113.1, 198.51.100.1, 192.0.2.1"}
119+
request.headers = Headers({"X-Forwarded-For": "203.0.113.1, 198.51.100.1, 192.0.2.1"})
119120
request.client = Mock(host="127.0.0.1")
120121

121122
ip = get_client_ip(request)
@@ -128,20 +129,54 @@ def test_get_client_ip_from_x_forwarded_for_with_spaces(self):
128129
from langflow.services.rate_limit.service import get_client_ip
129130

130131
request = Mock()
131-
request.headers = {"X-Forwarded-For": " 203.0.113.1 , 198.51.100.1 "}
132+
request.headers = Headers({"X-Forwarded-For": " 203.0.113.1 , 198.51.100.1 "})
132133
request.client = Mock(host="127.0.0.1")
133134

134135
ip = get_client_ip(request)
135136

136137
# Should use rightmost IP, stripped of whitespace
137138
assert ip == "198.51.100.1"
138139

140+
def test_get_client_ip_joins_repeated_x_forwarded_for_lines(self):
141+
"""Repeated X-Forwarded-For lines must be joined before taking the rightmost entry.
142+
143+
Some proxies append their own header line instead of extending the client's. Reading only
144+
the first line would key the rate limiter on an attacker-chosen value, letting a caller pin
145+
or rotate their own bucket.
146+
"""
147+
from langflow.services.rate_limit.service import get_client_ip
148+
149+
request = Mock()
150+
request.headers = Headers(
151+
raw=[
152+
(b"x-forwarded-for", b"203.0.113.1"), # client-supplied line
153+
(b"x-forwarded-for", b"192.0.2.1"), # line appended by the trusted proxy
154+
]
155+
)
156+
request.client = Mock(host="10.0.0.5")
157+
158+
ip = get_client_ip(request)
159+
160+
assert ip == "192.0.2.1"
161+
162+
def test_get_client_ip_ignores_blank_x_forwarded_for(self):
163+
"""A blank header must fall back to the peer rather than returning an empty key."""
164+
from langflow.services.rate_limit.service import get_client_ip
165+
166+
request = Mock()
167+
request.headers = Headers({"X-Forwarded-For": " , "})
168+
request.client = Mock(host="192.168.1.100")
169+
170+
ip = get_client_ip(request)
171+
172+
assert ip == "192.168.1.100"
173+
139174
def test_get_client_ip_from_direct_connection(self):
140175
"""Test IP extraction from direct client connection (no proxy)."""
141176
from langflow.services.rate_limit.service import get_client_ip
142177

143178
request = Mock()
144-
request.headers = {}
179+
request.headers = Headers({})
145180
request.client = Mock(host="192.168.1.100")
146181

147182
ip = get_client_ip(request)
@@ -153,7 +188,7 @@ def test_get_client_ip_fallback_to_unknown(self):
153188
from langflow.services.rate_limit.service import get_client_ip
154189

155190
request = Mock()
156-
request.headers = {}
191+
request.headers = Headers({})
157192
request.client = None
158193

159194
ip = get_client_ip(request)
@@ -165,7 +200,7 @@ def test_get_client_ip_prefers_x_forwarded_for(self):
165200
from langflow.services.rate_limit.service import get_client_ip
166201

167202
request = Mock()
168-
request.headers = {"X-Forwarded-For": "203.0.113.1"}
203+
request.headers = Headers({"X-Forwarded-For": "203.0.113.1"})
169204
request.client = Mock(host="127.0.0.1")
170205

171206
ip = get_client_ip(request)

0 commit comments

Comments
 (0)