Security fix for X-Forwarded-For HTTP header

library/Zend/Http/PhpEnvironment/RemoteAddress.php

@@ -101,7 +101,7 @@ public function getIpAddress()
         if ($ip) {
             return $ip;
         }
-
+        
         // direct IP address
         if (isset($_SERVER['REMOTE_ADDR'])) {
             return $_SERVER['REMOTE_ADDR'];
@@ -113,16 +113,17 @@ public function getIpAddress()
     /**
      * Attempt to get the IP address for a proxied client
      *
+     * @see http://tools.ietf.org/html/draft-ietf-appsawg-http-forwarded-10#section-5.2
      * @return false|string
      */
     protected function getIpAddressFromProxy()
     {
-        if (!$this->useProxy) {
+        if (!$this->useProxy || !in_array($_SERVER['REMOTE_ADDR'],
+            $this->trustedProxies)) {
             return false;
         }
 
         $header = $this->proxyHeader;
-
         if (!isset($_SERVER[$header]) || empty($_SERVER[$header])) {
             return false;
         }
@@ -131,17 +132,15 @@ protected function getIpAddressFromProxy()
         $ips = explode(',', $_SERVER[$header]);
         // trim, so we can compare against trusted proxies properly
         $ips = array_map('trim', $ips);
-        // remove trusted proxy IPs
-        $ips = array_diff($ips, $this->trustedProxies);
-
-        // Any left?
-        if (empty($ips)) {
-            return false;
+        // remove trusted proxy IPs, should remain only one IP (the client)
+        $clientIp = array_diff($ips, $this->trustedProxies);
+        
+        // The client IP is always the first in the proxies list
+        if (count($clientIp) === 1 && $clientIp[0] === $ips[0]) {
+            return $clientIp[0];
         }
 
-        // Return right-most
-        $ip  = array_pop($ips);
-        return $ip;
+        return false;
     }
 
 

tests/ZendTest/Http/PhpEnvironment/RemoteAddress.php

@@ -0,0 +1,129 @@
+<?php
+/**
+ * Zend Framework (http://framework.zend.com/)
+ *
+ * @link      http://github.com/zendframework/zf2 for the canonical source repository
+ * @copyright Copyright (c) 2005-2013 Zend Technologies USA Inc. (http://www.zend.com)
+ * @license   http://framework.zend.com/license/new-bsd New BSD License
+ */
+
+namespace ZendTest\Http\PhpEnvironment;
+
+use PHPUnit_Framework_TestCase as TestCase;
+use Zend\Http\Headers;
+use Zend\Http\Header\GenericHeader;
+use Zend\Http\PhpEnvironment\RemoteAddress as RemoteAddr;
+
+class RemoteAddress extends TestCase
+{
+    /**
+     * Original environemnt
+     *
+     * @var array
+     */
+    protected $originalEnvironment;
+
+    /**
+     * Save the original environment and set up a clean one.
+     */
+    public function setUp()
+    {
+        $this->originalEnvironment = array(
+            'post'   => $_POST,
+            'get'    => $_GET,
+            'cookie' => $_COOKIE,
+            'server' => $_SERVER,
+            'env'    => $_ENV,
+            'files'  => $_FILES,
+        );
+
+        $_POST   = array();
+        $_GET    = array();
+        $_COOKIE = array();
+        $_SERVER = array();
+        $_ENV    = array();
+        $_FILES  = array();
+
+        $this->remoteAddress = new RemoteAddr();
+    }
+
+    /**
+     * Restore the original environment
+     */
+    public function tearDown()
+    {
+        $_POST   = $this->originalEnvironment['post'];
+        $_GET    = $this->originalEnvironment['get'];
+        $_COOKIE = $this->originalEnvironment['cookie'];
+        $_SERVER = $this->originalEnvironment['server'];
+        $_ENV    = $this->originalEnvironment['env'];
+        $_FILES  = $this->originalEnvironment['files'];
+    }
+
+    public function testSetGetUseProxy()
+    {
+        $this->remoteAddress->setUseProxy(false);
+        $this->assertFalse($this->remoteAddress->getUseProxy());
+    }
+
+    public function testSetGetDefaultUseProxy()
+    {
+        $this->remoteAddress->setUseProxy();
+        $this->assertTrue($this->remoteAddress->getUseProxy());
+    }
+
+    public function testSetTrustedProxies()
+    {
+        $result = $this->remoteAddress->setTrustedProxies(array(
+            '192.168.0.10', '192.168.0.1'
+        ));
+        $this->assertTrue($result instanceOf RemoteAddr);
+    }
+
+    public function testGetIpAddress()
+    {
+        $_SERVER['REMOTE_ADDR'] = '127.0.0.1';
+        $this->assertEquals('127.0.0.1', $this->remoteAddress->getIpAddress()); 
+    }
+
+    public function testGetIpAddressFromProxy()
+    {
+        $this->remoteAddress->setUseProxy(true);
+        $this->remoteAddress->setTrustedProxies(array(
+            '192.168.0.10', '10.0.0.1'
+        ));
+        $_SERVER['REMOTE_ADDR'] = '192.168.0.10';
+        $_SERVER['HTTP_X_FORWARDED_FOR'] = '8.8.8.8, 10.0.0.1';
+        $this->assertEquals('8.8.8.8', $this->remoteAddress->getIpAddress());
+    }
+
+    public function testGetIpAddressFromProxyRemoteAddressNotTrusted()
+    {
+        $this->remoteAddress->setUseProxy(true);
+        $this->remoteAddress->setTrustedProxies(array(
+            '10.0.0.1'
+        ));
+        // the REMOTE_ADDR is not in the trusted IPs, possible attack here
+        $_SERVER['REMOTE_ADDR'] = '1.1.1.1';
+        $_SERVER['HTTP_X_FORWARDED_FOR'] = '8.8.8.8, 10.0.0.1';
+        $this->assertEquals('1.1.1.1', $this->remoteAddress->getIpAddress());
+    }
+
+    /**
+     * Test to prevent attack on the HTTP_X_FORWARDED_FOR header
+     * The client IP is always the first on the left
+     *
+     * @see http://tools.ietf.org/html/draft-ietf-appsawg-http-forwarded-10#section-5.2
+     */
+    public function testGetIpAddressFromProxyFakeData()
+    {
+        $this->remoteAddress->setUseProxy(true);
+        $this->remoteAddress->setTrustedProxies(array(
+            '192.168.0.10', '10.0.0.1', '10.0.0.2'
+        ));
+        $_SERVER['REMOTE_ADDR'] = '192.168.0.10';
+        // 1.1.1.1 is the fake IP
+        $_SERVER['HTTP_X_FORWARDED_FOR'] = '8.8.8.8, 10.0.0.2, 1.1.1.1, 10.0.0.1';
+        $this->assertEquals('192.168.0.10', $this->remoteAddress->getIpAddress());
+    }
+}