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.
cakephp · Mar 14, 2016 · 6291e03 · 6291e03
1 parent 4f8d53c
commit 6291e03
Show file tree

Hide file tree

Showing 2 changed files with 9 additions and 18 deletions.
diff --git a/src/Network/Request.php b/src/Network/Request.php
@@ -523,21 +523,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
@@ -530,7 +530,7 @@ public function testDefaultEnvValue()
      *
      * @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',
@@ -541,17 +541,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());
     }
 
     /**