diff --git a/system/HTTP/IncomingRequest.php b/system/HTTP/IncomingRequest.php index cc66e35f4ddc..c7a326c17d85 100644 --- a/system/HTTP/IncomingRequest.php +++ b/system/HTTP/IncomingRequest.php @@ -276,8 +276,18 @@ public function isSecure(): bool return false; } - if ($this->hasHeader('X-Forwarded-Proto') && $this->header('X-Forwarded-Proto')->getValue() === 'https') { - return true; + if ($this->hasHeader('X-Forwarded-Proto')) { + $proto = $this->header('X-Forwarded-Proto')->getValue(); + + // A single value may be a string or a one-element array. Multiple + // values are ambiguous and cannot be trusted, so they are ignored. + if (is_array($proto)) { + $proto = count($proto) === 1 ? reset($proto) : null; + } + + if (is_string($proto) && strtolower(trim($proto)) === 'https') { + return true; + } } if (! $this->hasHeader('Front-End-Https')) { diff --git a/tests/system/HTTP/IncomingRequestTest.php b/tests/system/HTTP/IncomingRequestTest.php index 464b46e2ce2c..89646081e53c 100644 --- a/tests/system/HTTP/IncomingRequestTest.php +++ b/tests/system/HTTP/IncomingRequestTest.php @@ -828,6 +828,12 @@ public static function provideIsSecureWithForwardedHeaders(): iterable 'X-Forwarded-Proto trusted proxy IP' => [ 'X-Forwarded-Proto', 'https', '10.0.1.200', ['10.0.1.200' => 'X-Forwarded-For'], true, ], + 'X-Forwarded-Proto trusted proxy IP uppercase' => [ + 'X-Forwarded-Proto', 'HTTPS', '10.0.1.200', ['10.0.1.200' => 'X-Forwarded-For'], true, + ], + 'X-Forwarded-Proto trusted proxy IP with surrounding whitespace' => [ + 'X-Forwarded-Proto', ' https ', '10.0.1.200', ['10.0.1.200' => 'X-Forwarded-For'], true, + ], 'X-Forwarded-Proto no trusted proxies' => [ 'X-Forwarded-Proto', 'https', '10.0.1.200', [], false, ], @@ -879,6 +885,32 @@ public static function provideIsSecureWithForwardedHeaders(): iterable ]; } + public function testIsSecureWithForwardedProtoSingleElementArray(): void + { + service('superglobals')->setServer('REMOTE_ADDR', '10.0.1.200'); + + $config = new App(); + $config->proxyIPs = ['10.0.1.200' => 'X-Forwarded-For']; + + $request = $this->createRequest($config); + $request->setHeader('X-Forwarded-Proto', ['https']); + + $this->assertTrue($request->isSecure()); + } + + public function testIsSecureWithForwardedProtoMultipleValuesIsNotSecure(): void + { + service('superglobals')->setServer('REMOTE_ADDR', '10.0.1.200'); + + $config = new App(); + $config->proxyIPs = ['10.0.1.200' => 'X-Forwarded-For']; + + $request = $this->createRequest($config); + $request->setHeader('X-Forwarded-Proto', ['https', 'http']); + + $this->assertFalse($request->isSecure()); + } + public function testUserAgent(): void { service('superglobals')->setServer('HTTP_USER_AGENT', 'Mozilla'); diff --git a/user_guide_src/source/changelogs/v4.7.6.rst b/user_guide_src/source/changelogs/v4.7.6.rst index 04eb1be1d52b..608899f60ae4 100644 --- a/user_guide_src/source/changelogs/v4.7.6.rst +++ b/user_guide_src/source/changelogs/v4.7.6.rst @@ -33,6 +33,10 @@ Bugs Fixed - **Security:** Fixed a bug where a valid JSON request body that is not an object (for example a JSON array) was corrupted after CSRF verification when the token was sent via the ``X-CSRF-TOKEN`` header. +- **HTTP:** Fixed a bug where ``IncomingRequest::isSecure()`` did not + recognize ``https`` in the ``X-Forwarded-Proto`` header from a trusted + proxy when the value contained uppercase letters or surrounding + whitespace, or was represented as a single-element array. See the repo's `CHANGELOG.md `_