From 9b9f911b7b83b29e9d6eec85e2c624b098f97a39 Mon Sep 17 00:00:00 2001 From: Sayed Kaif Date: Mon, 20 Jul 2026 21:40:23 +0530 Subject: [PATCH] scan X-Forwarded-For right-to-left in get_client_ip --- httplib.h | 24 ++++++++++++------------ test/test.cc | 40 ++++++++++++++++++++++++++++++++++++++-- 2 files changed, 50 insertions(+), 14 deletions(-) diff --git a/httplib.h b/httplib.h index 261790bff7..efba4647d4 100644 --- a/httplib.h +++ b/httplib.h @@ -12420,25 +12420,25 @@ get_client_ip(const std::string &x_forwarded_for, // caller can fall back to the connection-level remote address. if (ip_list.empty()) { return std::string(); } - for (size_t i = 0; i < ip_list.size(); ++i) { - auto ip = ip_list[i]; + // Each hop appends the address it received the request from, so the rightmost + // entries are the ones written by our own infrastructure while the leftmost + // are whatever the original client chose to send. Walk from the right and + // skip trusted proxies; the first address that is not a trusted proxy is the + // furthest point still attributable to a real hop, i.e. the client. Scanning + // from the left instead lets a client forge an arbitrary address by following + // it with a trusted proxy's address, which the left-to-right scan then + // returned as the client. + for (size_t i = ip_list.size(); i-- > 0;) { + const auto &ip = ip_list[i]; auto is_trusted_proxy = std::any_of(trusted_proxies.begin(), trusted_proxies.end(), [&](const std::string &proxy) { return ip == proxy; }); - if (is_trusted_proxy) { - if (i == 0) { - // If the trusted proxy is the first IP, there's no preceding client IP - return ip; - } else { - // Return the IP immediately before the trusted proxy - return ip_list[i - 1]; - } - } + if (!is_trusted_proxy) { return ip; } } - // If no trusted proxy is found, return the first IP in the list + // Every hop was a trusted proxy; fall back to the first entry. return ip_list.front(); } diff --git a/test/test.cc b/test/test.cc index e5dd95e5e2..8d46f671fe 100644 --- a/test/test.cc +++ b/test/test.cc @@ -15456,7 +15456,7 @@ TEST(ForwardedHeadersTest, MultipleTrustedProxies_UsesClientIP) { EXPECT_EQ(observed_remote_addr, "198.51.100.23"); } -TEST(ForwardedHeadersTest, TrustedProxyNotInHeader_UsesFirstFromXFF) { +TEST(ForwardedHeadersTest, TrustedProxyNotInHeader_UsesRightmostIP) { Server svr; svr.set_trusted_proxies({"192.0.2.45"}); @@ -15486,8 +15486,44 @@ TEST(ForwardedHeadersTest, TrustedProxyNotInHeader_UsesFirstFromXFF) { ASSERT_TRUE(res); EXPECT_EQ(StatusCode::OK_200, res->status); + // No listed hop is a trusted proxy, so the rightmost entry (the one written + // by the closest hop) is used. The leftmost entry is fully client-controlled + // and must not be selected. EXPECT_EQ(observed_xff, "198.51.100.23, 198.51.100.24"); - EXPECT_EQ(observed_remote_addr, "198.51.100.23"); + EXPECT_EQ(observed_remote_addr, "198.51.100.24"); +} + +TEST(ForwardedHeadersTest, SpoofedIPBeforeTrustedProxyIsIgnored) { + Server svr; + + svr.set_trusted_proxies({"10.0.0.1"}); + + std::string observed_remote_addr; + + svr.Get("/ip", [&](const Request &req, Response &res) { + observed_remote_addr = req.remote_addr; + res.set_content("ok", "text/plain"); + }); + + thread t = thread([&]() { svr.listen(HOST, PORT); }); + auto se = detail::scope_exit([&] { + svr.stop(); + t.join(); + ASSERT_FALSE(svr.is_running()); + }); + + svr.wait_until_ready(); + + // The trusted proxy appends the address it saw (5.6.7.8). The client prepends + // a forged address and the trusted proxy's own address; the forged value must + // not win over the address the trusted proxy actually recorded. + Client cli(HOST, PORT); + auto res = cli.Get( + "/ip", Headers{{"X-Forwarded-For", "1.2.3.4, 10.0.0.1, 5.6.7.8"}}); + + ASSERT_TRUE(res); + EXPECT_EQ(StatusCode::OK_200, res->status); + EXPECT_EQ(observed_remote_addr, "5.6.7.8"); } TEST(ForwardedHeadersTest, LastHopTrusted_SelectsImmediateLeftIP) {