diff --git a/src/Header/SetCookie.php b/src/Header/SetCookie.php index e03bd23df4..838a173910 100644 --- a/src/Header/SetCookie.php +++ b/src/Header/SetCookie.php @@ -9,7 +9,6 @@ namespace Zend\Http\Header; -use Closure; use Zend\Uri\UriFactory; /** @@ -97,7 +96,6 @@ class SetCookie implements MultipleHeaderInterface */ public static function fromString($headerLine, $bypassHeaderFieldName = false) { - /* @var $setCookieProcessor Closure */ static $setCookieProcessor = null; if ($setCookieProcessor === null) { @@ -107,7 +105,7 @@ public static function fromString($headerLine, $bypassHeaderFieldName = false) $keyValuePairs = preg_split('#;\s*#', $headerLine); foreach ($keyValuePairs as $keyValue) { - if (preg_match('#^(?[^=]+)=\s*("?)(?[^"]+)\2#',$keyValue,$matches)) { + if (preg_match('#^(?P[^=]+)=\s*("?)(?P[^"]+)\2#', $keyValue, $matches)) { $headerKey = $matches['headerKey']; $headerValue= $matches['headerValue']; } else { diff --git a/src/PhpEnvironment/RemoteAddress.php b/src/PhpEnvironment/RemoteAddress.php index b714d46df4..9687dae72a 100644 --- a/src/PhpEnvironment/RemoteAddress.php +++ b/src/PhpEnvironment/RemoteAddress.php @@ -113,16 +113,18 @@ 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 + || (isset($_SERVER['REMOTE_ADDR']) && !in_array($_SERVER['REMOTE_ADDR'], $this->trustedProxies)) + ) { return false; } $header = $this->proxyHeader; - if (!isset($_SERVER[$header]) || empty($_SERVER[$header])) { return false; } @@ -139,8 +141,12 @@ protected function getIpAddressFromProxy() return false; } - // Return right-most - $ip = array_pop($ips); + // Since we've removed any known, trusted proxy servers, the right-most + // address represents the first IP we do not know about -- i.e., we do + // not know if it is a proxy server, or a client. As such, we treat it + // as the originating IP. + // @see http://en.wikipedia.org/wiki/X-Forwarded-For + $ip = array_pop($ips); return $ip; } diff --git a/test/ClientTest.php b/test/ClientTest.php index fa622799e8..53a4614c98 100644 --- a/test/ClientTest.php +++ b/test/ClientTest.php @@ -9,7 +9,6 @@ namespace ZendTest\Http; -use ReflectionClass; use Zend\Uri\Http; use Zend\Http\Client; use Zend\Http\Cookies; diff --git a/test/PhpEnvironment/RemoteAddressTest.php b/test/PhpEnvironment/RemoteAddressTest.php new file mode 100644 index 0000000000..fd936fdff8 --- /dev/null +++ b/test/PhpEnvironment/RemoteAddressTest.php @@ -0,0 +1,151 @@ +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 first IP address from the right not representing a known proxy server; as such, we + // must treat it as a client IP. + $_SERVER['HTTP_X_FORWARDED_FOR'] = '8.8.8.8, 10.0.0.2, 1.1.1.1, 10.0.0.1'; + $this->assertEquals('1.1.1.1', $this->remoteAddress->getIpAddress()); + } + + /** + * Tests if an empty string is returned if the server variable + * REMOTE_ADDR is not set. + * + * This happens when you run a local unit test, or a PHP script with + * PHP from the command line. + */ + public function testGetIpAddressReturnsEmptyStringOnNoRemoteAddr() + { + // Store the set IP address for later use + if (isset($_SERVER['REMOTE_ADDR'])) { + $ipAddress = $_SERVER['REMOTE_ADDR']; + unset($_SERVER['REMOTE_ADDR']); + } + + $this->remoteAddress->setUseProxy(true); + $this->assertEquals('', $this->remoteAddress->getIpAddress()); + + if (isset($ipAddress)) { + $_SERVER['REMOTE_ADDR'] = $ipAddress; + } + } +}