From 48af49ddde16c8b99edb701f1c31283455b2b0b6 Mon Sep 17 00:00:00 2001 From: mark_story Date: Thu, 10 Mar 2016 22:02:34 -0500 Subject: [PATCH] Don't trust CLIENT_IP The client_ip header can easily be forged. In 'safe' modes we should only trust the remote_addr which comes from the sapi. Remove support for http_clientaddress as I can't seem to find where this ever came from in PHP on the http specs. --- lib/Cake/Network/CakeRequest.php | 16 +++------------- lib/Cake/Test/Case/Network/CakeRequestTest.php | 14 +++++++------- 2 files changed, 10 insertions(+), 20 deletions(-) diff --git a/lib/Cake/Network/CakeRequest.php b/lib/Cake/Network/CakeRequest.php index eb9cebd847b..8bd2b3c5074 100644 --- a/lib/Cake/Network/CakeRequest.php +++ b/lib/Cake/Network/CakeRequest.php @@ -417,20 +417,10 @@ public function contentType() { public function clientIp($safe = true) { if (!$safe && env('HTTP_X_FORWARDED_FOR')) { $ipaddr = preg_replace('/(?:,.*)/', '', env('HTTP_X_FORWARDED_FOR')); + } elseif (!$safe && env('HTTP_CLIENT_IP')) { + $ipaddr = env('HTTP_CLIENT_IP'); } else { - if (env('HTTP_CLIENT_IP')) { - $ipaddr = env('HTTP_CLIENT_IP'); - } else { - $ipaddr = env('REMOTE_ADDR'); - } - } - - if (env('HTTP_CLIENTADDRESS')) { - $tmpipaddr = env('HTTP_CLIENTADDRESS'); - - if (!empty($tmpipaddr)) { - $ipaddr = preg_replace('/(?:,.*)/', '', $tmpipaddr); - } + $ipaddr = env('REMOTE_ADDR'); } return trim($ipaddr); } diff --git a/lib/Cake/Test/Case/Network/CakeRequestTest.php b/lib/Cake/Test/Case/Network/CakeRequestTest.php index 90f92c7bf53..418ef635568 100644 --- a/lib/Cake/Test/Case/Network/CakeRequestTest.php +++ b/lib/Cake/Test/Case/Network/CakeRequestTest.php @@ -711,18 +711,18 @@ public function testclientIp() { $_SERVER['HTTP_X_FORWARDED_FOR'] = '192.168.1.5, 10.0.1.1, proxy.com'; $_SERVER['HTTP_CLIENT_IP'] = '192.168.1.2'; $_SERVER['REMOTE_ADDR'] = '192.168.1.3'; + $request = new CakeRequest('some/path'); - $this->assertEquals('192.168.1.5', $request->clientIp(false)); - $this->assertEquals('192.168.1.2', $request->clientIp()); + $this->assertEquals('192.168.1.3', $request->clientIp(), 'Use remote_addr in safe mode'); + $this->assertEquals('192.168.1.5', $request->clientIp(false), 'Use x-forwarded'); unset($_SERVER['HTTP_X_FORWARDED_FOR']); - $this->assertEquals('192.168.1.2', $request->clientIp()); + $this->assertEquals('192.168.1.3', $request->clientIp(), 'safe uses remote_addr'); + $this->assertEquals('192.168.1.2', $request->clientIp(false), 'unsafe reads from client_ip'); unset($_SERVER['HTTP_CLIENT_IP']); - $this->assertEquals('192.168.1.3', $request->clientIp()); - - $_SERVER['HTTP_CLIENTADDRESS'] = '10.0.1.2, 10.0.1.1'; - $this->assertEquals('10.0.1.2', $request->clientIp()); + $this->assertEquals('192.168.1.3', $request->clientIp(), 'use remote_addr'); + $this->assertEquals('192.168.1.3', $request->clientIp(false), 'use remote_addr'); } /**