From cadffa95052dc5e5d65021deab628900b14cb39c Mon Sep 17 00:00:00 2001
From: Mark Story <mark@mark-story.com>
Date: Thu, 10 Mar 2016 21:53:35 -0500
Subject: [PATCH] Don't trust Client-IP header unless behind a proxy

REMOTE_ADDR is a far safer place to get an client's IP over the header
which is easily spoofed. If someone is trusting the proxy we'll prefer
x-forwarded-for and fallback to client-ip should that not exist.

Remove support for http_clientaddress as I can't find any record of it
existing in either the php docs or http specs.
---
 src/Network/Request.php                | 15 +++------------
 tests/TestCase/Network/RequestTest.php | 12 ++++++------
 2 files changed, 9 insertions(+), 18 deletions(-)

diff --git a/src/Network/Request.php b/src/Network/Request.php
index eda0641f64e..5ce1c693bc5 100644
--- a/src/Network/Request.php
+++ b/src/Network/Request.php
@@ -514,21 +514,12 @@ public function clientIp()
     {
         if ($this->trustProxy && $this->env('HTTP_X_FORWARDED_FOR')) {
             $ipaddr = preg_replace('/(?:,.*)/', '', $this->env('HTTP_X_FORWARDED_FOR'));
+        } elseif ($this->trustProxy && $this->env('HTTP_CLIENT_IP')) {
+            $ipaddr = $this->env('HTTP_CLIENT_IP');
         } else {
-            if ($this->env('HTTP_CLIENT_IP')) {
-                $ipaddr = $this->env('HTTP_CLIENT_IP');
-            } else {
-                $ipaddr = $this->env('REMOTE_ADDR');
-            }
+            $ipaddr = $this->env('REMOTE_ADDR');
         }
 
-        if ($this->env('HTTP_CLIENTADDRESS')) {
-            $tmpipaddr = $this->env('HTTP_CLIENTADDRESS');
-
-            if (!empty($tmpipaddr)) {
-                $ipaddr = preg_replace('/(?:,.*)/', '', $tmpipaddr);
-            }
-        }
         return trim($ipaddr);
     }
 
diff --git a/tests/TestCase/Network/RequestTest.php b/tests/TestCase/Network/RequestTest.php
index e9206ba5f77..0ede402502c 100644
--- a/tests/TestCase/Network/RequestTest.php
+++ b/tests/TestCase/Network/RequestTest.php
@@ -504,7 +504,7 @@ public function testMethodOverrides()
      *
      * @return void
      */
-    public function testclientIp()
+    public function testClientIp()
     {
         $request = new Request(['environment' => [
             'HTTP_X_FORWARDED_FOR' => '192.168.1.5, 10.0.1.1, proxy.com',
@@ -515,17 +515,17 @@ public function testclientIp()
         $request->trustProxy = true;
         $this->assertEquals('192.168.1.5', $request->clientIp());
 
-        $request->trustProxy = false;
+        $request->env('HTTP_X_FORWARDED_FOR', '');
         $this->assertEquals('192.168.1.2', $request->clientIp());
 
+        $request->trustProxy = false;
+        $this->assertEquals('192.168.1.3', $request->clientIp());
+
         $request->env('HTTP_X_FORWARDED_FOR', '');
-        $this->assertEquals('192.168.1.2', $request->clientIp());
+        $this->assertEquals('192.168.1.3', $request->clientIp());
 
         $request->env('HTTP_CLIENT_IP', '');
         $this->assertEquals('192.168.1.3', $request->clientIp());
-
-        $request->env('HTTP_CLIENTADDRESS', '10.0.1.2, 10.0.1.1');
-        $this->assertEquals('10.0.1.2', $request->clientIp());
     }
 
     /**