Fix check_request() bypass in places using get_uids() [CVE-2018-9846] (…

…#6238)
roundcube · Apr 9, 2018 · 8e543f8 · 8e543f8
1 parent 915e50a
commit 8e543f8
Show file tree

Hide file tree

Showing 9 changed files with 24 additions and 21 deletions.
diff --git a/CHANGELOG b/CHANGELOG
@@ -77,6 +77,7 @@ CHANGELOG Roundcube Webmail
 - Fix parsing date strings (e.g. from a Date: mail header) with comments (#6216)
 - Fix possible IMAP command injection and type juggling vulnerabilities (#6229)
 - Enigma: Fix key selection for signing
+- Fix check_request() bypass in places using get_uids() [CVE-2018-9846] (#6238)
 
 RELEASE 1.3.5
 -------------

diff --git a/plugins/archive/archive.php b/plugins/archive/archive.php
@@ -131,9 +131,7 @@ function move_messages()
         $archive_type   = $rcmail->config->get('archive_type', '');
         $archive_folder = $rcmail->config->get('archive_mbox');
         $archive_prefix = $archive_folder . $delimiter;
-        $current_mbox   = rcube_utils::get_input_value('_mbox', rcube_utils::INPUT_POST);
         $search_request = rcube_utils::get_input_value('_search', rcube_utils::INPUT_GPC);
-        $uids           = rcube_utils::get_input_value('_uid', rcube_utils::INPUT_POST);
 
         // count messages before changing anything
         if ($_POST['_from'] != 'show') {
@@ -153,8 +151,8 @@ function move_messages()
             'destinations' => array(),
         );
 
-        foreach (rcmail::get_uids(null, null, $multifolder) as $mbox => $uids) {
-            if (!$archive_folder  || strpos($mbox, $archive_prefix) === 0) {
+        foreach (rcmail::get_uids(null, null, $multifolder, rcube_utils::INPUT_POST) as $mbox => $uids) {
+            if (!$archive_folder || strpos($mbox, $archive_prefix) === 0) {
                 $count = count($uids);
                 continue;
             }

diff --git a/plugins/managesieve/managesieve.php b/plugins/managesieve/managesieve.php
@@ -202,9 +202,10 @@ function mail_headers($args)
      */
     function managesieve_actions()
     {
+        $uids = rcmail::get_uids(null, null, $multifolder, rcube_utils::INPUT_POST);
+
         // handle fetching email headers for the new filter form
-        if ($uid = rcube_utils::get_input_value('_uid', rcube_utils::INPUT_POST)) {
-            $uids    = rcmail::get_uids();
+        if (!empty($uids)) {
             $mailbox = key($uids);
             $message = new rcube_message($uids[$mailbox][0], $mailbox);
             $headers = $this->parse_headers($message->headers);

diff --git a/plugins/markasjunk/markasjunk.php b/plugins/markasjunk/markasjunk.php
@@ -58,7 +58,7 @@ function request_action()
         $rcmail  = rcmail::get_instance();
         $storage = $rcmail->get_storage();
 
-        foreach (rcmail::get_uids() as $mbox => $uids) {
+        foreach (rcmail::get_uids(null, null, $multifolder, rcube_utils::INPUT_POST) as $mbox => $uids) {
             $storage->unset_flag($uids, 'NONJUNK', $mbox);
             $storage->set_flag($uids, 'JUNK', $mbox);
         }

diff --git a/plugins/zipdownload/zipdownload.php b/plugins/zipdownload/zipdownload.php
@@ -178,8 +178,8 @@ public function download_messages()
     {
         $rcmail = rcmail::get_instance();
 
-        if ($rcmail->config->get('zipdownload_selection', $this->default_limit) && !empty($_POST['_uid'])) {
-            $messageset = rcmail::get_uids();
+        if ($rcmail->config->get('zipdownload_selection', $this->default_limit)) {
+            $messageset = rcmail::get_uids(null, null, $multi, rcube_utils::INPUT_POST);
             if (count($messageset)) {
                 $this->_download_messages($messageset);
             }

diff --git a/program/include/rcmail.php b/program/include/rcmail.php
@@ -2402,16 +2402,17 @@ public function message_part_size($part)
      * @param string $uids           UID value to decode
      * @param string $mbox           Default mailbox value (if not encoded in UIDs)
      * @param bool   $is_multifolder Will be set to True if multi-folder request
+     * @param int    $mode           Request mode. Default: rcube_utils::INPUT_GPC.
      *
      * @return array  List of message UIDs per folder
      */
-    public static function get_uids($uids = null, $mbox = null, &$is_multifolder = false)
+    public static function get_uids($uids = null, $mbox = null, &$is_multifolder = false, $mode = null)
     {
         // message UID (or comma-separated list of IDs) is provided in
         // the form of <ID>-<MBOX>[,<ID>-<MBOX>]*
 
-        $_uid  = $uids ?: rcube_utils::get_input_value('_uid', rcube_utils::INPUT_GPC);
-        $_mbox = $mbox ?: (string) rcube_utils::get_input_value('_mbox', rcube_utils::INPUT_GPC);
+        $_uid  = $uids ?: rcube_utils::get_input_value('_uid', $mode ?: rcube_utils::INPUT_GPC);
+        $_mbox = $mbox ?: (string) rcube_utils::get_input_value('_mbox', $mode ?: rcube_utils::INPUT_GPC);
 
         // already a hash array
         if (is_array($_uid) && !isset($_uid[0])) {
@@ -2430,8 +2431,9 @@ public static function get_uids($uids = null, $mbox = null, &$is_multifolder = f
             }
         }
         else {
-            if (is_string($_uid))
+            if (is_string($_uid)) {
                 $_uid = explode(',', $_uid);
+            }
 
             // create a per-folder UIDs array
             foreach ((array)$_uid as $uid) {
@@ -2446,7 +2448,7 @@ public static function get_uids($uids = null, $mbox = null, &$is_multifolder = f
                 if ($uid == '*') {
                     $result[$mbox] = $uid;
                 }
-                else {
+                else if (preg_match('/^[0-9:.]+$/', $uid)) {
                     $result[$mbox][] = $uid;
                 }
             }

diff --git a/program/steps/mail/copy.inc b/program/steps/mail/copy.inc
@@ -29,7 +29,7 @@ if (!empty($_POST['_uid']) && strlen($_POST['_target_mbox'])) {
     $target  = rcube_utils::get_input_value('_target_mbox', rcube_utils::INPUT_POST, true);
     $sources = array();
 
-    foreach (rcmail::get_uids(null, null, $multifolder) as $mbox => $uids) {
+    foreach (rcmail::get_uids(null, null, $multifolder, rcube_utils::INPUT_POST) as $mbox => $uids) {
         if ($mbox === $target) {
             $copied++;
         }

diff --git a/program/steps/mail/mark.inc b/program/steps/mail/mark.inc
@@ -65,7 +65,7 @@ if ($_uids && $flag) {
         $input = array($mbox => '*');
     }
     else {
-        $input = rcmail::get_uids();
+        $input = rcmail::get_uids(null, null, $dummy, rcube_utils::INPUT_POST);
     }
 
     foreach ($input as $mbox => $uids) {
@@ -88,9 +88,10 @@ if ($_uids && $flag) {
     }
 
     if ($flag == 'DELETED' && $read_deleted && !empty($_POST['_ruid'])) {
-        $ruids = rcube_utils::get_input_value('_ruid', rcube_utils::INPUT_POST);
-        foreach (rcmail::get_uids($ruids) as $mbox => $uids) {
-            $read += (int)$RCMAIL->storage->set_flag($uids, 'SEEN', $mbox);
+        if ($ruids = rcube_utils::get_input_value('_ruid', rcube_utils::INPUT_POST)) {
+            foreach (rcmail::get_uids($ruids) as $mbox => $uids) {
+                $read += (int)$RCMAIL->storage->set_flag($uids, 'SEEN', $mbox);
+            }
         }
 
         if ($read && !$skip_deleted) {

diff --git a/program/steps/mail/move_del.inc b/program/steps/mail/move_del.inc
@@ -39,7 +39,7 @@ if ($RCMAIL->action == 'move' && !empty($_POST['_uid']) && strlen($_POST['_targe
     $target  = rcube_utils::get_input_value('_target_mbox', rcube_utils::INPUT_POST, true);
     $success = true;
 
-    foreach (rcmail::get_uids(null, null, $multifolder) as $mbox => $uids) {
+    foreach (rcmail::get_uids(null, null, $multifolder, rcube_utils::INPUT_POST) as $mbox => $uids) {
         if ($mbox === $target) {
             $count += is_array($uids) ? count($uids) : 1;
         }
@@ -75,7 +75,7 @@ if ($RCMAIL->action == 'move' && !empty($_POST['_uid']) && strlen($_POST['_targe
 }
 // delete messages
 else if ($RCMAIL->action == 'delete' && !empty($_POST['_uid'])) {
-    foreach (rcmail::get_uids(null, null, $multifolder) as $mbox => $uids) {
+    foreach (rcmail::get_uids(null, null, $multifolder, rcube_utils::INPUT_POST) as $mbox => $uids) {
         $del      += (int)$RCMAIL->storage->delete_message($uids, $mbox);
         $count    += is_array($uids) ? count($uids) : 1;
         $sources[] = $mbox;