Skip to content

Commit 5ca8c99

Browse files
MGatnerMGatner
authored
Merge pull request from GHSA-ghw3-5qvm-3mqc
fix: proxyIPs
2 parents c946f2c + fbc46bb commit 5ca8c99

File tree

11 files changed

+314
-112
lines changed

11 files changed

+314
-112
lines changed

‎app/Config/App.php‎

Lines changed: 10 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -332,18 +332,21 @@ class App extends BaseConfig
332332
*
333333
* If your server is behind a reverse proxy, you must whitelist the proxy
334334
* IP addresses from which CodeIgniter should trust headers such as
335-
* HTTP_X_FORWARDED_FOR and HTTP_CLIENT_IP in order to properly identify
335+
* X-Forwarded-For or Client-IP in order to properly identify
336336
* the visitor's IP address.
337337
*
338-
* You can use both an array or a comma-separated list of proxy addresses,
339-
* as well as specifying whole subnets. Here are a few examples:
338+
* You need to set a proxy IP address or IP address with subnets and
339+
* the HTTP header for the client IP address.
340340
*
341-
* Comma-separated: '10.0.1.200,192.168.5.0/24'
342-
* Array: ['10.0.1.200', '192.168.5.0/24']
341+
* Here are some examples:
342+
* [
343+
* '10.0.1.200' => 'X-Forwarded-For',
344+
* '192.168.5.0/24' => 'X-Real-IP',
345+
* ]
343346
*
344-
* @var string|string[]
347+
* @var array<string, string>
345348
*/
346-
public $proxyIPs = '';
349+
public $proxyIPs = [];
347350

348351
/**
349352
* --------------------------------------------------------------------------

‎phpstan-baseline.neon.dist‎

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -500,11 +500,6 @@ parameters:
500500
count: 1
501501
path: system/HTTP/RedirectResponse.php
502502

503-
-
504-
message: "#^Property CodeIgniter\\\\HTTP\\\\Request\\:\\:\\$proxyIPs \\(array\\|string\\) on left side of \\?\\? is not nullable\\.$#"
505-
count: 1
506-
path: system/HTTP/Request.php
507-
508503
-
509504
message: "#^Property CodeIgniter\\\\HTTP\\\\Request\\:\\:\\$uri \\(CodeIgniter\\\\HTTP\\\\URI\\) in empty\\(\\) is not falsy\\.$#"
510505
count: 1

‎system/HTTP/Request.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ class Request extends Message implements MessageInterface, RequestInterface
2323
/**
2424
* Proxy IPs
2525
*
26-
* @var array|string
26+
* @var array<string, string>
2727
*
2828
* @deprecated Check the App config directly
2929
*/

‎system/HTTP/RequestTrait.php‎

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

1212
namespace CodeIgniter\HTTP;
1313

14+
use CodeIgniter\Exceptions\ConfigException;
1415
use CodeIgniter\Validation\FormatRules;
1516

