Skip to content

Commit

Permalink
Merge pull request from GHSA-ghw3-5qvm-3mqc
Browse files Browse the repository at this point in the history 
fix: proxyIPs
  • Loading branch information
@MGatner
MGatner authored Dec 22, 2022
2 parents c946f2c + fbc46bb commit 5ca8c99
Show file tree
Hide file tree
Showing 11 changed files with 314 additions and 112 deletions.
17 changes: 10 additions & 7 deletions app/Config/App.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -332,18 +332,21 @@ class App extends BaseConfig
*
* If your server is behind a reverse proxy, you must whitelist the proxy
* IP addresses from which CodeIgniter should trust headers such as
* HTTP_X_FORWARDED_FOR and HTTP_CLIENT_IP in order to properly identify
* X-Forwarded-For or Client-IP in order to properly identify
* the visitor's IP address.
*
* You can use both an array or a comma-separated list of proxy addresses,
* as well as specifying whole subnets. Here are a few examples:
* You need to set a proxy IP address or IP address with subnets and
* the HTTP header for the client IP address.
*
* Comma-separated: '10.0.1.200,192.168.5.0/24'
* Array: ['10.0.1.200', '192.168.5.0/24']
* Here are some examples:
* [
* '10.0.1.200' => 'X-Forwarded-For',
* '192.168.5.0/24' => 'X-Real-IP',
* ]
*
* @var string|string[]
* @var array<string, string>
*/
public $proxyIPs = '';
public $proxyIPs = [];

/**
* --------------------------------------------------------------------------
Expand Down
5 changes: 0 additions & 5 deletions phpstan-baseline.neon.dist
View file
Original file line number Diff line number Diff line change
Expand Up @@ -500,11 +500,6 @@ parameters:
count: 1
path: system/HTTP/RedirectResponse.php

-
message: "#^Property CodeIgniter\\\\HTTP\\\\Request\\:\\:\\$proxyIPs \\(array\\|string\\) on left side of \\?\\? is not nullable\\.$#"
count: 1
path: system/HTTP/Request.php

-
message: "#^Property CodeIgniter\\\\HTTP\\\\Request\\:\\:\\$uri \\(CodeIgniter\\\\HTTP\\\\URI\\) in empty\\(\\) is not falsy\\.$#"
count: 1
Expand Down
2 changes: 1 addition & 1 deletion system/HTTP/Request.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ class Request extends Message implements MessageInterface, RequestInterface
/**
* Proxy IPs
*
* @var array|string
* @var array<string, string>
*
* @deprecated Check the App config directly
*/
Expand Down
154 changes: 89 additions & 65 deletions system/HTTP/RequestTrait.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@

namespace CodeIgniter\HTTP;

use CodeIgniter\Exceptions\ConfigException;
use CodeIgniter\Validation\FormatRules;

