diff --git a/composer.json b/composer.json index 8f74bcc9..b407b982 100644 --- a/composer.json +++ b/composer.json @@ -61,6 +61,11 @@ "allow-plugins": { "dealerdirect/phpcodesniffer-composer-installer": true }, + "policy": { + "advisories": { + "ignore": ["PKSA-y2cr-5h3j-g3ys"] + } + }, "sort-packages": true }, "scripts": { diff --git a/src/AuthenticationService.php b/src/AuthenticationService.php index c9d03513..b9c57e14 100644 --- a/src/AuthenticationService.php +++ b/src/AuthenticationService.php @@ -459,8 +459,19 @@ public function getLoginRedirect(ServerRequestInterface $request): ?string ) { return null; } + $value = (string)$params[$redirectParam]; - $parsed = parse_url($params[$redirectParam]); + // In the `Location` header, Browsers normalize \ to / + // (see WHATWG URL Standard). + // We do the same to prevent injection via \ sequences. + $normalized = str_replace('\\', '/', $value); + + // A leading run of `//` or `\\` are rejected + if (str_starts_with($normalized, '//')) { + return null; + } + + $parsed = parse_url($normalized); if ($parsed === false) { return null; } @@ -468,6 +479,9 @@ public function getLoginRedirect(ServerRequestInterface $request): ?string return null; } $parsed += ['path' => '/', 'query' => '']; + if (str_contains($parsed['path'], '\\')) { + return null; + } if (strlen($parsed['path']) && $parsed['path'][0] !== '/') { $parsed['path'] = "/{$parsed['path']}"; } diff --git a/tests/TestCase/AuthenticationServiceTest.php b/tests/TestCase/AuthenticationServiceTest.php index 2b96b8b8..60075903 100644 --- a/tests/TestCase/AuthenticationServiceTest.php +++ b/tests/TestCase/AuthenticationServiceTest.php @@ -915,7 +915,7 @@ public function testGetUnauthenticatedRedirectUrlWithBasePath() ); } - public function testGetLoginRedirect() + public function testGetLoginRedirectInvalid() { $service = new AuthenticationService([ 'unauthenticatedRedirect' => '/users/login', @@ -956,6 +956,39 @@ public function testGetLoginRedirect() '/path/with?query=string', $service->getLoginRedirect($request), ); + + $request = ServerRequestFactory::fromGlobals( + ['REQUEST_URI' => '/login'], + ['redirect' => '/\\evil.com'], + ); + $this->assertNull( + $service->getLoginRedirect($request), + ); + + $request = ServerRequestFactory::fromGlobals( + ['REQUEST_URI' => '/login'], + ['redirect' => '\\/\\evil.com'], + ); + $this->assertNull( + $service->getLoginRedirect($request), + ); + + $request = ServerRequestFactory::fromGlobals( + ['REQUEST_URI' => '/login'], + ['redirect' => '\\evil.com/path'], + ); + $this->assertSame( + '/evil.com/path', + $service->getLoginRedirect($request), + ); + + $request = ServerRequestFactory::fromGlobals( + ['REQUEST_URI' => '/login'], + ['redirect' => '\\\\evil.com/path'], + ); + $this->assertNull( + $service->getLoginRedirect($request), + ); } /**