Fixed issue #19598: [security] No CSRF protection on userManagement (…

…thanks to paoloelia) (#3880)
LimeSurvey · Jun 19, 2024 · 43c4960 · 43c4960
1 parent 9e7b55f
commit 43c4960
Showing 1 changed file with 33 additions and 8 deletions.
diff --git a/application/controllers/UserManagementController.php b/application/controllers/UserManagementController.php
@@ -34,6 +34,19 @@ public function accessRules()
         );
     }
 
+    /**
+     * @inheritdoc
+     */
+    public function filters()
+    {
+        return [
+            'postOnly + applyEdit, runAddDummyUser, deleteUser, userActivateDeactivate,'
+            . ' batchStatus, saveUserPermissions, saveThemePermissions, saveRole, importUsers, deleteMultiple,'
+            . ' batchSendAndResetLoginData, batchPermissions, batchAddGroup, batchApplyRoles,'
+            . ' TakeOwnership'
+        ];
+    }
+
     /**
      * @return string|string[]|null
      * @throws CException
@@ -245,13 +258,6 @@ public function actionRunAddDummyUser()
                 ['errors' => [gT("You do not have permission to access this page.")], 'noButton' => true]
             );
         }
-        if (!App()->request->isPostRequest) {
-            //it has to be post request when inserting data to DB
-            return $this->renderPartial(
-                'partial/error',
-                ['errors' => [gT("Access denied.")], 'noButton' => true]
-            );
-        }
         $times = App()->request->getPost('times', 5);
         $minPwLength = \LimeSurvey\Models\Services\PasswordManagement::MIN_PASSWORD_LENGTH;
         $passwordSize = (int) App()->request->getPost('passwordsize', $minPwLength);
@@ -521,13 +527,16 @@ public function actionBatchStatus()
 
     /**
      * Activate / deactivate user
-     *
+     * @todo : move this to a private function !!!
      * @param array $userIds
      * @param string $operation activate or deactivate
      * @return array
      */
     public function userActivation($userIds, $operation)
     {
+        if (!App()->getRequest()->getIsPostRequest()) {
+            throw new CHttpException(400, gT('Your request is invalid.'));
+        }
         $results = [];
         foreach ($userIds as $iUserId) {
             $oUser = User::model()->findByPk($iUserId);
@@ -1399,13 +1408,17 @@ public function actionTakeOwnership()
 
     /**
      * Deletes a user
+     * @todo : move to a private function
      *
      * @param int $uid
      * @return boolean
      * @throws CException
      */
     public function deleteUser(int $uid): bool
     {
+        if (!App()->getRequest()->getIsPostRequest()) {
+            throw new CHttpException(400, gT('Your request is invalid.'));
+        }
         $permission_users_delete = Permission::model()->hasGlobalPermission('users', 'delete');
         $permission_superadmin_read = Permission::model()->hasGlobalPermission('superadmin', 'read');
         if (!$permission_users_delete) {
@@ -1461,6 +1474,7 @@ public function deleteUser(int $uid): bool
     /**
      * Returns the data model based on the primary key given in the GET variable.
      * If the data model is not found, an HTTP exception will be raised.
+     * Why not a private function here ?
      *
      * @param int $id the ID of the model to be loaded
      *
@@ -1480,13 +1494,20 @@ public function loadModel(int $id): User
 
     /**
      * Update admin-user
+     * @todo : move to and private function, but need review unit test before.
      *
      * @param array $aUser array with user details
      * @return object user - updated user object
      * @throws CException
      */
     public function updateAdminUser(array $aUser): User
     {
+        if (
+            !App()->getRequest()->getIsPostRequest()
+            && !(defined('PHP_ENV') && PHP_ENV == 'test') // For unit test
+        ) {
+            throw new CHttpException(400, gT('Your request is invalid.'));
+        }
         $oUser = $this->loadModel($aUser['uid']);
         // Abort if logged in user has no access to this user.
         // Using same logic as User::getButtons().
@@ -1561,13 +1582,17 @@ private function createAdminUser(array $aUser, bool $sendEmail = true): array
 
     /**
      * Create new user
+     * @todo : move to private function
      *
      * @param array $aUser array with user details
      * @return array returns all attributes from model user as an array
      * @throws CException
      */
     public function createNewUser(array $aUser): array
     {
+        if (!App()->getRequest()->getIsPostRequest()) {
+            throw new CHttpException(400, gT('Your request is invalid.'));
+        }
         if (!Permission::model()->hasGlobalPermission('users', 'create')) {
             return Yii::app()->getController()->renderPartial('/admin/super/_renderJson', [
                 "data" => [