Skip to content

Commit cb6d801

Browse files
kenjisMGatner
kenjis
authored and
MGatner
committed
fix: format of $proxyIPs
Specify HTTP header name for security reasons.
1 parent c946f2c commit cb6d801

8 files changed

Lines changed: 147 additions & 108 deletions

File tree

‎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 an 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: 83 additions & 66 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
/**
@@ -59,95 +60,111 @@ public function getIPAddress(): string
5960
/**
6061
* @deprecated $this->proxyIPs property will be removed in the future
6162
*/
63+
// @phpstan-ignore-next-line
6264
$proxyIPs = $this->proxyIPs ?? config('App')->proxyIPs;
63-
if (! empty($proxyIPs) && ! is_array($proxyIPs)) {
64-
$proxyIPs = explode(',', str_replace(' ', '', $proxyIPs));
65+
if (! empty($proxyIPs)) {
66+
// @phpstan-ignore-next-line
67+
if (! is_array($proxyIPs) || is_int(array_keys($proxyIPs)[0])) {
68+
throw new ConfigException(
69+
'You must set an array with Proxy IP address key and HTTP header name value in Config\App::$proxyIPs.'
70+
);
71+
}
6572
}
6673

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

6976
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();
77+
foreach ($proxyIPs as $proxyIP => $header) {
78+
// Check if we have an IP address or a subnet
79+
if (strpos($proxyIP, '/') === false) {
80+
// An IP address (and not a subnet) is specified.
81+
// We can compare right away.
82+
if ($proxyIP === $this->ipAddress) {
83+
$spoof = null;
84+
$headerObj = $this->header($header);
85+
86+
if ($headerObj !== null) {
87+
$spoof = $headerObj->getValue();
88+
89+
// Some proxies typically list the whole chain of IP
90+
// addresses through which the client has reached us.
91+
// e.g. client_ip, proxy_ip1, proxy_ip2, etc.
92+
sscanf($spoof, '%[^,]', $spoof);
93+
94+
if (! $ipValidator($spoof)) {
95+
$spoof = null;
96+
} else {
97+
$this->ipAddress = $spoof;
98+
break;
99+
}
100+
}
101+
}
76102

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);
103+
continue;
104+
}
81105

82-
if (! $ipValidator($spoof)) {
83-
$spoof = null;
84-
} else {
85-
break;
86-
}
106+
// We have a subnet ... now the heavy lifting begins
107+
if (! isset($separator)) {
108+
$separator = $ipValidator($this->ipAddress, 'ipv6') ? ':' : '.';
87109
}
88-
}
89110

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) {
97-
$this->ipAddress = $spoof;
98-
break;
99-
}
111+
// If the proxy entry doesn't match the IP protocol - skip it
112+
if (strpos($proxyIP, $separator) === false) {
113+
continue;
114+
}
100115

101-
continue;
102-
}
116+
// Convert the REMOTE_ADDR IP address to binary, if needed
117+
if (! isset($ip, $sprintf)) {
118+
if ($separator === ':') {
119+
// Make sure we're having the "full" IPv6 format
120+
$ip = explode(':', str_replace('::', str_repeat(':', 9 - substr_count($this->ipAddress, ':')), $this->ipAddress));
103121

104-
// We have a subnet ... now the heavy lifting begins
105-
if (! isset($separator)) {
106-
$separator = $ipValidator($this->ipAddress, 'ipv6') ? ':' : '.';
107-
}
122+
for ($j = 0; $j < 8; $j++) {
123+
$ip[$j] = intval($ip[$j], 16);
124+
}
108125

109-
// If the proxy entry doesn't match the IP protocol - skip it
110-
if (strpos($proxyIP, $separator) === false) {
111-
continue;
126+
$sprintf = '%016b%016b%016b%016b%016b%016b%016b%016b';
127+
} else {
128+
$ip = explode('.', $this->ipAddress);
129+
$sprintf = '%08b%08b%08b%08b';
112130
}
113131

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));
132+
$ip = vsprintf($sprintf, $ip);
133+
}
119134

120-
for ($j = 0; $j < 8; $j++) {
121-
$ip[$j] = intval($ip[$j], 16);
122-
}
135+
// Split the netmask length off the network address
136+
sscanf($proxyIP, '%[^/]/%d', $netaddr, $masklen);
123137

124-
$sprintf = '%016b%016b%016b%016b%016b%016b%016b%016b';
125-
} else {
126-
$ip = explode('.', $this->ipAddress);
127-
$sprintf = '%08b%08b%08b%08b';
128-
}
138+
// Again, an IPv6 address is most likely in a compressed form
139+
if ($separator === ':') {
140+
$netaddr = explode(':', str_replace('::', str_repeat(':', 9 - substr_count($netaddr, ':')), $netaddr));
129141

130-
$ip = vsprintf($sprintf, $ip);
142+
for ($i = 0; $i < 8; $i++) {
143+
$netaddr[$i] = intval($netaddr[$i], 16);
131144
}
145+
} else {
146+
$netaddr = explode('.', $netaddr);
147+
}
132148

