scan X-Forwarded-For right-to-left in get_client_ip (#2503)

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();
 }
 

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) {