fix: [security] Tightening of the role assignment permissions

- If a decoupled perm_admin role was configured on the system, this could be assigned by low privilege administrators, leading to privilege escalation - This fix moves the responsibility of the check to the ACL component rather than the controller - As reported by Jeroen Pinoy (@Wachizungu)
cerebrate-project · Nov 28, 2024 · 0131422 · 0131422
1 parent 55cac2e
commit 0131422
Show file tree

Hide file tree

Showing 2 changed files with 16 additions and 8 deletions.
diff --git a/src/Controller/Component/ACLComponent.php b/src/Controller/Component/ACLComponent.php
@@ -363,11 +363,19 @@ public function canEditUser(User $currentUser, User $user): bool
             return true;
         }
 
-        if ($user['role']['perm_community_admin']) {
-            return false; // org_admins cannot edit admins
+        $this->Roles = TableRegistry::get('Roles');
+        $validRoles = [];
+        if (!$currentUser['role']['perm_community_admin']) {
+            if ($currentUser['role']['perm_group_admin']) {
+                $validRoles = $this->Roles->find('list')->select(['id', 'name'])->order(['name' => 'asc'])->where(['perm_community_admin' => 0, 'perm_group_admin' => 0, 'perm_admin' => 0])->all()->toArray();
+            } else {
+                $validRoles = $this->Roles->find('list')->select(['id', 'name'])->order(['name' => 'asc'])->where(['perm_community_admin' => 0, 'perm_group_admin' => 0, 'perm_org_admin' => 0, 'perm_admin' => 0])->all()->toArray();
+            }
+        } else {
+            $validRoles = $this->Roles->find('list')->order(['name' => 'asc'])->all()->toArray();
         }
-        if ($currentUser['role']['perm_org_admin'] && $user['role']['perm_group_admin']) {
-            return false; // org_admins cannot edit group_admin
+        if (!in_array($user['role_id'], array_keys($validRoles)) && $currentUser['id'] != $user['id']) {
+            return false;
         }
         if ($currentUser['role']['perm_group_admin']) {
             $this->OrgGroups = TableRegistry::get('OrgGroups');

diff --git a/src/Controller/UsersController.php b/src/Controller/UsersController.php
@@ -86,10 +86,10 @@ public function add()
         $individual_ids = [];
         if (!$currentUser['role']['perm_community_admin']) {
             if ($currentUser['role']['perm_group_admin']) {
-                $validRoles = $this->Users->Roles->find('list')->select(['id', 'name'])->order(['name' => 'asc'])->where(['perm_community_admin' => 0, 'perm_group_admin' => 0])->all()->toArray();
+                $validRoles = $this->Users->Roles->find('list')->select(['id', 'name'])->order(['name' => 'asc'])->where(['perm_community_admin' => 0, 'perm_group_admin' => 0, 'perm_admin' => 0])->all()->toArray();
                 $individual_ids = $this->Users->Individuals->find('aligned', ['organisation_id' => $currentUser['organisation_id']])->all()->extract('id')->toArray();
             } else {
-                $validRoles = $this->Users->Roles->find('list')->select(['id', 'name'])->order(['name' => 'asc'])->where(['perm_community_admin' => 0, 'perm_group_admin' => 0, 'perm_org_admin' => 0])->all()->toArray();
+                $validRoles = $this->Users->Roles->find('list')->select(['id', 'name'])->order(['name' => 'asc'])->where(['perm_community_admin' => 0, 'perm_group_admin' => 0, 'perm_org_admin' => 0, 'perm_admin' => 0])->all()->toArray();
 
             }
             if (empty($individual_ids)) {
@@ -247,10 +247,10 @@ public function edit($id = false)
         $validOrgIds = [];
         if (!$currentUser['role']['perm_community_admin']) {
             if ($currentUser['role']['perm_group_admin']) {
-                $validRoles = $this->Users->Roles->find('list')->select(['id', 'name'])->order(['name' => 'asc'])->where(['perm_community_admin' => 0, 'perm_group_admin' => 0])->all()->toArray();
+                $validRoles = $this->Users->Roles->find('list')->select(['id', 'name'])->order(['name' => 'asc'])->where(['perm_community_admin' => 0, 'perm_group_admin' => 0, 'perm_admin' => 0])->all()->toArray();
                 $validOrgIds = $this->Users->Organisations->OrgGroups->getGroupOrgIdsForUser($currentUser);
             } else {
-                $validRoles = $this->Users->Roles->find('list')->select(['id', 'name'])->order(['name' => 'asc'])->where(['perm_community_admin' => 0, 'perm_group_admin' => 0, 'perm_org_admin' => 0])->all()->toArray();
+                $validRoles = $this->Users->Roles->find('list')->select(['id', 'name'])->order(['name' => 'asc'])->where(['perm_community_admin' => 0, 'perm_group_admin' => 0, 'perm_org_admin' => 0, 'perm_admin' => 0])->all()->toArray();
             }
         } else {
             $validRoles = $this->Users->Roles->find('list')->order(['name' => 'asc'])->all()->toArray();