Fixed issue #19133: [security] User can send email to group members, …

…while not having permissions (#3587)
LimeSurvey · Nov 3, 2023 · cdcf12b · cdcf12b
1 parent d8ab7b1
commit cdcf12b
Show file tree

Hide file tree

Showing 2 changed files with 53 additions and 61 deletions.
diff --git a/application/controllers/UserGroupController.php b/application/controllers/UserGroupController.php
@@ -101,10 +101,6 @@ public function actionIndex()
      */
     public function actionViewGroup($ugid, bool $header = false)
     {
-        if (!Permission::model()->hasGlobalPermission('usergroups', 'read')) {
-            throw new CHttpException(403);
-        }
-
         $ugid = (int)$ugid;
         if (empty($ugid)) {
             throw new CHttpException(400, gT('GroupId missing'));
@@ -114,19 +110,15 @@ public function actionViewGroup($ugid, bool $header = false)
         if (empty($userGroup)) {
             throw new CHttpException(404, gT("User group not found."));
         }
-
-        // Only allow access if user is:
-        // - Superadmin
-        // - Owner of the group
-        // - Member of the group
+        /* Check Permssion to view */
         if (
             !(
-                $userGroup->owner_id == Yii::app()->user->id ||
-                $userGroup->hasUser(Yii::app()->user->id) ||
-                Permission::model()->hasGlobalPermission('superadmin', 'read')
+                Permission::model()->hasGlobalPermission('superadmin', 'read') // superadmin
+                ||  $userGroup->owner_id == Yii::app()->user->id // owner
+                || ($userGroup->hasUser(Yii::app()->user->id) && Permission::model()->hasGlobalPermission('usergroups', 'read')) // inside group and have global UserGroup view
             )
         ) {
-            throw new CHttpException(403);
+            throw new CHttpException(403, gT("No access : you do not have permission to this users group."));
         }
 
         $aData = [];
@@ -168,12 +160,15 @@ public function actionViewGroup($ugid, bool $header = false)
         $aData['topbar']['title'] = gT('User group') . ': ' . $userGroup->name;
         $aData['topbar']['backLink'] = App()->createUrl('userGroup/index');
 
+
         $aData['topbar']['middleButtons'] = $this->renderPartial(
             'partial/topbarBtns_manageGroup/leftSideButtons',
             [
                 'userGroupId' => $userGroup->ugid,
-                'hasPermission' => (Yii::app()->session['loginID'] == $userGroup->owner_id ||
-                    Permission::model()->hasGlobalPermission('superadmin', 'read'))
+                'hasPermission' => (
+                    Permission::model()->hasGlobalPermission('superadmin', 'read')
+                    || App()->getCurrentUserId() == $userGroup->owner_id
+                )
             ],
             true
         );
@@ -467,37 +462,36 @@ public function actionMailToAllUsersInGroup(int $ugid)
     {
         $ugid = sanitize_int($ugid);
         $action = Yii::app()->request->getPost("action");
+        $currentUserId = App()->getCurrentUserId();
+        $userGroup = UserGroup::model()->findByPk($ugid);
+        if (empty($userGroup)) {
+            throw new CHttpException(404, gT("User group not found."));
+        }
+        if (
+            !Permission::model()->hasGlobalPermission('superadmin', 'read') // User is not a superadmin
+            && $userGroup->owner_id != $currentUserId // User is not owner
+        ) {
+            throw new CHttpException(403, gT("No access : you do not have permission to send emails to all users."));
+        }
+        $redirectUrl = App()->createUrl("userGroup/viewGroup", ['ugid' => $ugid]);
         $aData = [];
-
+        $aData['ugid'] = $ugid;
         if ($action == "mailsendusergroup") {
-            // user must be in user group or superadmin
-            if ($ugid === null) {
-                $ugid = (int) Yii::app()->request->getPost('ugid');
+            try {
+                $sendCopy = Yii::app()->getRequest()->getPost('copymail') == 1 ? 1 : 0;
+                $emailSendingResults = UserGroup::model()->sendUserEmails(
+                    $ugid,
+                    Yii::app()->getRequest()->getPost('subject'),
+                    Yii::app()->getRequest()->getPost('body'),
+                    $sendCopy
+                );
+                App()->user->setFlash('success', $emailSendingResults);
+            } catch (Exception $e) {
+                // TODO: Show error message?
+                App()->user->setFlash('error', gT("Error: no email has been send."));
             }
-            $result = UserInGroup::model()->findAllByPk(array('ugid' => (int)$ugid, 'uid' => Yii::app()->session['loginID']));
-            if (count($result) > 0 || Permission::model()->hasGlobalPermission('superadmin', 'read')) {
-                try {
-                    $sendCopy = Yii::app()->getRequest()->getPost('copymail') == 1 ? 1 : 0;
-                    $emailSendingResults = UserGroup::model()->sendUserEmails(
-                        $ugid,
-                        Yii::app()->getRequest()->getPost('subject'),
-                        Yii::app()->getRequest()->getPost('body'),
-                        $sendCopy
-                    );
-
-                    Yii::app()->user->setFlash('success', $emailSendingResults);
-                    $this->redirect(array('userGroup/viewGroup/ugid/' . $ugid));
-                } catch (Exception $e) {
-                    // TODO: Show error message?
-                    Yii::app()->user->setFlash('error', gT("Error: no email has been send."));
-                    $this->redirect(array('userGroup/viewGroup/ugid/' . $ugid));
-                }
-            } else {
-                Yii::app()->user->setFlash('error', gT("You do not have permission to send emails to all users. "));
-                $this->redirect(array('userGroup/viewGroup/ugid/' . $ugid));
-            }
-        } else {
-            $aData['ugid'] = $ugid;
+            $this->redirect($redirectUrl);
+            App()->end(); // redirect end : add it here for clarity
         }
 
         $aData['topbar']['title'] = gT('Mail to all Members');
@@ -509,9 +503,6 @@ public function actionMailToAllUsersInGroup(int $ugid)
         );
 
         $this->aData = $aData;
-
-        $this->render('mailUserGroup_view', [
-            'ugid' => $aData['ugid']
-        ]);
+        $this->render('mailUserGroup_view', $aData);
     }
 }
diff --git a/application/views/userGroup/partial/topbarBtns_manageGroup/leftSideButtons.php b/application/views/userGroup/partial/topbarBtns_manageGroup/leftSideButtons.php
@@ -2,20 +2,21 @@
 
 /** @var int $userGroupId */
 /** @var bool $hasPermission */
-
-$this->widget(
-    'ext.ButtonWidget.ButtonWidget',
-    [
-        'name' => 'group-mail-button',
-        'id' => 'group-mail-button',
-        'text' => gT('Mail to all Members'),
-        'icon' => 'ri-mail-send-fill',
-        'link' => $this->createUrl("userGroup/mailToAllUsersInGroup/ugid/" . $userGroupId),
-        'htmlOptions' => [
-            'class' => 'btn btn-outline-secondary',
-        ],
-    ]
-);
+if ($hasPermission) {
+    $this->widget(
+        'ext.ButtonWidget.ButtonWidget',
+        [
+            'name' => 'group-mail-button',
+            'id' => 'group-mail-button',
+            'text' => gT('Mail to all Members'),
+            'icon' => 'ri-mail-send-fill',
+            'link' => $this->createUrl("userGroup/mailToAllUsersInGroup/ugid/" . $userGroupId),
+            'htmlOptions' => [
+                'class' => 'btn btn-outline-secondary',
+            ],
+        ]
+    );
+}
 
 if ($hasPermission) {
     $this->widget(