CVE-2019-12794: Improper Privilege Management (CWE-269)
value: An issue was discovered in MISP 2.4.108. Organization admins could reset credentials for site admins (organization admins have the inherent ability to reset passwords for all of their organization's users). This, however, could be abused in a situation where the host organization of an instance creates organization admins. An organization admin could set a password manually for the site admin or simply use the API key of the site admin to impersonate them. The potential for abuse only occurs when the host organization creates lower-privilege organization admins instead of the usual site admins. Also, only organization admins of the same organization as the site admin could abuse this.
CWE description: The product does not properly assign, modify, track, or check privileges for an actor, creating an unintended sphere of control for that actor.
CVSS v3 base score: 6.6
Published: 2019-06-11T17:29Z
Repository: https://github.com/MISP/MISP
Language: PHP
Files changed: app/Controller/LogsController.php, app/Controller/UsersController.php, app/Model/Log.php
Commit date: 2019-06-11 11:05:34 +0200
Commit message: fix: [security] Org admins could reset credentials for site admins

- org admins have the inherent ability to reset passwords for all of their org's users
- this however could be abused if for some reason the host org of an instance would create org admins
  - the org admin could set a password manually for the site admin or simply use the API key of the site admin to impersonate them
- the potential for abuse is very circumstancial as it requires the host org to create lower privilege org admins instead of the usual site admins
- only org admins of the same organisation as the site admin could abuse this

- as reported by Raymond Schippers

Diff: diff --git a/app/Controller/LogsController.php b/app/Controller/LogsController.php
index 3099a609d9..329449d002 100644
--- a/app/Controller/LogsController.php
+++ b/app/Controller/LogsController.php
@@ -341,11 +341,15 @@ class LogsController extends AppController
                 $this->paginate = array(
                     'limit' => 60,
                     'conditions' => $conditions,
                     'order' => array('Log.id' => 'DESC')
                 );
-                $this->set('list', $this->paginate());
+                $list = $this->paginate();
+                if (empty($this->Auth->user('Role')['perm_site_admin'])) {
+                    $list = $this->Log->filterSiteAdminSensitiveLogs($list);
+                }
+                $this->set('list', $list);
 
                 // and store into session
                 $this->Session->write('paginate_conditions_log', $this->paginate);
                 $this->Session->write('paginate_conditions_log_email', $filters['email']);
                 $this->Session->write('paginate_conditions_log_org', $filters['org']);
@@ -392,11 +396,15 @@ class LogsController extends AppController
                 if (!isset($this->paginate['order'])) {
                     $this->paginate['order'] = array('Log.id' => 'DESC');
                 }
                 $conditions = $this->__buildSearchConditions($filters);
                 $this->paginate['conditions'] = $conditions;
-                $this->set('list', $this->paginate());
+                $list = $this->paginate();
+                if (empty($this->Auth->user('Role')['perm_site_admin'])) {
+                    $list = $this->Log->filterSiteAdminSensitiveLogs($list);
+                }
+                $this->set('list', $list);
 
                 // set the same view as the index page
                 $this->render('admin_index');
             }
         } else {
diff --git a/app/Controller/UsersController.php b/app/Controller/UsersController.php
index a9ce2a750a..dcbd6d8610 100644
--- a/app/Controller/UsersController.php
+++ b/app/Controller/UsersController.php
@@ -17,11 +17,11 @@ class UsersController extends AppController
             'order' => array(
                     'Organisation.name' => 'ASC'
             ),
             'contain' => array(
                 'Organisation' => array('id', 'name'),
-                'Role' => array('id', 'name', 'perm_auth')
+                'Role' => array('id', 'name', 'perm_auth', 'perm_site_admin')
             )
     );
 
     public $helpers = array('Js' => array('Jquery'));
 
