From 197203727417a03d87053a47e5aa5175a76e3e0b Mon Sep 17 00:00:00 2001 From: Aleksander Machniak <alec@alec.pl> Date: Thu, 17 Oct 2013 04:24:53 -0400 Subject: [PATCH] Fix vulnerability in handling _session argument of utils/save-prefs (#1489382) --- program/include/rcube_user.php | 309 ++++++++++++++++++++++++++++++++++++++++----------- 1 files changed, 242 insertions(+), 67 deletions(-) diff --git a/program/include/rcube_user.php b/program/include/rcube_user.php index b1263ca..500b922 100644 --- a/program/include/rcube_user.php +++ b/program/include/rcube_user.php @@ -4,8 +4,8 @@ +-----------------------------------------------------------------------+ | program/include/rcube_user.inc | | | - | This file is part of the RoundCube Webmail client | - | Copyright (C) 2005-2010, RoundCube Dev. - Switzerland | + | This file is part of the Roundcube Webmail client | + | Copyright (C) 2005-2010, The Roundcube Dev Team | | Licensed under the GNU GPL | | | | PURPOSE: | @@ -29,22 +29,38 @@ */ class rcube_user { - public $ID = null; - public $data = null; - public $language = null; + public $ID; + public $data; + public $language; - private $db = null; + /** + * Holds database connection. + * + * @var rcube_mdb2 + */ + private $db; + /** + * rcmail object. + * + * @var rcmail + */ + private $rc; + + const SEARCH_ADDRESSBOOK = 1; + const SEARCH_MAIL = 2; /** * Object constructor * - * @param object DB Database connection + * @param int $id User id + * @param array $sql_arr SQL result set */ function __construct($id = null, $sql_arr = null) { - $this->db = rcmail::get_instance()->get_dbh(); - + $this->rc = rcmail::get_instance(); + $this->db = $this->rc->get_dbh(); + if ($id && !$sql_arr) { $sql_result = $this->db->query( "SELECT * FROM ".get_table_name('users')." WHERE user_id = ?", $id); @@ -62,7 +78,7 @@ /** * Build a user name string (as e-mail address) * - * @param string Username part (empty or 'local' or 'domain') + * @param string $part Username part (empty or 'local' or 'domain') * @return string Full user name or its part */ function get_username($part = null) @@ -74,9 +90,10 @@ if ($part == 'local') { return $local; } - // if no domain was provided use the default if available - if (empty($domain)) - $domain = $this->data['mail_host']; + // if no domain was provided... + if (empty($domain)) { + $domain = $this->rc->config->mail_domain($this->data['mail_host']); + } if ($part == 'domain') { return $domain; @@ -102,8 +119,25 @@ if (!empty($this->language)) $prefs = array('language' => $this->language); - if ($this->ID && $this->data['preferences']) - $prefs += (array)unserialize($this->data['preferences']); + if ($this->ID) { + // Preferences from session (write-master is unavailable) + if (!empty($_SESSION['preferences'])) { + // Check last write attempt time, try to write again (every 5 minutes) + if ($_SESSION['preferences_time'] < time() - 5 * 60) { + $saved_prefs = unserialize($_SESSION['preferences']); + $this->rc->session->remove('preferences'); + $this->rc->session->remove('preferences_time'); + $this->save_prefs($saved_prefs); + } + else { + $this->data['preferences'] = $_SESSION['preferences']; + } + } + + if ($this->data['preferences']) { + $prefs += (array)unserialize($this->data['preferences']); + } + } return $prefs; } @@ -112,21 +146,21 @@ /** * Write the given user prefs to the user's record * - * @param array User prefs to save + * @param array $a_user_prefs User prefs to save * @return boolean True on success, False on failure */ function save_prefs($a_user_prefs) { if (!$this->ID) return false; - - $config = rcmail::get_instance()->config; + + $config = $this->rc->config; $old_prefs = (array)$this->get_prefs(); // merge (partial) prefs array with existing settings $save_prefs = $a_user_prefs + $old_prefs; unset($save_prefs['language']); - + // don't save prefs with default values if they haven't been changed yet foreach ($a_user_prefs as $key => $value) { if (!isset($old_prefs[$key]) && ($value == $config->get($key))) @@ -146,10 +180,25 @@ $this->language = $_SESSION['language']; - if ($this->db->affected_rows()) { + // Update success + if ($this->db->affected_rows() !== false) { $config->set_user_prefs($a_user_prefs); $this->data['preferences'] = $save_prefs; + + if (isset($_SESSION['preferences'])) { + $this->rc->session->remove('preferences'); + $this->rc->session->remove('preferences_time'); + } return true; + } + // Update error, but we are using replication (we have read-only DB connection) + // and we are storing session not in the SQL database + // we can store preferences in session and try to write later (see get_prefs()) + else if ($this->db->is_replicated() && $config->get('session_storage', 'db') != 'db') { + $_SESSION['preferences'] = $save_prefs; + $_SESSION['preferences_time'] = time(); + $config->set_user_prefs($a_user_prefs); + $this->data['preferences'] = $save_prefs; } return false; @@ -159,7 +208,7 @@ /** * Get default identity of this user * - * @param int Identity ID. If empty, the default identity is returned + * @param int $id Identity ID. If empty, the default identity is returned * @return array Hash array with all cols of the identity record */ function get_identity($id = null) @@ -172,6 +221,7 @@ /** * Return a list of all identities linked with this user * + * @param string $sql_add Optional WHERE clauses * @return array List of identities */ function list_identities($sql_add = '') @@ -184,11 +234,11 @@ ($sql_add ? " ".$sql_add : ""). " ORDER BY ".$this->db->quoteIdentifier('standard')." DESC, name ASC, identity_id ASC", $this->ID); - + while ($sql_arr = $this->db->fetch_assoc($sql_result)) { $result[] = $sql_arr; } - + return $result; } @@ -196,8 +246,8 @@ /** * Update a specific identity record * - * @param int Identity ID - * @param array Hash array with col->value pairs to save + * @param int $iid Identity ID + * @param array $data Hash array with col->value pairs to save * @return boolean True if saved successfully, false if nothing changed */ function update_identity($iid, $data) @@ -206,7 +256,7 @@ return false; $query_cols = $query_params = array(); - + foreach ((array)$data as $col => $value) { $query_cols[] = $this->db->quoteIdentifier($col) . ' = ?'; $query_params[] = $value; @@ -222,15 +272,15 @@ call_user_func_array(array($this->db, 'query'), array_merge(array($sql), $query_params)); - + return $this->db->affected_rows(); } - - + + /** * Create a new identity record linked with this user * - * @param array Hash array with col->value pairs to save + * @param array $data Hash array with col->value pairs to save * @return int The inserted identity ID or false on error */ function insert_identity($data) @@ -257,12 +307,12 @@ return $this->db->insert_id('identities'); } - - + + /** * Mark the given identity as deleted * - * @param int Identity ID + * @param int $iid Identity ID * @return boolean True if deleted successfully, false if nothing changed */ function delete_identity($iid) @@ -279,8 +329,8 @@ // we'll not delete last identity if ($sql_arr['ident_count'] <= 1) - return false; - + return -1; + $this->db->query( "UPDATE ".get_table_name('identities'). " SET del = 1, changed = ".$this->db->now(). @@ -291,12 +341,12 @@ return $this->db->affected_rows(); } - - + + /** * Make this identity the default one for this user * - * @param int The identity ID + * @param int $iid The identity ID */ function set_default($iid) { @@ -311,8 +361,8 @@ $iid); } } - - + + /** * Update user's last_login timestamp */ @@ -326,8 +376,8 @@ $this->ID); } } - - + + /** * Clear the saved object state */ @@ -336,43 +386,43 @@ $this->ID = null; $this->data = null; } - - + + /** * Find a user record matching the given name and host * - * @param string IMAP user name - * @param string IMAP host name - * @return object rcube_user New user instance + * @param string $user IMAP user name + * @param string $host IMAP host name + * @return rcube_user New user instance */ static function query($user, $host) { $dbh = rcmail::get_instance()->get_dbh(); - + // query for matching user name $query = "SELECT * FROM ".get_table_name('users')." WHERE mail_host = ? AND %s = ?"; $sql_result = $dbh->query(sprintf($query, 'username'), $host, $user); - + // query for matching alias if (!($sql_arr = $dbh->fetch_assoc($sql_result))) { $sql_result = $dbh->query(sprintf($query, 'alias'), $host, $user); $sql_arr = $dbh->fetch_assoc($sql_result); } - + // user already registered -> overwrite username if ($sql_arr) return new rcube_user($sql_arr['user_id'], $sql_arr); else return false; } - - + + /** * Create a new user record and return a rcube_user instance * - * @param string IMAP user name - * @param string IMAP host - * @return object rcube_user New user instance + * @param string $user IMAP user name + * @param string $host IMAP host + * @return rcube_user New user instance */ static function create($user, $host) { @@ -385,8 +435,8 @@ $user_email = is_array($email_list[0]) ? $email_list[0]['email'] : $email_list[0]; } - $data = $rcmail->plugins->exec_hook('create_user', - array('user'=>$user, 'user_name'=>$user_name, 'user_email'=>$user_email)); + $data = $rcmail->plugins->exec_hook('user_create', + array('user'=>$user, 'user_name'=>$user_name, 'user_email'=>$user_email, 'host'=>$host)); // plugin aborted this operation if ($data['abort']) @@ -404,7 +454,7 @@ strip_newlines($user), strip_newlines($host), strip_newlines($data['alias'] ? $data['alias'] : $user_email), - $_SESSION['language']); + strip_newlines($data['language'] ? $data['language'] : $_SESSION['language'])); if ($user_id = $dbh->insert_id('users')) { // create rcube_user instance to make plugin hooks work @@ -444,9 +494,9 @@ $record['user_id'] = $user_id; $record['standard'] = $standard; - $plugin = $rcmail->plugins->exec_hook('create_identity', + $plugin = $rcmail->plugins->exec_hook('identity_create', array('login' => true, 'record' => $record)); - + if (!$plugin['abort'] && $plugin['record']['email']) { $rcmail->user->insert_identity($plugin['record']); } @@ -461,15 +511,15 @@ 'file' => __FILE__, 'message' => "Failed to create new user"), true, false); } - + return $user_id ? $user_instance : false; } - - + + /** * Resolve username using a virtuser plugins * - * @param string E-mail address to resolve + * @param string $email E-mail address to resolve * @return string Resolved IMAP username */ static function email2user($email) @@ -485,9 +535,9 @@ /** * Resolve e-mail address from virtuser plugins * - * @param string User name - * @param boolean If true returns first found entry - * @param boolean If true returns email as array (email and name for identity) + * @param string $user User name + * @param boolean $first If true returns first found entry + * @param boolean $extended If true returns email as array (email and name for identity) * @return mixed Resolved e-mail address string or array of strings */ static function user2email($user, $first=true, $extended=false) @@ -500,4 +550,129 @@ return empty($plugin['email']) ? NULL : $plugin['email']; } + + /** + * Return a list of saved searches linked with this user + * + * @param int $type Search type + * + * @return array List of saved searches indexed by search ID + */ + function list_searches($type) + { + $plugin = $this->rc->plugins->exec_hook('saved_search_list', array('type' => $type)); + + if ($plugin['abort']) { + return (array) $plugin['result']; + } + + $result = array(); + + $sql_result = $this->db->query( + "SELECT search_id AS id, ".$this->db->quoteIdentifier('name') + ." FROM ".get_table_name('searches') + ." WHERE user_id = ?" + ." AND ".$this->db->quoteIdentifier('type')." = ?" + ." ORDER BY ".$this->db->quoteIdentifier('name'), + (int) $this->ID, (int) $type); + + while ($sql_arr = $this->db->fetch_assoc($sql_result)) { + $sql_arr['data'] = unserialize($sql_arr['data']); + $result[$sql_arr['id']] = $sql_arr; + } + + return $result; + } + + + /** + * Return saved search data. + * + * @param int $id Row identifier + * + * @return array Data + */ + function get_search($id) + { + $plugin = $this->rc->plugins->exec_hook('saved_search_get', array('id' => $id)); + + if ($plugin['abort']) { + return $plugin['result']; + } + + $sql_result = $this->db->query( + "SELECT ".$this->db->quoteIdentifier('name') + .", ".$this->db->quoteIdentifier('data') + .", ".$this->db->quoteIdentifier('type') + ." FROM ".get_table_name('searches') + ." WHERE user_id = ?" + ." AND search_id = ?", + (int) $this->ID, (int) $id); + + while ($sql_arr = $this->db->fetch_assoc($sql_result)) { + return array( + 'id' => $id, + 'name' => $sql_arr['name'], + 'type' => $sql_arr['type'], + 'data' => unserialize($sql_arr['data']), + ); + } + + return null; + } + + + /** + * Deletes given saved search record + * + * @param int $sid Search ID + * + * @return boolean True if deleted successfully, false if nothing changed + */ + function delete_search($sid) + { + if (!$this->ID) + return false; + + $this->db->query( + "DELETE FROM ".get_table_name('searches') + ." WHERE user_id = ?" + ." AND search_id = ?", + (int) $this->ID, $sid); + + return $this->db->affected_rows(); + } + + + /** + * Create a new saved search record linked with this user + * + * @param array $data Hash array with col->value pairs to save + * + * @return int The inserted search ID or false on error + */ + function insert_search($data) + { + if (!$this->ID) + return false; + + $insert_cols[] = 'user_id'; + $insert_values[] = (int) $this->ID; + $insert_cols[] = $this->db->quoteIdentifier('type'); + $insert_values[] = (int) $data['type']; + $insert_cols[] = $this->db->quoteIdentifier('name'); + $insert_values[] = $data['name']; + $insert_cols[] = $this->db->quoteIdentifier('data'); + $insert_values[] = serialize($data['data']); + + $sql = "INSERT INTO ".get_table_name('searches') + ." (".join(', ', $insert_cols).")" + ." VALUES (".join(', ', array_pad(array(), sizeof($insert_values), '?')).")"; + + call_user_func_array(array($this->db, 'query'), + array_merge(array($sql), $insert_values)); + + return $this->db->insert_id('searches'); + } + } -- Gitblit v1.9.1