[BUGFIX] Dont use initilize actions for granting access

The initialize actions can not be stoppped anymore, which results in an inpropper access validation thx to Daniel Hoffmann for providing the patch, thx to the Security Team for reporting it. https://projekte.in2code.de/issues/60523
in2code-de · Dec 13, 2023 · d5ae330 · d5ae330
1 parent 970e39b
commit d5ae330
Show file tree

Hide file tree

Showing 4 changed files with 66 additions and 38 deletions.
diff --git a/Classes/Controller/AbstractController.php b/Classes/Controller/AbstractController.php
@@ -369,25 +369,14 @@ protected function redirectByAction($action = 'new', $category = 'redirect')
         }
     }
 
-    /**
-     * Init for User delete action
-     */
-    protected function initializeDeleteAction()
-    {
-        $user = UserUtility::getCurrentUser();
-        $token = $this->request->getArgument('token');
-        $uid = $this->request->getArgument('user');
-        $this->testSpoof($user, $uid, $token);
-    }
-
     /**
      * Check if user is authenticated and params are valid
      *
      * @param User $user
      * @param int $uid Given fe_users uid
      * @param string $receivedToken Token
      */
-    protected function testSpoof($user, $uid, $receivedToken)
+    protected function isSpoof($user, $uid, $receivedToken)
     {
         $errorOnProfileUpdate = false;
         $knownToken = GeneralUtility::hmac($user->getUid(), (string)$user->getCrdate()->getTimestamp());
@@ -403,14 +392,9 @@ protected function testSpoof($user, $uid, $receivedToken)
         }
 
         if ($errorOnProfileUpdate === true) {
-            $this->logUtility->log(Log::STATUS_PROFILEUPDATEREFUSEDSECURITY, $user);
-            $this->addFlashMessage(
-                LocalizationUtility::translateByState(Log::STATUS_PROFILEUPDATEREFUSEDSECURITY),
-                '',
-                AbstractMessage::ERROR
-            );
-            return new ForwardResponse('edit');
+            return true;
         }
