8000 bug #18688 [HttpFoundation] Warning when request has both Forwarded a… · symfony/http-foundation@e8fa5fe · GitHub
[go: up one dir, main page]
Skip to content

Commit e8fa5fe

Browse files
fabpotfabpot
committed
bug #18688 [HttpFoundation] Warning when request has both Forwarded and X-Forwarded-For (magnusnordlander)
This PR was squashed before being merged into the 2.7 branch (closes #18688). Discussion ---------- [HttpFoundation] Warning when request has both Forwarded and X-Forwarded-For | Q | A | ------------- | --- | Branch? | 2.7 | Bug fix? | yes | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | | License | MIT | Doc PR | symfony/symfony-docs#6526 Emit a warning when a request has both a trusted Forwarded header and a trusted X-Forwarded-For header, as this is most likely a misconfiguration which causes security issues. Commits ------- ee8842f [HttpFoundation] Warning when request has both Forwarded and X-Forwarded-For
2 parents 10f89ef + 8d40e27 commit e8fa5fe

File tree

3 files changed

+142
-26
lines changed

3 files changed

+142
-26
lines changed

‎Exception/ConflictingHeadersException.php

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
<?php
2+
3+
/*
4+
* This file is part of the Symfony package.
5+
*
6+
* (c) Fabien Potencier <fabien@symfony.com>
7+
*
8+
* For the full copyright and license information, please view the LICENSE
9+
* file that was distributed with this source code.
10+
*/
11+
12+
namespace Symfony\Component\HttpFoundation\Exception;
13+
14+
/**
15+
* The HTTP request contains headers with conflicting information.
16+
*
17+
* This exception should trigger an HTTP 400 response in your application code.
18+
*
19+
* @author Magnus Nordlander <magnus@fervo.se>
20+
*/
21+
class ConflictingHeadersException extends \RuntimeException
22+
{
23+
}

‎Request.php

Lines changed: 51 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111

1212
namespace Symfony\Component\HttpFoundation;
1313

14+
use Symfony\Component\HttpFoundation\Exception\ConflictingHeadersException;
1415
use Symfony\Component\HttpFoundation\Session\SessionInterface;
1516

1617
/**
@@ -805,41 +806,34 @@ public function getClientIps()
805806
return array($ip);
806807
}
807808

808-
if (self::$trustedHeaders[self::HEADER_FORWARDED] && $this->headers->has(self::$trustedHeaders[self::HEADER_FORWARDED])) {
809+
$hasTrustedForwardedHeader = self::$trustedHeaders[self::HEADER_FORWARDED] && $this->headers->has(self::$trustedHeaders[self::HEADER_FORWARDED]);
810+
$hasTrustedClientIpHeader = self::$trustedHeaders[self::HEADER_CLIENT_IP] && $this->headers->has(self::$trustedHeaders[self::HEADER_CLIENT_IP]);
811+
812+
if ($hasTrustedForwardedHeader) {
809813
$forwardedHeader = $this->headers->get(self::$trustedHeaders[self::HEADER_FORWARDED]);
810814
preg_match_all('{(for)=("?\[?)([a-z0-9\.:_\-/]*)}', $forwardedHeader, $matches);
811-
$clientIps = $matches[3];
812-
} elseif (self::$trustedHeaders[self::HEADER_CLIENT_IP] && $this->headers->has(self::$trustedHeaders[self::HEADER_CLIENT_IP])) {
813-
$clientIps = array_map('trim', explode(',', $this->headers->get(self::$trustedHeaders[self::HEADER_CLIENT_IP])));
814-
}
815+
$forwardedClientIps = $matches[3];
815816

816-
$clientIps[] = $ip; // Complete the IP chain with the IP the request actually came from
817-
$firstTrustedIp = null;
818-
819-
foreach ($clientIps as $key => $clientIp) {
820-
// Remove port (unfortunately, it does happen)
821-
if (preg_match('{((?:\d+\.){3}\d+)\:\d+}', $clientIp, $match)) {
822-
$clientIps[$key] = $clientIp = $match[1];
823-
}
817+
$forwardedClientIps = $this->normalizeAndFilterClientIps($forwardedClientIps, $ip);
818+
$clientIps = $forwardedClientIps;
819+
}
824820

825-
if (!filter_var($clientIp, FILTER_VALIDATE_IP)) {
826-
unset($clientIps[$key]);
821+
if ($hasTrustedClientIpHeader) {
822+
$xForwardedForClientIps = array_map('trim', explode(',', $this->headers->get(self::$trustedHeaders[self::HEADER_CLIENT_IP])));
827823

828-
continue;
829-
}
824+
$xForwardedForClientIps = $this->normalizeAndFilterClientIps($xForwardedForClientIps, $ip);
825+
$clientIps = $xForwardedForClientIps;
826+
}
830827

831-
if (IpUtils::checkIp($clientIp, self::$trustedProxies)) {
832-
unset($clientIps[$key]);
828+
if ($hasTrustedForwardedHeader && $hasTrustedClientIpHeader && $forwardedClientIps !== $xForwardedForClientIps) {
829+
throw new ConflictingHeadersException('The request has both a trusted Forwarded header and a trusted Client IP header, conflicting with each other with regards to the originating IP addresses of the request. This is the result of a misconfiguration. You should either configure your proxy only to send one of these headers, or configure Symfony to distrust one of them.');
830+
}
833831

834-
// Fallback to this when the client IP falls into the range of trusted proxies
835-
if (null === $firstTrustedIp) {
836-
$firstTrustedIp = $clientIp;
837-
}
838-
}
832+
if (!$hasTrustedForwardedHeader && !$hasTrustedClientIpHeader) {
833+
return $this->normalizeAndFilterClientIps(array(), $ip);
839834
}
840835

841-
// Now the IP chain contains only untrusted proxies and the client IP
842-
return $clientIps ? array_reverse($clientIps) : array($firstTrustedIp);
836+
return $clientIps;
843837
}
844838

845839
/**
@@ -1930,4 +1924,35 @@ private function isFromTrustedProxy()
19301924
{
19311925
return self::$trustedProxies && IpUtils::checkIp($this->server->get('REMOTE_ADDR'), self::$trustedProxies);
19321926
}
1927+
1928+
private function normalizeAndFilterClientIps(array $clientIps, $ip)
1929+
{
1930+
$clientIps[] = $ip; // Complete the IP chain with the IP the request actually came from
1931+
$firstTrustedIp = null;
1932+
1933+
foreach ($clientIps as $key => $clientIp) {
1934+
// Remove port (unfortunately, it does happen)
1935+
if (preg_match('{((?:\d+\.){3}\d+)\:\d+}', $clientIp, $match)) {
1936+
$clientIps[$key] = $clientIp = $match[1];
1937+
}
1938+
1939+
if (!filter_var($clientIp, FILTER_VALIDATE_IP)) {
1940+
unset($clientIps[$key]);
1941+
1942+
continue;
1943+
}
1944+
1945+
if (IpUtils::checkIp($clientIp, self::$trustedProxies)) {
1946+
unset($clientIps[$key]);
1947+
1948+
// Fallback to this when the client IP falls into the range of trusted proxies
1949+
if (null === $firstTrustedIp) {
1950+
$firstTrustedIp = $clientIp;
1951+
}
1952+
}
1953+
}
1954+
1955+
// Now the IP chain contains only untrusted proxies and the client IP
1956+
return $clientIps ? array_reverse($clientIps) : array($firstTrustedIp);
1957+
}
19331958
}

‎Tests/RequestTest.php

Lines changed: 68 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -923,6 +923,74 @@ public function testGetClientIpsProvider()
923923
);
924924
}
925925

926+
/**
927+
* @expectedException \Symfony\Component\HttpFoundation\Exception\ConflictingHeadersException
928+
* @dataProvider testGetClientIpsWithConflictingHeadersProvider
929+
*/
930+
public function testGetClientIpsWithConflictingHeaders($httpForwarded, $httpXForwardedFor)
931+
{
932+
$request = new Request();
933+
934+
$server = array(
935+
'REMOTE_ADDR' => '88.88.88.88',
936+
'HTTP_FORWARDED' => $httpForwarded,
937+
'HTTP_X_FORWARDED_FOR' => $httpXForwardedFor,
938+
);
939+
940+
Request::setTrustedProxies(array('88.88.88.88'));
941+
942+
$request->initialize(array(), array(), array(), array(), array(), $server);
943+
944+
$request->getClientIps();
945+
}
946+
947+
public function testGetClientIpsWithConflictingHeadersProvider()
948+
{
949+
// $httpForwarded $httpXForwardedFor
950+
return array(
951+
array('for=87.65.43.21', '192.0.2.60'),
952+
array('for=87.65.43.21, for=192.0.2.60', '192.0.2.60'),
953+
array('for=192.0.2.60', '192.0.2.60,87.65.43.21'),
954+
array('for="::face", for=192.0.2.60', '192.0.2.60,192.0.2.43'),
955+
array('for=87.65.43.21, for=192.0.2.60', '192.0.2.60,87.65.43.21'),
956+
);
957+
}
958+
959+
/**
960+
* @dataProvider testGetClientIpsWithAgreeingHeadersProvider
961+
*/
962+
public function testGetClientIpsWithAgreeingHeaders($httpForwarded, $httpXForwardedFor)
963+
{
964+
$request = new Request();
965+
966+
$server = array(
967+
'REMOTE_ADDR' => '88.88.88.88',
968+
'HTTP_FORWARDED' => $httpForwarded,
969+
'HTTP_X_FORWARDED_FOR' => $httpXForwardedFor,
970+
);
971+
972+
Request::setTrustedProxies(array('88.88.88.88'));
973+
974+
$request->initialize(array(), array(), array(), array(), array(), $server);
975+
976+
$request->getClientIps();
977+
978+
Request::setTrustedProxies(array());
979+
}
980+
981+
public function testGetClientIpsWithAgreeingHeadersProvider()
982+
{
983+
// $httpForwarded $httpXForwardedFor
984+
return array(
985+
array('for="192.0.2.60"', '192.0.2.60'),
986+
array('for=192.0.2.60, for=87.65.43.21', '192.0.2.60,87.65.43.21'),
987+
array('for="[::face]", for=192.0.2.60', '::face,192.0.2.60'),
988+
array('for="192.0.2.60:80"', '192.0.2.60'),
989+
array('for=192.0.2.60;proto=http;by=203.0.113.43', '192.0.2.60'),
990+
array('for="[2001:db8:cafe::17]:4711"', '2001:db8:cafe::17'),
991+
);
992+
}
993+
926994
public function testGetContentWorksTwiceInDefaultMode()
927995
{
928996
$req = new Request();

0 commit comments

Comments
 (0)
0