@@ -348,15 +348,20 @@ class UsersController extends AppController
             'date_created',
             'date_modified'
                     ),
                     'contain' => array(
                             'Organisation' => array('id', 'name'),
-                            'Role' => array('id', 'name', 'perm_auth')
+                            'Role' => array('id', 'name', 'perm_auth', 'perm_site_admin')
                     )
             ));
             foreach ($users as $key => $value) {
-                unset($users['User']['password']);
+                if (empty($this->Auth->user('Role')['perm_site_admin'])) {
+                    if ($value['Role']['perm_site_admin']) {
+                        $users[$key]['User']['authkey'] = __('Redacted');
+                    }
+                }
+                unset($users[$key]['User']['password']);
             }
             return $this->RestResponse->viewData($users, $this->response->type());
         } else {
             $this->set('urlparams', $urlParams);
             $this->set('passedArgsArray', $passedArgsArray);
@@ -364,11 +369,17 @@ class UsersController extends AppController
             if ($this->_isSiteAdmin()) {
                 $this->set('users', $this->paginate());
             } else {
                 $conditions['User.org_id'] = $this->Auth->user('org_id');
                 $this->paginate['conditions']['AND'][] = $conditions;
-                $this->set('users', $this->paginate());
+                $users = $this->paginate();
+                foreach ($users as $key => $value) {
+                    if ($value['Role']['perm_site_admin']) {
+                        $users[$key]['User']['authkey'] = __('Redacted');
+                    }
+                }
+                $this->set('users', $users);
             }
             if ($this->request->is('ajax')) {
                 $this->autoRender = false;
                 $this->layout = false;
                 $this->render('ajax/admin_index');
@@ -460,10 +471,13 @@ class UsersController extends AppController
             $pgpDetails = $this->User->verifySingleGPG($user);
             $user['User']['pgp_status'] = isset($pgpDetails[2]) ? $pgpDetails[2] : 'OK';
             $user['User']['fingerprint'] = !empty($pgpDetails[4]) ? $pgpDetails[4] : 'N/A';
         }
         $user['User']['orgAdmins'] = $this->User->getOrgAdminsForOrg($user['User']['org_id'], $user['User']['id']);
+        if (empty($this->Auth->user('Role')['perm_site_admin']) && !(empty($user['Role']['perm_site_admin']))) {
+            $user['User']['authkey'] = __('Redacted');
+        }
         $this->set('user', $user);
         if (!$this->_isSiteAdmin() && !($this->_isAdmin() && $this->Auth->user('org_id') == $user['User']['org_id'])) {
             throw new MethodNotAllowedException();
         }
         if ($this->_isRest()) {
@@ -692,24 +706,25 @@ class UsersController extends AppController
             throw new NotFoundException(__('Invalid user'));
         }
         $params = array();
         $allowedRole = '';
         $userToEdit = $this->User->find('first', array(
-                'conditions' => array('id' => $id),
+                'conditions' => array('User.id' => $id),
                 'recursive' => -1,
-                'fields' => array('id', 'role_id', 'email', 'org_id'),
+                'fields' => array('User.id', 'User.role_id', 'User.email', 'User.org_id', 'Role.perm_site_admin'),
+                'contain' => array('Role')
         ));
         if (!$this->_isSiteAdmin()) {
             // Org admins should be able to select the role that is already assigned to an org user when editing them.
             // What happened previously:
             // Org admin edits another org admin of the same org
             // Org admin is not allowed to set privileged access roles (site_admin/sync/regex)
             // MISP automatically chooses the first available option for the user as the selected setting (usually user)
             // Org admin is downgraded to a user
             // Now we make an exception for the already assigned role, both in the form and the actual edit.
-            if ($userToEdit['User']['org_id'] != $this->Auth->user('org_id')) {
-                throw new Exception('Invalid user');
+            if ($userToEdit['User']['org_id'] != $this->Auth->user('org_id') || !empty($userToEdit['Role']['perm_site_admin'])) {
+                throw new NotFoundException(__('Invalid user'));
             }
             $allowedRole = $userToEdit['User']['role_id'];
             $params = array('conditions' => array(
                     'OR' => array(
                             'AND' => array(
diff --git a/app/Model/Log.php b/app/Model/Log.php
index dc215b9037..20a928cf04 100644
--- a/app/Model/Log.php
+++ b/app/Model/Log.php
@@ -293,6 +293,33 @@ class Log extends AppModel
             }
             $syslog->write($action, $entry);
         }
         return true;
     }
+
+    public function filterSiteAdminSensitiveLogs($list)
+    {
+        $this->User = ClassRegistry::init('User');
+        $site_admin_roles = $this->User->Role->find('list', array(
+            'recursive' => -1,
+            'conditions' => array('Role.perm_site_admin' => 1),
+            'fields' => array('Role.id', 'Role.id')
+        ));
+        $site_admins = $this->User->find('list', array(
+            'recursive' => -1,
+            'conditions' => array(
+                'User.role_id' => array_values($site_admin_roles)
+            ),
+            'fields' => array('User.id', 'User.id')
+        ));
+        foreach ($list as $k => $v) {
+            if (
+                $v['Log']['model'] === 'User' &&
+                in_array($v['Log']['model_id'], array_values($site_admins)) &&
+                in_array($v['Log']['action'], array('add', 'edit', 'reset_auth_key'))
+            ) {
+                $list[$k]['Log']['change'] = __('Redacted');
+            }
+        }
+        return $list;
+    }
 }

Vulnerable code:                 $this->set('list', $this->paginate());
                $this->set('list', $this->paginate());
                'Role' => array('id', 'name', 'perm_auth')
                            'Role' => array('id', 'name', 'perm_auth')
                unset($users['User']['password']);
                $this->set('users', $this->paginate());
                'conditions' => array('id' => $id),
                'fields' => array('id', 'role_id', 'email', 'org_id'),
            if ($userToEdit['User']['org_id'] != $this->Auth->user('org_id')) {
                throw new Exception('Invalid user');
Fixed code:                 $list = $this->paginate();
                if (empty($this->Auth->user('Role')['perm_site_admin'])) {
                    $list = $this->Log->filterSiteAdminSensitiveLogs($list);
                }
                $this->set('list', $list);
                $list = $this->paginate();
                if (empty($this->Auth->user('Role')['perm_site_admin'])) {
                    $list = $this->Log->filterSiteAdminSensitiveLogs($list);
                }
                $this->set('list', $list);
                'Role' => array('id', 'name', 'perm_auth', 'perm_site_admin')
                            'Role' => array('id', 'name', 'perm_auth', 'perm_site_admin')
                if (empty($this->Auth->user('Role')['perm_site_admin'])) {
                    if ($value['Role']['perm_site_admin']) {
                        $users[$key]['User']['authkey'] = __('Redacted');
                    }
                }
                unset($users[$key]['User']['password']);
                $users = $this->paginate();
                foreach ($users as $key => $value) {
                    if ($value['Role']['perm_site_admin']) {
                        $users[$key]['User']['authkey'] = __('Redacted');
                    }
                }
                $this->set('users', $users);
        if (empty($this->Auth->user('Role')['perm_site_admin']) && !(empty($user['Role']['perm_site_admin']))) {
            $user['User']['authkey'] = __('Redacted');
        }
                'conditions' => array('User.id' => $id),
                'fields' => array('User.id', 'User.role_id', 'User.email', 'User.org_id', 'Role.perm_site_admin'),
                'contain' => array('Role')
            if ($userToEdit['User']['org_id'] != $this->Auth->user('org_id') || !empty($userToEdit['Role']['perm_site_admin'])) {
                throw new NotFoundException(__('Invalid user'));

    public function filterSiteAdminSensitiveLogs($list)
    {
        $this->User = ClassRegistry::init('User');
        $site_admin_roles = $this->User->Role->find('list', array(
            'recursive' => -1,
            'conditions' => array('Role.perm_site_admin' => 1),
            'fields' => array('Role.id', 'Role.id')
        ));
        $site_admins = $this->User->find('list', array(
            'recursive' => -1,
            'conditions' => array(
                'User.role_id' => array_values($site_admin_roles)
            ),
            'fields' => array('User.id', 'User.id')
        ));
        foreach ($list as $k => $v) {
            if (
                $v['Log']['model'] === 'User' &&
                in_array($v['Log']['model_id'], array_values($site_admins)) &&
                in_array($v['Log']['action'], array('add', 'edit', 'reset_auth_key'))
            ) {
                $list[$k]['Log']['change'] = __('Redacted');
            }
        }
        return $list;
    }