8000 [HttpFoundation] Fix missing handling of for/host/proto info from "Fo… · symfony/symfony@40c4d23 · GitHub
[go: up one dir, main page]
Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

Commit 40c4d23

Browse files
nicolas-grekasnicolas-grekas
committed
[HttpFoundation] Fix missing handling of for/host/proto info from "Forwarded" header
1 parent a19e3fe commit 40c4d23

File tree

2 files changed

+120
-58
lines changed

2 files changed

+120
-58
lines changed

‎src/Symfony/Component/HttpFoundation/Request.php

Lines changed: 68 additions & 55 deletions
Original file line numberDiff line numberDiff line change
@@ -206,6 +206,15 @@ class Request
206206

207207
protected static $requestFactory;
208208

209+
private $isForwardedValid = true;
210+
211+
private static $forwardedParams = array(
212+
self::HEADER_CLIENT_IP => 'for',
213+
self::HEADER_CLIENT_HOST => 'host',
214+
self::HEADER_CLIENT_PROTO => 'proto',
215+
self::HEADER_CLIENT_PORT => 'for',
216+
);
217+
209218
/**
210219
* Constructor.
211220
*
@@ -806,41 +815,13 @@ public function setSession(SessionInterface $session)
806815
*/
807816
public function getClientIps()
808817
{
809-
$clientIps = array();
810818
$ip = $this->server->get('REMOTE_ADDR');
811819

812820
if (!$this->isFromTrustedProxy()) {
813821
return array($ip);
814822
}
815823

816-
$hasTrustedForwardedHeader = self::$trustedHeaders[self::HEADER_FORWARDED] && $this->headers->has(self::$trustedHeaders[self::HEADER_FORWARDED]);
817-
$hasTrustedClientIpHeader = self::$trustedHeaders[self::HEADER_CLIENT_IP] && $this->headers->has(self::$trustedHeaders[self::HEADER_CLIENT_IP]);
818-
819-
if ($hasTrustedForwardedHeader) {
820-
$forwardedHeader = $this->headers->get(self::$trustedHeaders[self::HEADER_FORWARDED]);
821-
preg_match_all('{(for)=("?\[?)([a-z0-9\.:_\-/]*)}', $forwardedHeader, $matches);
822-
$forwardedClientIps = $matches[3];
823-
824-
$forwardedClientIps = $this->normalizeAndFilterClientIps($forwardedClientIps, $ip);
825-
$clientIps = $forwardedClientIps;
826-
}
827-
828-
if ($hasTrustedClientIpHeader) {
829-
$xForwardedForClientIps = array_map('trim', explode(',', $this->headers->get(self::$trustedHeaders[self::HEADER_CLIENT_IP])));
830-
831-
$xForwardedForClientIps = $this->normalizeAndFilterClientIps($xForwardedForClientIps, $ip);
832-
$clientIps = $xForwardedForClientIps;
833-
}
834-
835-
if ($hasTrustedForwardedHeader && $hasTrustedClientIpHeader && $forwardedClientIps !== $xForwardedForClientIps) {
836-
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.');
837-
}
838-
839-
if (!$hasTrustedForwardedHeader && !$hasTrustedClientIpHeader) {
840-
return $this->normalizeAndFilterClientIps(array(), $ip);
841-
}
842-
843-
return $clientIps;
824+
return $this->getTrustedValues(self::HEADER_CLIENT_IP, $ip) ?: array($ip);
844825
}
845826

