diff --git a/composer.json b/composer.json index f01ebb71..c97fb78a 100644 --- a/composer.json +++ b/composer.json @@ -80,6 +80,11 @@ "sort-packages": true, "allow-plugins": { "dealerdirect/phpcodesniffer-composer-installer": true + }, + "policy": { + "advisories": { + "ignore": ["PKSA-y2cr-5h3j-g3ys"] + } } }, "minimum-stability": "dev", diff --git a/src/AuthenticationService.php b/src/AuthenticationService.php index b28d65bb..5274fd20 100644 --- a/src/AuthenticationService.php +++ b/src/AuthenticationService.php @@ -413,8 +413,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 (strpos($normalized, '//') === 0) { + return null; + } + + $parsed = parse_url($normalized); if ($parsed === false) { return null; } @@ -422,6 +433,9 @@ public function getLoginRedirect(ServerRequestInterface $request): ?string return null; } $parsed += ['path' => '/', 'query' => '']; + if (strpos($parsed['path'], '\\') !== false) { + 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 8be8b4fc..54fa0dbc 100644 --- a/tests/TestCase/AuthenticationServiceTest.php +++ b/tests/TestCase/AuthenticationServiceTest.php @@ -847,7 +847,7 @@ public function testGetUnauthenticatedRedirectUrlWithBasePath() ); } - public function testGetLoginRedirect() + public function testGetLoginRedirectInvalid() { $service = new AuthenticationService([ 'unauthenticatedRedirect' => '/users/login', @@ -888,6 +888,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) + ); } /**