From a43c7bd76cae8ab17c48a830809dc84399663a84 Mon Sep 17 00:00:00 2001 From: Axel Guckelsberger Date: Mon, 20 Sep 2021 08:20:52 +0200 Subject: [PATCH] fix possibly open redirect in login function --- CHANGELOG-3.0.md | 1 + .../Controller/AccessController.php | 19 ++++++++++++++++--- 2 files changed, 17 insertions(+), 3 deletions(-) diff --git a/CHANGELOG-3.0.md b/CHANGELOG-3.0.md index 278e76e797..6545b6420f 100644 --- a/CHANGELOG-3.0.md +++ b/CHANGELOG-3.0.md @@ -13,6 +13,7 @@ - [Permissions] Correctly handle non-existing username during permission testing. - [Users] Dispatch `UserPostLoginFailureEvent` after login failure as expected. - [Users] Add missing check in `CurrentUserApi` to avoid an error in PHP8. + - [Users] Fix possibly open redirect in login function. - [ZAuth] Fix wrong `DateTime` argument in `UserVerificationRepository`. - Features: diff --git a/src/system/UsersModule/Controller/AccessController.php b/src/system/UsersModule/Controller/AccessController.php index db012494f9..22856a3a5e 100644 --- a/src/system/UsersModule/Controller/AccessController.php +++ b/src/system/UsersModule/Controller/AccessController.php @@ -171,7 +171,7 @@ public function loginAction( $returnUrl = $userPreSuccessLoginEvent->getRedirectUrl(); } - return !empty($returnUrl) ? $this->redirect($returnUrl) : $this->redirectToRoute('home'); + return !empty($returnUrl) ? $this->redirect($this->sanitizeReturnUrl($request, $returnUrl)) : $this->redirectToRoute('home'); } } } @@ -185,7 +185,7 @@ public function loginAction( $eventDispatcher->dispatch($userPostFailLoginEvent); $returnUrl = $userPostFailLoginEvent->getRedirectUrl(); - return !empty($returnUrl) ? $this->redirect($returnUrl) : $this->redirectToRoute('home'); + return !empty($returnUrl) ? $this->redirect($this->sanitizeReturnUrl($request, $returnUrl)) : $this->redirectToRoute('home'); } /** @@ -210,8 +210,21 @@ public function logoutAction( } return isset($returnUrl) - ? $this->redirect($returnUrl) + ? $this->redirect($this->sanitizeReturnUrl($request, $returnUrl)) : $this->redirectToRoute('home', ['_locale' => $this->getParameter('locale')]) ; } + + private function sanitizeReturnUrl(Request $request, $returnUrl = null) + { + if (null === $returnUrl || empty($returnUrl)) { + return $returnUrl; + } + + if ('/' !== mb_substr($returnUrl, 0, 1)) { + $returnUrl = '/' . $returnUrl; + } + + return $request->getUriForPath($returnUrl); + } }