fix possibly open redirect in login function

zikula · Sep 20, 2021 · a43c7bd · a43c7bd
1 parent cc9521e
commit a43c7bd
Show file tree

Hide file tree

Showing 2 changed files with 17 additions and 3 deletions.
diff --git 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
@@ -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);
+    }
 }