CVE-2020-8894: Insufficient Information (NVD-CWE-noinfo)
value: An issue was discovered in MISP before 2.4.121. ACLs for discussion threads were mishandled in app/Controller/ThreadsController.php and app/Model/Thread.php.
CWE description: There is insufficient information about the issue to classify it; details are unkown or unspecified.
CVSS v3 base score: 6.5
Published: 2020-02-12T00:15Z
Repository: https://github.com/MISP/MISP
Language: PHP
Files changed: app/Controller/ThreadsController.php, app/Model/Thread.php
Commit date: 2020-02-08 10:34:23 +0100
Commit message: fix: [security] discussion thread ACL issues fixed

- as reported by Dawid Czarnecki

Diff: diff --git a/app/Controller/ThreadsController.php b/app/Controller/ThreadsController.php
index ad00f9cda4b..b34e8569919 100644
--- a/app/Controller/ThreadsController.php
+++ b/app/Controller/ThreadsController.php
@@ -61,71 +61,36 @@ class ThreadsController extends AppController
         }
     }
 
     private function __view($thread_id, $eventView, $post_id)
     {
+        $conditions = array('id' => $thread_id);
         if ($eventView) {
-            $id = $thread_id;
-            // Show the discussion
-            $this->Thread->Behaviors->unload('SysLogLogable.SysLogLogable');
-            $params = array('conditions' => array('event_id' => $id),
-                    'recursive' => -1,
-                    'fields' => array('id', 'event_id', 'distribution', 'title', 'sharing_group_id', 'org_id')
-            );
-            $thread = $this->Thread->find('first', $params);
-            if (!empty($thread)) {
-                if (!$this->_isSiteAdmin()) {
-                    if ($thread['Thread']['distribution'] == 0 && $thread['Thread']['org_id'] != $this->Auth->user('org_id')) {
-                        throw new MethodNotAllowedException('Invalid Thread.');
-                    }
-                    if ($thread['Thread']['distribution'] == 4) {
-                        if (!$this->Thread->SharingGroup->checkIfAuthorised($this->Auth->user(), $thread['Thread']['sharing_group_id'])) {
-                            new NotFoundException('Invalid thread.');
-                        }
-                    }
-                }
-                $thread_id = $thread['Thread']['id'];
-            } else {
-                $thread_id = 0;
+            $event_id = $thread_id;
+            if (!$this->request->is('ajax')) {
+                $this->redirect(array('controller' => 'events', 'action' => 'view', $event_id));
             }
-            $this->set('currentEvent', $id);
+            $conditions = array('event_id' => $event_id);
+            $this->set('currentEvent', $event_id);
+            $this->set('event_id', $event_id);
+            $this->set('context', 'event');
         } else {
-            $this->Thread->recursive = -1;
-            $this->Thread->id = $thread_id;
-
-            //If the thread doesn't exist, throw exception
-            if (!$this->Thread->exists()) {
+            $this->set('context', 'thread');
+        }
+        $this->set('myuserid', $this->Auth->user('id'));
+        $thread = $this->Thread->find('first', array(
+            'conditions' => $conditions,
+            'recursive' => -1
+        ));
+        if (empty($thread)) {
+            if (!$eventView) {
                 throw new NotFoundException('Invalid thread.');
             }
-            $thread = $this->Thread->read();
-
-            // If the thread belongs to an event, we have to make sure that the event's distribution level hasn't changed.
-            // This is also a good time to update the thread's distribution level if that did happen.
-            if (!empty($thread['Thread']['event_id'])) {
-                $this->loadModel('Event');
-                $this->Event->id = $thread['Thread']['event_id'];
-                $this->Event->recursive = -1;
-                $this->Event->read(array('id', 'distribution', 'org_id', 'sharing_group_id'));
-                if ($this->Event->data['Event']['distribution'] != $thread['Thread']['distribution']) {
-                    $this->Thread->saveField('distribution', $this->Event->data['Event']['distribution']);
-                }
-                if ($this->Event->data['Event']['sharing_group_id'] != $thread['Thread']['sharing_group_id']) {
-                    $this->Thread->saveField('sharing_group_id', $this->Event->data['Event']['sharing_group_id']);
-                }
-                $this->set('event_id', $thread['Thread']['event_id']);
-            }
-
-            // If the user shouldn't be allowed to see the event send him away.
-            if (!$this->_isSiteAdmin()) {
-                if ($thread['Thread']['distribution'] == 0 && $thread['Thread']['org_id'] != $this->Auth->user('org_id')) {
-                    throw new MethodNotAllowedException('Invalid Thread.');
-                }
-                if ($thread['Thread']['distribution'] == 4) {
-                    if (!$this->Thread->SharingGroup->checkIfAuthorised($this->Auth->user(), $thread['Thread']['sharing_group_id'])) {
-                        new NotFoundException('Invalid thread.');
-                    }
-                }
+        } else {
+            $thread_id = $thread['Thread']['id'];
+            if (!$this->Thread->checkIfAuthorised($this->Auth->user(), $thread_id)) {
+                throw new NotFoundException('Invalid thread.');
             }
         }
         if ($thread_id) {
             $this->paginate = array(
                     'limit' => 10,
@@ -164,30 +129,21 @@ class ThreadsController extends AppController
                 $this->set('posts', $posts);
                 $this->set('post_id', $post_id);
                 $this->set('thread', $thread);
             }
         }
-        if ($eventView) {
-            $this->set('context', 'event');
-            if (!$this->request->is('ajax')) {
-                $this->redirect(array('controller' => 'events', 'action' => 'view', $id));
-            }
-        } else {
-            $this->set('context', 'thread');
-        }
-        $this->set('myuserid', $this->Auth->user('id'));
         if ($this->request->is('ajax')) {
             $this->layout = 'ajax';
             $this->render('/Elements/eventdiscussion');
         }
     }
 
     public function index()
     {
         $this->loadModel('Posts');
         $this->loadModel('SharingGroup');
-        $sgids = $this->SharingGroup->fetchAllAuthorised($this->Auth->user);
+        $sgids = $this->SharingGroup->fetchAllAuthorised($this->Auth->user());
         $conditions = null;
         if (!$this->_isSiteAdmin()) {
             $conditions['AND']['OR'] = array(
                 'Thread.distribution' => array(1, 2, 3),
                 array(
@@ -204,11 +160,11 @@ class ThreadsController extends AppController
                 )
             );
         }
         $conditions['AND'][] = array('Thread.post_count >' => 0);
         $this->paginate = array(
-                'conditions' => array($conditions),
+                'conditions' => $conditions,
                 'fields' => array('date_modified', 'date_created', 'org_id', 'distribution', 'title', 'post_count', 'sharing_group_id'),
                 'contain' => array(
                     'Post' =>array(
                         'fields' => array(),
                         'limit' => 1,
diff --git a/app/Model/Thread.php b/app/Model/Thread.php
index 002d1c09ddd..52dbb890460 100644
--- a/app/Model/Thread.php
+++ b/app/Model/Thread.php
@@ -36,6 +36,53 @@ class Thread extends AppModel
             }
             $this->save($thread);
             return true;
         }
     }
+
+    // convenience method to check whether a user can see an event
+    public function checkIfAuthorised($user, $id)
+    {
+        if (!isset($user['id'])) {
+            throw new MethodNotAllowedException('Invalid user.');
+        }
+        $this->id = $id;
+        if (!$this->exists()) {
+            return false;
+        }
+        $thread = $this->find('first', array(
+            'conditions' => array('id' => $id),
+            'recursive' => -1
+        ));
+        if (!empty($thread['Thread']['event_id'])) {
+            $event = $this->Event->fetchEvent($user, array(
+                'eventid' => $thread['Thread']['event_id'],
+                'metadata' => true
+            ));
+            if (empty($event)) {
+                return false;
+            }
+            $event = $event[0];
+            // update the distribution if it diverged from the event
+            if (
+                $event['Event']['distribution'] != $thread['Thread']['distribution'] ||
+                $event['Event']['sharing_group_id'] != $thread['Thread']['sharing_group_id']
+            ) {
+                $this->Behaviors->unload('SysLogLogable.SysLogLogable');
+                $thread['Thread']['distribution'] = $event['Event']['distribution'];
+                $thread['Thread']['sharing_group_id'] = $event['Event']['sharing_group_id'];
+                $this->save($thread);
+            }
+            return !empty($event);
+        }
+        if ($user['Role']['perm_site_admin']) {
+            return true;
+        }
+        if ($thread['Thread']['org_id'] == $user['org_id'] || ($thread['Thread']['distribution'] > 0 && $thread['Thread']['distribution'] < 4)) {
+            return true;
+        }
+        if ($thread['Thread']['distribution'] == 4 && $this->SharingGroup->checkIfAuthorised($user, $thread['Thread']['sharing_group_id'])) {
+            return true;
+        }
+        return false;
+    }
 }

Vulnerable code:             $id = $thread_id;
            // Show the discussion
            $this->Thread->Behaviors->unload('SysLogLogable.SysLogLogable');
            $params = array('conditions' => array('event_id' => $id),
                    'recursive' => -1,
                    'fields' => array('id', 'event_id', 'distribution', 'title', 'sharing_group_id', 'org_id')
            );
            $thread = $this->Thread->find('first', $params);
            if (!empty($thread)) {
                if (!$this->_isSiteAdmin()) {
                    if ($thread['Thread']['distribution'] == 0 && $thread['Thread']['org_id'] != $this->Auth->user('org_id')) {
                        throw new MethodNotAllowedException('Invalid Thread.');
                    }
                    if ($thread['Thread']['distribution'] == 4) {
                        if (!$this->Thread->SharingGroup->checkIfAuthorised($this->Auth->user(), $thread['Thread']['sharing_group_id'])) {
                            new NotFoundException('Invalid thread.');
                        }
                    }
                }
                $thread_id = $thread['Thread']['id'];
            } else {
                $thread_id = 0;
            $this->set('currentEvent', $id);
            $this->Thread->recursive = -1;
            $this->Thread->id = $thread_id;

            //If the thread doesn't exist, throw exception
            if (!$this->Thread->exists()) {
            $thread = $this->Thread->read();

            // If the thread belongs to an event, we have to make sure that the event's distribution level hasn't changed.
            // This is also a good time to update the thread's distribution level if that did happen.
            if (!empty($thread['Thread']['event_id'])) {
                $this->loadModel('Event');
                $this->Event->id = $thread['Thread']['event_id'];
                $this->Event->recursive = -1;
                $this->Event->read(array('id', 'distribution', 'org_id', 'sharing_group_id'));
                if ($this->Event->data['Event']['distribution'] != $thread['Thread']['distribution']) {
                    $this->Thread->saveField('distribution', $this->Event->data['Event']['distribution']);
                }
                if ($this->Event->data['Event']['sharing_group_id'] != $thread['Thread']['sharing_group_id']) {
                    $this->Thread->saveField('sharing_group_id', $this->Event->data['Event']['sharing_group_id']);
                }
                $this->set('event_id', $thread['Thread']['event_id']);
            }

            // If the user shouldn't be allowed to see the event send him away.
            if (!$this->_isSiteAdmin()) {
                if ($thread['Thread']['distribution'] == 0 && $thread['Thread']['org_id'] != $this->Auth->user('org_id')) {
                    throw new MethodNotAllowedException('Invalid Thread.');
                }
                if ($thread['Thread']['distribution'] == 4) {
                    if (!$this->Thread->SharingGroup->checkIfAuthorised($this->Auth->user(), $thread['Thread']['sharing_group_id'])) {
                        new NotFoundException('Invalid thread.');
                    }
                }
        if ($eventView) {
            $this->set('context', 'event');
            if (!$this->request->is('ajax')) {
                $this->redirect(array('controller' => 'events', 'action' => 'view', $id));
            }
        } else {
            $this->set('context', 'thread');
        }
        $this->set('myuserid', $this->Auth->user('id'));
        $sgids = $this->SharingGroup->fetchAllAuthorised($this->Auth->user);
                'conditions' => array($conditions),
Fixed code:         $conditions = array('id' => $thread_id);
            $event_id = $thread_id;
            if (!$this->request->is('ajax')) {
                $this->redirect(array('controller' => 'events', 'action' => 'view', $event_id));
            $conditions = array('event_id' => $event_id);
            $this->set('currentEvent', $event_id);
            $this->set('event_id', $event_id);
            $this->set('context', 'event');
            $this->set('context', 'thread');
        }
        $this->set('myuserid', $this->Auth->user('id'));
        $thread = $this->Thread->find('first', array(
            'conditions' => $conditions,
            'recursive' => -1
        ));
        if (empty($thread)) {
            if (!$eventView) {
        } else {
            $thread_id = $thread['Thread']['id'];
            if (!$this->Thread->checkIfAuthorised($this->Auth->user(), $thread_id)) {
                throw new NotFoundException('Invalid thread.');
        $sgids = $this->SharingGroup->fetchAllAuthorised($this->Auth->user());
                'conditions' => $conditions,

    // convenience method to check whether a user can see an event
    public function checkIfAuthorised($user, $id)
    {
        if (!isset($user['id'])) {
            throw new MethodNotAllowedException('Invalid user.');
        }
        $this->id = $id;
        if (!$this->exists()) {
            return false;
        }
        $thread = $this->find('first', array(
            'conditions' => array('id' => $id),
            'recursive' => -1
        ));
        if (!empty($thread['Thread']['event_id'])) {
            $event = $this->Event->fetchEvent($user, array(
                'eventid' => $thread['Thread']['event_id'],
                'metadata' => true
            ));
            if (empty($event)) {
                return false;
            }
            $event = $event[0];
            // update the distribution if it diverged from the event
            if (
                $event['Event']['distribution'] != $thread['Thread']['distribution'] ||
                $event['Event']['sharing_group_id'] != $thread['Thread']['sharing_group_id']
            ) {
                $this->Behaviors->unload('SysLogLogable.SysLogLogable');
                $thread['Thread']['distribution'] = $event['Event']['distribution'];
                $thread['Thread']['sharing_group_id'] = $event['Event']['sharing_group_id'];
                $this->save($thread);
            }
            return !empty($event);
        }
        if ($user['Role']['perm_site_admin']) {
            return true;
        }
        if ($thread['Thread']['org_id'] == $user['org_id'] || ($thread['Thread']['distribution'] > 0 && $thread['Thread']['distribution'] < 4)) {
            return true;
        }
        if ($thread['Thread']['distribution'] == 4 && $this->SharingGroup->checkIfAuthorised($user, $thread['Thread']['sharing_group_id'])) {
            return true;
        }
        return false;
    }