Skip to content

Commit

Permalink
bug #16736 [Request] Ignore invalid IP addresses sent by proxies (Gro…
Browse files Browse the repository at this point in the history
…mNaN)

This PR was submitted for the 2.8 branch but it was merged into the 2.3 branch instead (closes #16736).

Discussion
----------

[Request] Ignore invalid IP addresses sent by proxies

| Q             | A
| ------------- | ---
| Bug fix?      | yes
| New feature?  | no
| BC breaks?    | ?
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | #15525
| License       | MIT
| Doc PR        | n/a

The [RFC 7239](https://tools.ietf.org/html/rfc7239#section-6.2) allows other values that IP addresses to be passed in `Forwarded`header and [Nginx can add `unknown` to the `X-Forwarded-For`header](http://www.squid-cache.org/Doc/config/forwarded_for/).

To prevent these invalid IP addresses from being returned as "Client IP", this PR ensure that they are excluded.

Commits
-------

6578806 [Request] Ignore invalid IP addresses sent by proxies
  • Loading branch information
fabpot committed Jan 25, 2016
2 parents ec26f6e + 6578806 commit 50b48f6
Show file tree
Hide file tree
Showing 2 changed files with 14 additions and 3 deletions.
14 changes: 11 additions & 3 deletions src/Symfony/Component/HttpFoundation/Request.php
Expand Up @@ -769,8 +769,7 @@ public function getClientIps()

$clientIps = array_map('trim', explode(',', $this->headers->get(self::$trustedHeaders[self::HEADER_CLIENT_IP])));
$clientIps[] = $ip; // Complete the IP chain with the IP the request actually came from

$ip = $clientIps[0]; // Fallback to this when the client IP falls into the range of trusted proxies
$firstTrustedIp = null;

// Eliminate all IPs from the forwarded IP chain which are trusted proxies
foreach ($clientIps as $key => $clientIp) {
Expand All @@ -779,13 +778,22 @@ public function getClientIps()
$clientIps[$key] = $clientIp = $match[1];
}

if (!filter_var($clientIp, FILTER_VALIDATE_IP)) {
unset($clientIps[$key]);
}

if (IpUtils::checkIp($clientIp, self::$trustedProxies)) {
unset($clientIps[$key]);

// Fallback to this when the client IP falls into the range of trusted proxies
if (null === $firstTrustedIp) {
$firstTrustedIp = $clientIp;
}
}
}

// Now the IP chain contains only untrusted proxies and the client IP
return $clientIps ? array_reverse($clientIps) : array($ip);
return $clientIps ? array_reverse($clientIps) : array($firstTrustedIp);
}

/**
Expand Down
3 changes: 3 additions & 0 deletions src/Symfony/Component/HttpFoundation/Tests/RequestTest.php
Expand Up @@ -863,6 +863,9 @@ public function testGetClientIpsProvider()

// client IP with port
array(array('88.88.88.88'), '127.0.0.1', '88.88.88.88:12345, 127.0.0.1', array('127.0.0.1')),

// invalid forwarded IP is ignored
array(array('88.88.88.88'), '127.0.0.1', 'unknown,88.88.88.88', array('127.0.0.1')),
);
}

Expand Down

0 comments on commit 50b48f6

Please sign in to comment.