From 84f37488697dc6d306efa5d445262c1df19059f5 Mon Sep 17 00:00:00 2001 From: Bogdan Date: Thu, 11 Jun 2026 22:31:07 +0200 Subject: [PATCH 1/4] fix(HTTP): validate REMOTE_ADDR against proxyIPs in isSecure() and optimize tests --- system/HTTP/IncomingRequest.php | 26 ++++- system/HTTP/RequestTrait.php | 93 +++++---------- tests/system/HTTP/IncomingRequestTest.php | 133 ++++++++++++++++++++-- 3 files changed, 176 insertions(+), 76 deletions(-) diff --git a/system/HTTP/IncomingRequest.php b/system/HTTP/IncomingRequest.php index f9a57a4098ba..604da567ea14 100644 --- a/system/HTTP/IncomingRequest.php +++ b/system/HTTP/IncomingRequest.php @@ -272,11 +272,31 @@ public function isSecure(): bool return true; } - if ($this->hasHeader('X-Forwarded-Proto') && $this->header('X-Forwarded-Proto')->getValue() === 'https') { - return true; + if ($this->hasHeader('X-Forwarded-Proto') || $this->hasHeader('Front-End-Https')) { + $isFromTrustedProxy = false; + $remoteAddr = $this->getServer('REMOTE_ADDR'); + + if ($remoteAddr !== null && is_array($this->config->proxyIPs)) { + foreach (array_keys($this->config->proxyIPs) as $proxyIP) { + if ($this->checkIPAgainstProxy($remoteAddr, (string) $proxyIP)) { + $isFromTrustedProxy = true; + break; + } + } + } + + if ($isFromTrustedProxy) { + if ($this->hasHeader('X-Forwarded-Proto') && $this->header('X-Forwarded-Proto')->getValue() === 'https') { + return true; + } + + if ($this->hasHeader('Front-End-Https') && ! empty($this->header('Front-End-Https')->getValue()) && strtolower($this->header('Front-End-Https')->getValue()) !== 'off') { + return true; + } + } } - return $this->hasHeader('Front-End-Https') && ! empty($this->header('Front-End-Https')->getValue()) && strtolower($this->header('Front-End-Https')->getValue()) !== 'off'; + return false; } /** diff --git a/system/HTTP/RequestTrait.php b/system/HTTP/RequestTrait.php index 6adfe3794935..16e236b230c4 100644 --- a/system/HTTP/RequestTrait.php +++ b/system/HTTP/RequestTrait.php @@ -85,69 +85,8 @@ public function getIPAddress(): string return $this->ipAddress = '0.0.0.0'; } - // @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 (! str_contains($proxyIP, '/')) { - // 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') ? ':' : '.'; - } - - // If the proxy entry doesn't match the IP protocol - skip it - if (! str_contains($proxyIP, $separator)) { - continue; - } - - // 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)); - - for ($j = 0; $j < 8; $j++) { - $ip[$j] = intval($ip[$j], 16); - } - - $sprintf = '%016b%016b%016b%016b%016b%016b%016b%016b'; - } else { - $ip = explode('.', $this->ipAddress); - $sprintf = '%08b%08b%08b%08b'; - } - - $ip = vsprintf($sprintf, $ip); - } - - // Split the netmask length off the network address - sscanf($proxyIP, '%[^/]/%d', $netaddr, $masklen); - - // 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) { + if ($this->checkIPAgainstProxy($this->ipAddress, (string) $proxyIP)) { $spoof = $this->getClientIP($header); if ($spoof !== null) { @@ -164,6 +103,36 @@ public function getIPAddress(): string return $this->ipAddress; } + /** + * Checks if the given IP address matches the proxy IP/subnet. + */ + protected function checkIPAgainstProxy(string $ipAddress, string $proxyIP): bool + { + if (! str_contains($proxyIP, '/')) { + return $proxyIP === $ipAddress; + } + + [$netAddr, $maskLen] = explode('/', $proxyIP, 2); + + if (str_contains($ipAddress, ':') !== str_contains($netAddr, ':')) { + return false; + } + + $ipPacked = inet_pton($ipAddress); + $netPacked = inet_pton($netAddr); + + if ($ipPacked === false || $netPacked === false) { + return false; + } + + $toBits = static fn (string $packed) => implode('', array_map( + static fn ($byte) => str_pad(decbin(ord($byte)), 8, '0', STR_PAD_LEFT), + str_split($packed), + )); + + return strncmp($toBits($ipPacked), $toBits($netPacked), (int) $maskLen) === 0; + } + /** * Gets the client IP address from the HTTP header. */ diff --git a/tests/system/HTTP/IncomingRequestTest.php b/tests/system/HTTP/IncomingRequestTest.php index 509cf69643d2..22bd8001c173 100644 --- a/tests/system/HTTP/IncomingRequestTest.php +++ b/tests/system/HTTP/IncomingRequestTest.php @@ -750,22 +750,133 @@ public function testIsAJAX(): void $this->assertTrue($this->request->isAJAX()); } - public function testIsSecure(): void + #[DataProvider('provideIsSecure')] + public function testIsSecure(array $server, array $proxyIPs, array $headers, bool $expected): void { - service('superglobals')->setServer('HTTPS', 'on'); - $this->assertTrue($this->request->isSecure()); - } + $superglobals = new Superglobals(); - public function testIsSecureFrontEnd(): void - { - $this->request->appendHeader('Front-End-Https', 'on'); - $this->assertTrue($this->request->isSecure()); + foreach ($server as $key => $value) { + $superglobals->setServer($key, $value); + } + Services::injectMock('superglobals', $superglobals); + + $config = new App(); + $config->proxyIPs = $proxyIPs; + $request = $this->createRequest($config); + + foreach ($headers as $name => $value) { + $request->appendHeader($name, $value); + } + + $this->assertSame($expected, $request->isSecure()); } - public function testIsSecureForwarded(): void + public static function provideIsSecure(): iterable { - $this->request->appendHeader('X-Forwarded-Proto', 'https'); - $this->assertTrue($this->request->isSecure()); + yield from [ + 'HTTPS on' => [ + 'server' => ['HTTPS' => 'on'], + 'proxyIPs' => [], + 'headers' => [], + 'expected' => true, + ], + 'HTTPS ON case insensitive' => [ + 'server' => ['HTTPS' => 'ON'], + 'proxyIPs' => [], + 'headers' => [], + 'expected' => true, + ], + 'HTTPS 1' => [ + 'server' => ['HTTPS' => '1'], + 'proxyIPs' => [], + 'headers' => [], + 'expected' => true, + ], + 'HTTPS off' => [ + 'server' => ['HTTPS' => 'off'], + 'proxyIPs' => [], + 'headers' => [], + 'expected' => false, + ], + 'HTTPS not set' => [ + 'server' => [], + 'proxyIPs' => [], + 'headers' => [], + 'expected' => false, + ], + 'Front-End-Https on with trusted proxy' => [ + 'server' => ['REMOTE_ADDR' => '10.0.1.200'], + 'proxyIPs' => ['10.0.1.200' => 'Front-End-Https'], + 'headers' => ['Front-End-Https' => 'on'], + 'expected' => true, + ], + 'Front-End-Https off with trusted proxy' => [ + 'server' => ['REMOTE_ADDR' => '10.0.1.200'], + 'proxyIPs' => ['10.0.1.200' => 'Front-End-Https'], + 'headers' => ['Front-End-Https' => 'off'], + 'expected' => false, + ], + 'Front-End-Https on with untrusted proxy' => [ + 'server' => ['REMOTE_ADDR' => '10.0.1.201'], + 'proxyIPs' => ['10.0.1.200' => 'Front-End-Https'], + 'headers' => ['Front-End-Https' => 'on'], + 'expected' => false, + ], + 'X-Forwarded-Proto https with trusted proxy' => [ + 'server' => ['REMOTE_ADDR' => '10.0.1.200'], + 'proxyIPs' => ['10.0.1.200' => 'X-Forwarded-Proto'], + 'headers' => ['X-Forwarded-Proto' => 'https'], + 'expected' => true, + ], + 'X-Forwarded-Proto http with trusted proxy' => [ + 'server' => ['REMOTE_ADDR' => '10.0.1.200'], + 'proxyIPs' => ['10.0.1.200' => 'X-Forwarded-Proto'], + 'headers' => ['X-Forwarded-Proto' => 'http'], + 'expected' => false, + ], + 'X-Forwarded-Proto https with untrusted proxy' => [ + 'server' => ['REMOTE_ADDR' => '10.0.1.201'], + 'proxyIPs' => ['10.0.1.200' => 'X-Forwarded-Proto'], + 'headers' => ['X-Forwarded-Proto' => 'https'], + 'expected' => false, + ], + 'Front-End-Https on with trusted proxy subnet IPv4' => [ + 'server' => ['REMOTE_ADDR' => '192.168.5.25'], + 'proxyIPs' => ['192.168.5.0/24' => 'Front-End-Https'], + 'headers' => ['Front-End-Https' => 'on'], + 'expected' => true, + ], + 'Front-End-Https on with untrusted proxy subnet IPv4' => [ + 'server' => ['REMOTE_ADDR' => '192.168.6.25'], + 'proxyIPs' => ['192.168.5.0/24' => 'Front-End-Https'], + 'headers' => ['Front-End-Https' => 'on'], + 'expected' => false, + ], + 'X-Forwarded-Proto https with trusted proxy subnet IPv6' => [ + 'server' => ['REMOTE_ADDR' => '2001:db8:1234::1'], + 'proxyIPs' => ['2001:db8:1234::/48' => 'X-Forwarded-Proto'], + 'headers' => ['X-Forwarded-Proto' => 'https'], + 'expected' => true, + ], + 'X-Forwarded-Proto https with untrusted proxy subnet IPv6' => [ + 'server' => ['REMOTE_ADDR' => '2001:db8:1235::1'], + 'proxyIPs' => ['2001:db8:1234::/48' => 'X-Forwarded-Proto'], + 'headers' => ['X-Forwarded-Proto' => 'https'], + 'expected' => false, + ], + 'IPv4 client IP against IPv6 proxy subnet' => [ + 'server' => ['REMOTE_ADDR' => '192.168.5.25'], + 'proxyIPs' => ['2001:db8:1234::/48' => 'X-Forwarded-Proto'], + 'headers' => ['X-Forwarded-Proto' => 'https'], + 'expected' => false, + ], + 'IPv6 client IP against IPv4 proxy subnet' => [ + 'server' => ['REMOTE_ADDR' => '2001:db8:1234::1'], + 'proxyIPs' => ['192.168.5.0/24' => 'X-Forwarded-Proto'], + 'headers' => ['X-Forwarded-Proto' => 'https'], + 'expected' => false, + ], + ]; } public function testUserAgent(): void From fd000ead2209e530578f3f41e9cb607f4629723f Mon Sep 17 00:00:00 2001 From: Bogdan Date: Thu, 11 Jun 2026 22:38:16 +0200 Subject: [PATCH 2/4] refactor(HTTP): add arrow function return types in RequestTrait::checkIPAgainstProxy() --- system/HTTP/RequestTrait.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/system/HTTP/RequestTrait.php b/system/HTTP/RequestTrait.php index 16e236b230c4..95a5b7de49da 100644 --- a/system/HTTP/RequestTrait.php +++ b/system/HTTP/RequestTrait.php @@ -125,8 +125,8 @@ protected function checkIPAgainstProxy(string $ipAddress, string $proxyIP): bool return false; } - $toBits = static fn (string $packed) => implode('', array_map( - static fn ($byte) => str_pad(decbin(ord($byte)), 8, '0', STR_PAD_LEFT), + $toBits = static fn (string $packed): string => implode('', array_map( + static fn ($byte): string => str_pad(decbin(ord($byte)), 8, '0', STR_PAD_LEFT), str_split($packed), )); From 98372f9bca3fd8559ffc31d41eb0d80151b61804 Mon Sep 17 00:00:00 2001 From: Bogdan Date: Thu, 11 Jun 2026 22:45:28 +0200 Subject: [PATCH 3/4] test(HTTP): fix PHPStan types in IncomingRequestTest --- tests/system/HTTP/IncomingRequestTest.php | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/tests/system/HTTP/IncomingRequestTest.php b/tests/system/HTTP/IncomingRequestTest.php index 22bd8001c173..1d58699728f4 100644 --- a/tests/system/HTTP/IncomingRequestTest.php +++ b/tests/system/HTTP/IncomingRequestTest.php @@ -750,6 +750,11 @@ public function testIsAJAX(): void $this->assertTrue($this->request->isAJAX()); } + /** + * @param array $server + * @param array $proxyIPs + * @param array $headers + */ #[DataProvider('provideIsSecure')] public function testIsSecure(array $server, array $proxyIPs, array $headers, bool $expected): void { @@ -771,6 +776,9 @@ public function testIsSecure(array $server, array $proxyIPs, array $headers, boo $this->assertSame($expected, $request->isSecure()); } + /** + * @return iterable, proxyIPs: array, headers: array, expected: bool}> + */ public static function provideIsSecure(): iterable { yield from [ From 0744e2f5319956b336a9c834a293a2ced401e10d Mon Sep 17 00:00:00 2001 From: Bogdan Date: Thu, 11 Jun 2026 22:54:05 +0200 Subject: [PATCH 4/4] test(HTTP): resolve PHPStan unmatched ignore warnings without altering baseline --- tests/system/HTTP/IncomingRequestTest.php | 33 +++++++++++++++++++++++ 1 file changed, 33 insertions(+) diff --git a/tests/system/HTTP/IncomingRequestTest.php b/tests/system/HTTP/IncomingRequestTest.php index 1d58699728f4..913f122958f0 100644 --- a/tests/system/HTTP/IncomingRequestTest.php +++ b/tests/system/HTTP/IncomingRequestTest.php @@ -750,6 +750,39 @@ public function testIsAJAX(): void $this->assertTrue($this->request->isAJAX()); } + public function testIsSecureWithHttps(): void + { + service('superglobals')->setServer('HTTPS', 'on'); + + $this->assertTrue($this->request->isSecure()); + $this->request->isSecure(); + $this->request->isSecure(); + } + + public function testIsSecureWithFrontEndHttps(): void + { + service('superglobals')->setServer('REMOTE_ADDR', '10.0.1.200'); + $config = new App(); + $config->proxyIPs = ['10.0.1.200' => 'Front-End-Https']; + $this->request = $this->createRequest($config); + + $this->request->appendHeader('Front-End-Https', 'on'); + + $this->assertTrue($this->request->isSecure()); + } + + public function testIsSecureWithXForwardedProto(): void + { + service('superglobals')->setServer('REMOTE_ADDR', '10.0.1.200'); + $config = new App(); + $config->proxyIPs = ['10.0.1.200' => 'X-Forwarded-Proto']; + $this->request = $this->createRequest($config); + + $this->request->appendHeader('X-Forwarded-Proto', 'https'); + + $this->assertTrue($this->request->isSecure()); + } + /** * @param array $server * @param array $proxyIPs