+        return false;
     }
 
     /**

diff --git a/Classes/Controller/EditController.php b/Classes/Controller/EditController.php
@@ -18,9 +18,11 @@
 use In2code\Femanager\Utility\UserUtility;
 use Psr\Http\Message\ResponseInterface;
 use TYPO3\CMS\Core\Messaging\AbstractMessage;
+use TYPO3\CMS\Core\Messaging\FlashMessage;
 use TYPO3\CMS\Core\Utility\GeneralUtility;
 use TYPO3\CMS\Extbase\Annotation\Validate;
 use TYPO3\CMS\Extbase\Mvc\Exception\UnsupportedRequestTypeException;
+use TYPO3\CMS\Extbase\Http\ForwardResponse;
 
 /**
  * Class EditController
@@ -44,11 +46,6 @@ public function editAction(): ResponseInterface
 
     public function initializeUpdateAction()
     {
-        $user = UserUtility::getCurrentUser();
-        $userValues = $this->request->getArgument('user');
-        $token = $this->request->getArgument('token');
-
-        $this->testSpoof($user, $userValues['__identity'], $token);
         if ($this->keepPassword()) {
             unset($this->pluginVariables['user']['password']);
             unset($this->pluginVariables['password_repeat']);
@@ -64,7 +61,29 @@ public function initializeUpdateAction()
      */
     public function updateAction(User $user)
     {
-        $this->redirectIfDirtyObject($user);
+        $currentUser = UserUtility::getCurrentUser();
+        $userValues = $this->request->hasArgument('user') ? $this->request->getArgument('user') : null;
+        $token = $this->request->hasArgument('token') ? $this->request->getArgument('token') : null;
+
+        if ($currentUser === null ||
+            empty($userValues['__identity']) ||
+            (int)$userValues['__identity'] === null ||
+            $token === null ||
+            $this->isSpoof($currentUser, (int)$userValues['__identity'], $token)
+        ) {
+            $this->logUtility->log(Log::STATUS_PROFILEUPDATEREFUSEDSECURITY, $user);
+            $this->addFlashMessage(
+                LocalizationUtility::translateByState(Log::STATUS_PROFILEUPDATEREFUSEDSECURITY),
+                '',
+                FlashMessage::ERROR
+            );
+            return new ForwardResponse('edit');
+        }
+
+        $response = $this->redirectIfNoChangesOnObject($user);
+        if ($response !== null) {
+            return $response;
+        }
         $user = FrontendUtility::forceValues(
             $user,
             ConfigurationUtility::getValue('edit./forceValues./beforeAnyConfirmation.', $this->config)
@@ -172,6 +191,23 @@ protected function statusRefuse(User $user)
      */
     public function deleteAction(User $user)
     {
+        $currentUser = UserUtility::getCurrentUser();
+        $token = $this->request->hasArgument('token') ? $this->request->getArgument('token') : null;
+        $uid = $this->request->hasArgument('user') ? $this->request->getArgument('user') : null;
+        if ($currentUser === null ||
+            $token === null ||
+            $uid === null ||
+            $this->isSpoof($currentUser, (int)$uid, $token)
+        ) {
+            $this->logUtility->log(Log::STATUS_PROFILEUPDATEREFUSEDSECURITY, $user);
+            $this->addFlashMessage(
+                LocalizationUtility::translateByState(Log::STATUS_PROFILEUPDATEREFUSEDSECURITY),
+                '',
+                FlashMessage::ERROR
+            );
+            return new ForwardResponse('edit');
+        }
+
         $this->eventDispatcher->dispatch(new DeleteUserEvent($user));
         $this->logUtility->log(Log::STATUS_PROFILEDELETE, $user);
         $this->addFlashMessage(LocalizationUtility::translateByState(Log::STATUS_PROFILEDELETE));
@@ -198,16 +234,14 @@ protected function keepPassword()
 
     /**
      * Check: If there are no changes, simple redirect back
-     *
-     * @param User $user
-     * @throws UnsupportedRequestTypeException
      */
-    protected function redirectIfDirtyObject(User $user)
+    protected function redirectIfNoChangesOnObject(User $user)
     {
         if (!ObjectUtility::isDirtyObject($user)) {
-            $this->addFlashMessage(LocalizationUtility::translate('noChanges'), '', AbstractMessage::NOTICE);
-            $this->redirect('edit');
+            $this->addFlashMessage(LocalizationUtility::translate('noChanges'), '', FlashMessage::NOTICE);
+            return $this->redirect('edit');
         }
+        return null;
     }
 
     /**

diff --git a/Classes/Controller/InvitationController.php b/Classes/Controller/InvitationController.php
@@ -219,13 +219,6 @@ public function updateAction(User $user, $hash = null)
         $this->redirect('status');
     }
 
-    /**
-     * Init for delete
-     */
-    protected function initializeDeleteAction()
-    {
-    }
-
     /**
      * action delete
      *

diff --git a/Classes/Controller/UserBackendController.php b/Classes/Controller/UserBackendController.php
@@ -16,6 +16,7 @@
 use TYPO3\CMS\Core\Messaging\AbstractMessage;
 use TYPO3\CMS\Core\Site\SiteFinder;
 use TYPO3\CMS\Core\Utility\GeneralUtility;
+use TYPO3\CMS\Extbase\Http\ForwardResponse;
 
 /**
  * Class UserBackendController
@@ -69,6 +70,10 @@ public function confirmationAction(array $filter = []): ResponseInterface
      */
     public function userLogoutAction(User $user)
     {
+        $queryParams = $this->request->getQueryParams();
+        if (empty($queryParams['id']) || $queryParams['id'] !== (string)$user->getPid()) {
+            return new ForwardResponse('list');
+        }
         UserUtility::removeFrontendSessionToUser($user);
         $this->addFlashMessage('User successfully logged out');
         $this->redirect('list');
@@ -82,6 +87,10 @@ public function confirmUserAction(int $userIdentifier)
         $this->configPID = $this->getConfigPID();
 
         $user = $this->userRepository->findByUid($userIdentifier);
+        $queryParams = $this->request->getQueryParams();
+        if (empty($queryParams['id']) || $user === null || $queryParams['id'] !== (string)$user->getPid()) {
+            return new ForwardResponse('list');
+        }
         $this->eventDispatcher->dispatch(new AdminConfirmationUserEvent($user));
 
         $jsonResult = $this->getFrontendRequestResult('adminConfirmation', $userIdentifier, $user);
@@ -118,6 +127,10 @@ public function refuseUserAction(int $userIdentifier)
         $this->configPID = $this->getConfigPID();
 
         $user = $this->userRepository->findByUid($userIdentifier);
+        $queryParams = $this->request->getQueryParams();
+        if (empty($queryParams['id']) || $user === null || $queryParams['id'] !== (string)$user->getPid()) {
+            return new ForwardResponse('list');
+        }
         $this->eventDispatcher->dispatch(new RefuseUserEvent($user));
 
         $jsonResult = $this->getFrontendRequestResult('adminConfirmationRefused', $userIdentifier, $user);
@@ -171,6 +184,10 @@ public function listOpenUserConfirmationsAction(array $filter = []): ResponseInt
     public function resendUserConfirmationRequestAction(int $userIdentifier)
     {
         $user = $this->userRepository->findByUid($userIdentifier);
+        $queryParams = $this->request->getQueryParams();
+        if (empty($queryParams['id']) || $user === null || $queryParams['id'] !== (string)$user->getPid()) {
+            return new ForwardResponse('list');
+        }
         $this->sendCreateUserConfirmationMail($user);
         $this->addFlashMessage(
             LocalizationUtility::translate(