133-
// Split the netmask length off the network address
134-
sscanf($proxyIP, '%[^/]/%d', $netaddr, $masklen);
149+
// Convert to binary and finally compare
150+
if (strncmp($ip, vsprintf($sprintf, $netaddr), $masklen) === 0) {
151+
$spoof = null;
152+
$headerObj = $this->header($header);
135153

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));
154+
if ($headerObj !== null) {
155+
$spoof = $headerObj->getValue();
139156

140-
for ($i = 0; $i < 8; $i++) {
141-
$netaddr[$i] = intval($netaddr[$i], 16);
142-
}
143-
} else {
144-
$netaddr = explode('.', $netaddr);
145-
}
157+
// Some proxies typically list the whole chain of IP
158+
// addresses through which the client has reached us.
159+
// e.g. client_ip, proxy_ip1, proxy_ip2, etc.
160+
sscanf($spoof, '%[^,]', $spoof);
146161

147-
// Convert to binary and finally compare
148-
if (strncmp($ip, vsprintf($sprintf, $netaddr), $masklen) === 0) {
149-
$this->ipAddress = $spoof;
150-
break;
162+
if (! $ipValidator($spoof)) {
163+
$spoof = null;
164+
} else {
165+
$this->ipAddress = $spoof;
166+
break;
167+
}
151168
}
152169
}
153170
}

‎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;

‎tests/system/HTTP/IncomingRequestTest.php‎

Lines changed: 22 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -714,9 +714,12 @@ public function testGetIPAddressThruProxy()
714714
$expected = '123.123.123.123';
715715
$_SERVER['HTTP_X_FORWARDED_FOR'] = $expected;
716716
$_SERVER['REMOTE_ADDR'] = '10.0.1.200';
717-
$config = new App();
718-
$config->proxyIPs = '10.0.1.200,192.168.5.0/24';
719717

718+
$config = new App();
719+
$config->proxyIPs = [
720+
'10.0.1.200' => 'X-Forwarded-For',
721+
'192.168.5.0/24' => 'X-Forwarded-For',
722+
];
720723
$this->request = new Request($config);
721724
$this->request->populateHeaders();
722725

@@ -729,9 +732,12 @@ public function testGetIPAddressThruProxyInvalid()
729732
$_SERVER['HTTP_X_FORWARDED_FOR'] = '123.456.23.123';
730733
$expected = '10.0.1.200';
731734
$_SERVER['REMOTE_ADDR'] = $expected;
732-
$config = new App();
733-
$config->proxyIPs = '10.0.1.200,192.168.5.0/24';
734735

736+
$config = new App();
737+
$config->proxyIPs = [
738+
'10.0.1.200' => 'X-Forwarded-For',
739+
'192.168.5.0/24' => 'X-Forwarded-For',
740+
];
735741
$this->request = new Request($config);
736742
$this->request->populateHeaders();
737743

@@ -744,9 +750,12 @@ public function testGetIPAddressThruProxyNotWhitelisted()
744750
$expected = '10.10.1.200';
745751
$_SERVER['REMOTE_ADDR'] = $expected;
746752
$_SERVER['HTTP_X_FORWARDED_FOR'] = '123.456.23.123';
747-
$config = new App();
748-
$config->proxyIPs = '10.0.1.200,192.168.5.0/24';
749753

754+
$config = new App();
755+
$config->proxyIPs = [
756+
'10.0.1.200' => 'X-Forwarded-For',
757+
'192.168.5.0/24' => 'X-Forwarded-For',
758+
];
750759
$this->request = new Request($config);
751760
$this->request->populateHeaders();
752761

@@ -759,10 +768,10 @@ public function testGetIPAddressThruProxySubnet()
759768
$expected = '123.123.123.123';
760769
$_SERVER['HTTP_X_FORWARDED_FOR'] = $expected;
761770
$_SERVER['REMOTE_ADDR'] = '192.168.5.21';
762-
$config = new App();
763-
$config->proxyIPs = ['192.168.5.0/24'];
764771

765-
$this->request = new Request($config);
772+
$config = new App();
773+
$config->proxyIPs = ['192.168.5.0/24' => 'X-Forwarded-For'];
774+
$this->request = new Request($config);
766775
$this->request->populateHeaders();
767776

768777
// we should see the original forwarded address
@@ -773,10 +782,11 @@ public function testGetIPAddressThruProxyOutofSubnet()
773782
{
774783
$expected = '192.168.5.21';
775784
$_SERVER['REMOTE_ADDR'] = $expected;
776-
$config = new App();
777-
$config->proxyIPs = ['192.168.5.0/28'];
778785
$_SERVER['HTTP_X_FORWARDED_FOR'] = '123.123.123.123';
779-
$this->request = new Request($config);
786+
787+
$config = new App();
788+
$config->proxyIPs = ['192.168.5.0/28' => 'X-Forwarded-For'];
789+
$this->request = new Request($config);
780790
$this->request->populateHeaders();
781791

782792
// we should see the original forwarded address

0 commit comments

Comments
 (0)