diff --git a/src/AuthenticationService.php b/src/AuthenticationService.php index 051a5b79..8f0b6fb6 100644 --- a/src/AuthenticationService.php +++ b/src/AuthenticationService.php @@ -398,8 +398,19 @@ public function getLoginRedirect(ServerRequestInterface $request): ?string ) { return null; } + $value = (string)$params[$redirectParam]; - $parsed = parse_url((string)$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; } @@ -407,6 +418,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 90a193ba..61527e88 100644 --- a/tests/TestCase/AuthenticationServiceTest.php +++ b/tests/TestCase/AuthenticationServiceTest.php @@ -822,7 +822,7 @@ public function testGetUnauthenticatedRedirectUrlWithBasePath(): void ); } - public function testGetLoginRedirect(): void + public function testGetLoginRedirectInvalid(): void { $service = new AuthenticationService([ 'unauthenticatedRedirect' => '/users/login', @@ -863,6 +863,39 @@ public function testGetLoginRedirect(): void '/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), + ); } /**