/**
Expand Down Expand Up @@ -43,7 +44,9 @@ trait RequestTrait
/**
* Gets the user's IP address.
*
* @return string IP address
* @return string IP address if it can be detected, or empty string.
* If the IP address is not a valid IP address,
* then will return '0.0.0.0'.
*/
public function getIPAddress(): string
{
Expand All @@ -59,93 +62,86 @@ public function getIPAddress(): string
/**
* @deprecated $this->proxyIPs property will be removed in the future
*/
// @phpstan-ignore-next-line
$proxyIPs = $this->proxyIPs ?? config('App')->proxyIPs;
if (! empty($proxyIPs) && ! is_array($proxyIPs)) {
$proxyIPs = explode(',', str_replace(' ', '', $proxyIPs));
if (! empty($proxyIPs)) {
// @phpstan-ignore-next-line
if (! is_array($proxyIPs) || is_int(array_key_first($proxyIPs))) {
throw new ConfigException(
'You must set an array with Proxy IP address key and HTTP header name value in Config\App::$proxyIPs.'
);
}
}

$this->ipAddress = $this->getServer('REMOTE_ADDR');

if ($proxyIPs) {
foreach (['x-forwarded-for', 'client-ip', 'x-client-ip', 'x-cluster-client-ip'] as $header) {
$spoof = null;
$headerObj = $this->header($header);

if ($headerObj !== null) {
$spoof = $headerObj->getValue();

// Some proxies typically list the whole chain of IP
// addresses through which the client has reached us.
// e.g. client_ip, proxy_ip1, proxy_ip2, etc.
sscanf($spoof, '%[^,]', $spoof);

if (! $ipValidator($spoof)) {
$spoof = null;
} else {
break;
}
}
}

if ($spoof) {
foreach ($proxyIPs as $proxyIP) {
// Check if we have an IP address or a subnet
if (strpos($proxyIP, '/') === false) {
// An IP address (and not a subnet) is specified.
// We can compare right away.
if ($proxyIP === $this->ipAddress) {
// @TODO Extract all this IP address logic to another class.
foreach ($proxyIPs as $proxyIP => $header) {
// Check if we have an IP address or a subnet
if (strpos($proxyIP, '/') === false) {
// An IP address (and not a subnet) is specified.
// We can compare right away.
if ($proxyIP === $this->ipAddress) {
$spoof = $this->getClientIP($header);

if ($spoof !== null) {
$this->ipAddress = $spoof;
break;
}

continue;
}

// We have a subnet ... now the heavy lifting begins
if (! isset($separator)) {
$separator = $ipValidator($this->ipAddress, 'ipv6') ? ':' : '.';
}
continue;
}

// If the proxy entry doesn't match the IP protocol - skip it
if (strpos($proxyIP, $separator) === false) {
continue;
}
// We have a subnet ... now the heavy lifting begins
if (! isset($separator)) {
$separator = $ipValidator($this->ipAddress, 'ipv6') ? ':' : '.';
}

// Convert the REMOTE_ADDR IP address to binary, if needed
if (! isset($ip, $sprintf)) {
if ($separator === ':') {
// Make sure we're have the "full" IPv6 format
$ip = explode(':', str_replace('::', str_repeat(':', 9 - substr_count($this->ipAddress, ':')), $this->ipAddress));
// If the proxy entry doesn't match the IP protocol - skip it
if (strpos($proxyIP, $separator) === false) {
continue;
}

for ($j = 0; $j < 8; $j++) {
$ip[$j] = intval($ip[$j], 16);
}
// Convert the REMOTE_ADDR IP address to binary, if needed
if (! isset($ip, $sprintf)) {
if ($separator === ':') {
// Make sure we're having the "full" IPv6 format
$ip = explode(':', str_replace('::', str_repeat(':', 9 - substr_count($this->ipAddress, ':')), $this->ipAddress));

$sprintf = '%016b%016b%016b%016b%016b%016b%016b%016b';
} else {
$ip = explode('.', $this->ipAddress);
$sprintf = '%08b%08b%08b%08b';
for ($j = 0; $j < 8; $j++) {
$ip[$j] = intval($ip[$j], 16);
}

$ip = vsprintf($sprintf, $ip);
$sprintf = '%016b%016b%016b%016b%016b%016b%016b%016b';
} else {
$ip = explode('.', $this->ipAddress);
$sprintf = '%08b%08b%08b%08b';
}

// Split the netmask length off the network address
sscanf($proxyIP, '%[^/]/%d', $netaddr, $masklen);
$ip = vsprintf($sprintf, $ip);
}

// Again, an IPv6 address is most likely in a compressed form
if ($separator === ':') {
$netaddr = explode(':', str_replace('::', str_repeat(':', 9 - substr_count($netaddr, ':')), $netaddr));
// Split the netmask length off the network address
sscanf($proxyIP, '%[^/]/%d', $netaddr, $masklen);

for ($i = 0; $i < 8; $i++) {
$netaddr[$i] = intval($netaddr[$i], 16);
}
} else {
$netaddr = explode('.', $netaddr);
// Again, an IPv6 address is most likely in a compressed form
if ($separator === ':') {
$netaddr = explode(':', str_replace('::', str_repeat(':', 9 - substr_count($netaddr, ':')), $netaddr));

for ($i = 0; $i < 8; $i++) {
$netaddr[$i] = intval($netaddr[$i], 16);
}
} else {
$netaddr = explode('.', $netaddr);
}

// Convert to binary and finally compare
if (strncmp($ip, vsprintf($sprintf, $netaddr), $masklen) === 0) {
$spoof = $this->getClientIP($header);

// Convert to binary and finally compare
if (strncmp($ip, vsprintf($sprintf, $netaddr), $masklen) === 0) {
if ($spoof !== null) {
$this->ipAddress = $spoof;
break;
}
Expand All @@ -160,6 +156,34 @@ public function getIPAddress(): string
return empty($this->ipAddress) ? '' : $this->ipAddress;
}

/**
* Gets the client IP address from the HTTP header.
*/
private function getClientIP(string $header): ?string
{
$ipValidator = [
new FormatRules(),
'valid_ip',
];
$spoof = null;
$headerObj = $this->header($header);

if ($headerObj !== null) {
$spoof = $headerObj->getValue();

// Some proxies typically list the whole chain of IP
// addresses through which the client has reached us.
// e.g. client_ip, proxy_ip1, proxy_ip2, etc.
sscanf($spoof, '%[^,]', $spoof);

if (! $ipValidator($spoof)) {
$spoof = null;
}
}

return $spoof;
}

/**
* Fetch an item from the $_SERVER array.
*
Expand Down
2 changes: 1 addition & 1 deletion system/Test/Mock/MockAppConfig.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ class MockAppConfig extends App
public $cookieSecure = false;
public $cookieHTTPOnly = false;
public $cookieSameSite = 'Lax';
public $proxyIPs = '';
public $proxyIPs = [];
public $CSRFTokenName = 'csrf_test_name';
public $CSRFHeaderName = 'X-CSRF-TOKEN';
public $CSRFCookieName = 'csrf_cookie_name';
Expand Down
2 changes: 1 addition & 1 deletion system/Test/Mock/MockCLIConfig.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ class MockCLIConfig extends App
public $cookieSecure = false;
public $cookieHTTPOnly = false;
public $cookieSameSite = 'Lax';
public $proxyIPs = '';
public $proxyIPs = [];
public $CSRFTokenName = 'csrf_test_name';
public $CSRFCookieName = 'csrf_cookie_name';
public $CSRFExpire = 7200;
Expand Down
Loading

0 comments on commit 5ca8c99

Please sign in to comment.