1617
/**
@@ -43,7 +44,9 @@ trait RequestTrait
4344
/**
4445
* Gets the user's IP address.
4546
*
46-
* @return string IP address
47+
* @return string IP address if it can be detected, or empty string.
48+
* If the IP address is not a valid IP address,
49+
* then will return '0.0.0.0'.
4750
*/
4851
public function getIPAddress(): string
4952
{
@@ -59,93 +62,86 @@ public function getIPAddress(): string
5962
/**
6063
* @deprecated $this->proxyIPs property will be removed in the future
6164
*/
65+
// @phpstan-ignore-next-line
6266
$proxyIPs = $this->proxyIPs ?? config('App')->proxyIPs;
63-
if (! empty($proxyIPs) && ! is_array($proxyIPs)) {
64-
$proxyIPs = explode(',', str_replace(' ', '', $proxyIPs));
67+
if (! empty($proxyIPs)) {
68+
// @phpstan-ignore-next-line
69+
if (! is_array($proxyIPs) || is_int(array_key_first($proxyIPs))) {
70+
throw new ConfigException(
71+
'You must set an array with Proxy IP address key and HTTP header name value in Config\App::$proxyIPs.'
72+
);
73+
}
6574
}
6675

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

6978
if ($proxyIPs) {
70-
foreach (['x-forwarded-for', 'client-ip', 'x-client-ip', 'x-cluster-client-ip'] as $header) {
71-
$spoof = null;
72-
$headerObj = $this->header($header);
73-
74-
if ($headerObj !== null) {
75-
$spoof = $headerObj->getValue();
76-
77-
// Some proxies typically list the whole chain of IP
78-
// addresses through which the client has reached us.
79-
// e.g. client_ip, proxy_ip1, proxy_ip2, etc.
80-
sscanf($spoof, '%[^,]', $spoof);
81-
82-
if (! $ipValidator($spoof)) {
83-
$spoof = null;
84-
} else {
85-
break;
86-
}
87-
}
88-
}
89-
90-
if ($spoof) {
91-
foreach ($proxyIPs as $proxyIP) {
92-
// Check if we have an IP address or a subnet
93-
if (strpos($proxyIP, '/') === false) {
94-
// An IP address (and not a subnet) is specified.
95-
// We can compare right away.
96-
if ($proxyIP === $this->ipAddress) {
79+
// @TODO Extract all this IP address logic to another class.
80+
foreach ($proxyIPs as $proxyIP => $header) {
81+
// Check if we have an IP address or a subnet
82+
if (strpos($proxyIP, '/') === false) {
83+
// An IP address (and not a subnet) is specified.
84+
// We can compare right away.
85+
if ($proxyIP === $this->ipAddress) {
86+
$spoof = $this->getClientIP($header);
87+
88+
if ($spoof !== null) {
9789
$this->ipAddress = $spoof;
9890
break;
9991
}
100-
101-
continue;
10292
}
10393

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

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

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

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

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

130-
$ip = vsprintf($sprintf, $ip);
117+
$sprintf = '%016b%016b%016b%016b%016b%016b%016b%016b';
118+
} else {
119+
$ip = explode('.', $this->ipAddress);
120+
$sprintf = '%08b%08b%08b%08b';
131121
}
132122

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

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

140-
for ($i = 0; $i < 8; $i++) {
141-
$netaddr[$i] = intval($netaddr[$i], 16);
142-
}
143-
} else {
144-
$netaddr = explode('.', $netaddr);
129+
// Again, an IPv6 address is most likely in a compressed form
130+
if ($separator === ':') {
131+
$netaddr = explode(':', str_replace('::', str_repeat(':', 9 - substr_count($netaddr, ':')), $netaddr));
132+
133+
for ($i = 0; $i < 8; $i++) {
134+
$netaddr[$i] = intval($netaddr[$i], 16);
145135
}
136+
} else {
137+
$netaddr = explode('.', $netaddr);
138+
}
139+
140+
// Convert to binary and finally compare
141+
if (strncmp($ip, vsprintf($sprintf, $netaddr), $masklen) === 0) {
142+
$spoof = $this->getClientIP($header);
146143

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

159+
/**
160+
* Gets the client IP address from the HTTP header.
161+
*/
162+
private function getClientIP(string $header): ?string
163+
{
164+
$ipValidator = [
165+
new FormatRules(),
166+
'valid_ip',
167+
];
168+
$spoof = null;
169+
$headerObj = $this->header($header);
170+
171+
if ($headerObj !== null) {
172+
$spoof = $headerObj->getValue();
173+
174+
// Some proxies typically list the whole chain of IP
175+
// addresses through which the client has reached us.
176+
// e.g. client_ip, proxy_ip1, proxy_ip2, etc.
177+
sscanf($spoof, '%[^,]', $spoof);
178+
179+
if (! $ipValidator($spoof)) {
180+
$spoof = null;
181+
}
182+
}
183+
184+
return $spoof;
185+
}
186+
163187
/**
164188
* Fetch an item from the $_SERVER array.
165189
*

‎system/Test/Mock/MockAppConfig.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ class MockAppConfig extends App
2323
public $cookieSecure = false;
2424
public $cookieHTTPOnly = false;
2525
public $cookieSameSite = 'Lax';
26-
public $proxyIPs = '';
26+
public $proxyIPs = [];
2727
public $CSRFTokenName = 'csrf_test_name';
2828
public $CSRFHeaderName = 'X-CSRF-TOKEN';
2929
public $CSRFCookieName = 'csrf_cookie_name';

‎system/Test/Mock/MockCLIConfig.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ class MockCLIConfig extends App
2323
public $cookieSecure = false;
2424
public $cookieHTTPOnly = false;
2525
public $cookieSameSite = 'Lax';
26-
public $proxyIPs = '';
26+
public $proxyIPs = [];
2727
public $CSRFTokenName = 'csrf_test_name';
2828
public $CSRFCookieName = 'csrf_cookie_name';
2929
public $CSRFExpire = 7200;

0 commit comments

Comments
 (0)