846827
/**
@@ -966,31 +947,25 @@ public function getScheme()
966947
*/
967948
public function getPort()
968949
{
969-
if ($this->isFromTrustedProxy()) {
970-
if (self::$trustedHeaders[self::HEADER_CLIENT_PORT] && $port = $this->headers->get(self::$trustedHeaders[self::HEADER_CLIENT_PORT])) {
971-
return $port;
972-
}
973-
974-
if (self::$trustedHeaders[self::HEADER_CLIENT_PROTO] && 'https' === $this->headers->get(self::$trustedHeaders[self::HEADER_CLIENT_PROTO], 'http')) {
975-
return 443;
976-
}
950+
if ($this->isFromTrustedProxy() && $host = $this->getTrustedValues(self::HEADER_CLIENT_PORT)) {
951+
$host = $host[0];
952+
} elseif ($this->isFromTrustedProxy() && $host = $this->getTrustedValues(self::HEADER_CLIENT_HOST)) {
953+
$host = $host[0];
954+
} elseif (!$host = $this->headers->get('HOST')) {
955+
return $this->server->get('SERVER_PORT');
977956
}
978957

979-
if ($host = $this->headers->get('HOST')) {
980-
if ($host[0] === '[') {
981-
$pos = strpos($host, ':', strrpos($host, ']'));
982-
} else {
983-
$pos = strrpos($host, ':');
984-
}
985-
986-
if (false !== $pos) {
987-
return (int) substr($host, $pos + 1);
988-
}
958+
if ($host[0] === '[') {
959+
$pos = strpos($host, ':', strrpos($host, ']'));
960+
} else {
961+
$pos = strrpos($host, ':');
962+
}
989963

990-
return 'https' === $this->getScheme() ? 443 : 80;
964+
if (false !== $pos) {
965+
return (int) substr($host, $pos + 1);
991966
}
992967

993-
return $this->server->get('SERVER_PORT');
968+
return 'https' === $this->getScheme() ? 443 : 80;
994969
}
995970

996971
/**
@@ -1190,8 +1165,8 @@ public function getQueryString()
11901165
*/
11911166
public function isSecure()
11921167
{
1193-
if ($this->isFromTrustedProxy() && self::$trustedHeaders[self::HEADER_CLIENT_PROTO] && $proto = $this->headers->get(self::$trustedHeaders[self::HEADER_CLIENT_PROTO])) {
1194-
return in_array(strtolower(current(explode(',', $proto))), array('https', 'on', 'ssl', '1'));
1168+
if ($this->isFromTrustedProxy() && $proto = $this->getTrustedValues(self::HEADER_CLIENT_PROTO)) {
1169+
return in_array(strtolower($proto[0]), array('https', 'on', 'ssl', '1'), true);
11951170
}
11961171

11971172
$https = $this->server->get('HTTPS');
@@ -1216,10 +1191,8 @@ public function isSecure()
12161191
*/
12171192
public function getHost()
12181193
{
1219-
if ($this->isFromTrustedProxy() && self::$trustedHeaders[self::HEADER_CLIENT_HOST] && $host = $this->headers->get(self::$trustedHeaders[self::HEADER_CLIENT_HOST])) {
1220-
$elements = explode(',', $host);
1221-
1222-
$host = $elements[count($elements) - 1];
1194+
if ($this->isFromTrustedProxy() && $host = $this->getTrustedValues(self::HEADER_CLIENT_HOST)) {
1195+
$host = $host[0];
12231196
} elseif (!$host = $this->headers->get('HOST')) {
12241197
if (!$host = $this->server->get('SERVER_NAME')) {
12251198
$host = $this->server->get('SERVER_ADDR', '');
@@ -1948,8 +1921,48 @@ private function isFromTrustedProxy()
19481921
return self::$trustedProxies && IpUtils::checkIp($this->server->get('REMOTE_ADDR'), self::$trustedProxies);
19491922
}
19501923

1924+
private function getTrustedValues($type, $ip = null)
1925+
{
1926+
$clientValues = array();
1927+
$forwardedValues = array();
1928+
1929+
if (self::$trustedHeaders[$type] && $this->headers->has(self::$trustedHeaders[$type])) {
1930+
foreach (explode(',', $this->headers->get(self::$trustedHeaders[$type])) as $v) {
1931+
$clientValues[] = (self::HEADER_CLIENT_PORT === $type ? '0.0.0.0:' : '').trim($v);
1932+
}
1933+
}
1934+
1935+
if (self::$trustedHeaders[self::HEADER_FORWARDED] && $this->headers->has(self::$trustedHeaders[self::HEADER_FORWARDED])) {
1936+
$forwardedValues = $this->headers->get(self::$trustedHeaders[self::HEADER_FORWARDED]);
1937+
$forwardedValues = preg_match_all(sprintf('{(?:%s)=(?:"?\[?)([a-zA-Z0-9\.:_\-/]*+)}', self::$forwardedParams[$type]), $forwardedValues, $matches) ? $matches[1] : array();
1938+
}
1939+
1940+
if (null !== $ip) {
1941+
$clientValues = $this->normalizeAndFilterClientIps($clientValues, $ip);
1942+
$forwardedValues = $this->normalizeAndFilterClientIps($forwardedValues, $ip);
1943+
}
1944+
1945+
if ($forwardedValues === $clientValues || !$clientValues) {
1946+
return $forwardedValues;
1947+
}
1948+
1949+
if (!$forwardedValues) {
1950+
return $clientValues;
1951+
}
1952+
1953+
if (!$this->isForwardedValid) {
1954+
return null !== $ip ? array('0.0.0.0', $ip) : array();
1955+
}
1956+
$this->isForwardedValid = false;
1957+
1958+
throw new ConflictingHeadersException(sprintf('The request has both a trusted "%s" header and a trusted "%s" header, conflicting with each other. You should either configure your proxy to remove one of them, or configure your project to distrust the offending one.', self::$trustedHeaders[self::HEADER_FORWARDED], self::$trustedHeaders[$type]));
1959+
}
1960+
19511961
private function normalizeAndFilterClientIps(array $clientIps, $ip)
19521962
{
1963+
if (!$clientIps) {
1964+
return array();
1965+
}
19531966
$clientIps[] = $ip; // Complete the IP chain with the IP the request actually came from
19541967
$firstTrustedIp = null;
19551968

‎src/Symfony/Component/HttpFoundation/Tests/RequestTest.php

Lines changed: 52 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1626,12 +1626,12 @@ private function getRequestInstanceForClientIpsForwardedTests($remoteAddr, $http
16261626
return $request;
16271627
}
16281628

1629-
public function testTrustedProxies()
1629+
public function testTrustedProxiesXForwardedFor()
16301630
{
16311631
$request = Request::create('http://example.com/');
16321632
$request->server->set('REMOTE_ADDR', '3.3.3.3');
16331633
$request->headers->set('X_FORWARDED_FOR', '1.1.1.1, 2.2.2.2');
1634-
$request->headers->set('X_FORWARDED_HOST', 'foo.example.com, real.example.com:8080');
1634+
$request->headers->set('X_FORWARDED_HOST', 'foo.example.com:1234, real.example.com:8080');
16351635
$request->headers->set('X_FORWARDED_PROTO', 'https');
16361636
$request->headers->set('X_FORWARDED_PORT', 443);
16371637
$request->headers->set('X_MY_FOR', '3.3.3.3, 4.4.4.4');
@@ -1662,7 +1662,7 @@ public function testTrustedProxies()
16621662
// trusted proxy via setTrustedProxies()
16631663
Request::setTrustedProxies(array('3.3.3.3', '2.2.2.2'));
16641664
$this->assertEquals('1.1.1.1', $request->getClientIp());
1665-
$this->assertEquals('real.example.com', $request->getHost());
1665+
$this->assertEquals('foo.example.com', $request->getHost());
16661666
$this->assertEquals(443, $request->getPort());
16671667
$this->assertTrue($request->isSecure());
16681668

@@ -1709,6 +1709,55 @@ public function testTrustedProxies()
17091709
Request::setTrustedHeaderName(Request::HEADER_CLIENT_PROTO, 'X_FORWARDED_PROTO');
17101710
}
17111711

1712+
public function testTrustedProxiesForwarded()
1713+
{
1714+
$request = Request::create('http://example.com/');
1715+
$request->server->set('REMOTE_ADDR', '3.3.3.3');
1716+
$request->headers->set('FORWARDED', 'for=1.1.1.1:443, host=foo.example.com:1234, proto=https, for=2.2.2.2, host=real.example.com:8080');
1717+
1718+
// no trusted proxies
1719+
$this->assertEquals('3.3.3.3', $request->getClientIp());
1720+
$this->assertEquals('example.com', $request->getHost());
1721+
$this->assertEquals(80, $request->getPort());
1722+
$this->assertFalse($request->isSecure());
1723+
1724+
// disabling proxy trusting
1725+
Request::setTrustedProxies(array());
1726+
$this->assertEquals('3.3.3.3', $request->getClientIp());
1727+
$this->assertEquals('example.com', $request->getHost());
1728+
$this->assertEquals(80, $request->getPort());
1729+
$this->assertFalse($request->isSecure());
1730+
1731+
// request is forwarded by a non-trusted proxy
1732+
Request::setTrustedProxies(array('2.2.2.2'));
1733+
$this->assertEquals('3.3.3.3', $request->getClientIp());
1734+
$this->assertEquals('example.com', $request->getHost());
1735+
$this->assertEquals(80, $request->getPort());
1736+
$this->assertFalse($request->isSecure());
1737+
1738+
// trusted proxy via setTrustedProxies()
1739+
Request::setTrustedProxies(array('3.3.3.3', '2.2.2.2'));
1740+
$this->assertEquals('1.1.1.1', $request->getClientIp());
1741+
$this->assertEquals('foo.example.com', $request->getHost());
1742+
$this->assertEquals(443, $request->getPort());
1743+
$this->assertTrue($request->isSecure());
1744+
1745+
// trusted proxy via setTrustedProxies()
1746+
Request::setTrustedProxies(array('3.3.3.4', '2.2.2.2'));
1747+
$this->assertEquals('3.3.3.3', $request->getClientIp());
1748+
$this->assertEquals('example.com', $request->getHost());
1749+
$this->assertEquals(80, $request->getPort());
1750+
$this->assertFalse($request->isSecure());
1751+
1752+
// check various X_FORWARDED_PROTO header values
1753+
Request::setTrustedProxies(array('3.3.3.3', '2.2.2.2'));
1754+
$request->headers->set('FORWARDED', 'proto=ssl');
1755+
$this->assertTrue($request->isSecure());
1756+
1757+
$request->headers->set('FORWARDED', 'proto=https, proto=http');
1758+
$this->assertTrue($request->isSecure());
1759+
}
1760+
17121761
/**
17131762
* @expectedException \InvalidArgumentException
17141763
*/

0 commit comments

Comments
 (0)
0