Merge pull request #88 from HonzaCZ/master

RequestFactory: Fixed possible remoteAddr spoofing (issue #87)
nette · Mar 29, 2016 · c3a9a20 · c3a9a20
2 parents dca62fe + cae5d68
commit c3a9a20
Show file tree

Hide file tree

Showing 2 changed files with 33 additions and 6 deletions.
diff --git a/src/Http/RequestFactory.php b/src/Http/RequestFactory.php
@@ -203,11 +203,20 @@ public function createHttpRequest()
 			}
 
 			if (!empty($_SERVER['HTTP_X_FORWARDED_FOR'])) {
-				$remoteAddr = trim(explode(',', $_SERVER['HTTP_X_FORWARDED_FOR'])[0]);
+				$xForwardedForWithoutProxies = array_filter(explode(',', $_SERVER['HTTP_X_FORWARDED_FOR']), function ($ip) {
+					return !array_filter($this->proxies, function ($proxy) use ($ip) {
+						return Helpers::ipMatch(trim($ip), $proxy);
+					});
+				});
+				$remoteAddr = trim(end($xForwardedForWithoutProxies));
+				$xForwardedForRealIpKey = key($xForwardedForWithoutProxies);
 			}
 
-			if (!empty($_SERVER['HTTP_X_FORWARDED_HOST'])) {
-				$remoteHost = trim(explode(',', $_SERVER['HTTP_X_FORWARDED_HOST'])[0]);
+			if (isset($xForwardedForRealIpKey) && !empty($_SERVER['HTTP_X_FORWARDED_HOST'])) {
+				$xForwardedHost = explode(',', $_SERVER['HTTP_X_FORWARDED_HOST']);
+				if (isset($xForwardedHost[$xForwardedForRealIpKey])) {
+					$remoteHost = trim($xForwardedHost[$xForwardedForRealIpKey]);
+				}
 			}
 		}
 

diff --git a/tests/Http/RequestFactory.proxy.phpt b/tests/Http/RequestFactory.proxy.phpt
@@ -15,8 +15,8 @@ test(function () {
 	$_SERVER = [
 		'REMOTE_ADDR' => '127.0.0.3',
 		'REMOTE_HOST' => 'localhost',
-		'HTTP_X_FORWARDED_FOR' => '23.75.345.200, 10.0.0.1',
-		'HTTP_X_FORWARDED_HOST' => 'otherhost, anotherhost',
+		'HTTP_X_FORWARDED_FOR' => '23.75.45.200',
+		'HTTP_X_FORWARDED_HOST' => 'otherhost',
 	];
 
 	$factory = new RequestFactory;
@@ -25,6 +25,24 @@ test(function () {
 	Assert::same('localhost', $factory->createHttpRequest()->getRemoteHost());
 
 	$factory->setProxy('127.0.0.1/8');
-	Assert::same('23.75.345.200', $factory->createHttpRequest()->getRemoteAddress());
+	Assert::same('23.75.45.200', $factory->createHttpRequest()->getRemoteAddress());
 	Assert::same('otherhost', $factory->createHttpRequest()->getRemoteHost());
 });
+
+test(function () {
+	$_SERVER = [
+		'REMOTE_ADDR' => '10.0.0.2', //proxy2
+		'REMOTE_HOST' => 'proxy2',
+		'HTTP_X_FORWARDED_FOR' => '123.123.123.123, 172.16.0.1, 10.0.0.1',
+		'HTTP_X_FORWARDED_HOST' => 'fake, real, proxy1',
+	];
+
+	$factory = new RequestFactory;
+	$factory->setProxy('10.0.0.0/24');
+	Assert::same('172.16.0.1', $factory->createHttpRequest()->getRemoteAddress());
+	Assert::same('real', $factory->createHttpRequest()->getRemoteHost());
+
+	$factory->setProxy(['10.0.0.1', '10.0.0.2']);
+	Assert::same('172.16.0.1', $factory->createHttpRequest()->getRemoteAddress());
+	Assert::same('real', $factory->createHttpRequest()->getRemoteHost());
+});