From f06aa8058b7e32ba32d4551074b6e0b8a300f751 Mon Sep 17 00:00:00 2001 From: Thomas Bruederli <thomas@roundcube.net> Date: Mon, 21 Oct 2013 15:02:40 -0400 Subject: [PATCH] Bump version after security fix --- program/include/rcube_session.php | 162 +++++++++++++++++++++++++++++++++++------------------- 1 files changed, 105 insertions(+), 57 deletions(-) diff --git a/program/include/rcube_session.php b/program/include/rcube_session.php index a635010..e00401f 100644 --- a/program/include/rcube_session.php +++ b/program/include/rcube_session.php @@ -5,8 +5,12 @@ | program/include/rcube_session.php | | | | This file is part of the Roundcube Webmail client | - | Copyright (C) 2005-2011, The Roundcube Dev Team | - | Licensed under the GNU GPL | + | Copyright (C) 2005-2012, The Roundcube Dev Team | + | Copyright (C) 2011, Kolab Systems AG | + | | + | Licensed under the GNU General Public License version 3 or | + | any later version with exceptions for skins & plugins. | + | See the README file for a full license statement. | | | | PURPOSE: | | Provide database supported session management | @@ -36,12 +40,12 @@ private $unsets = array(); private $gc_handlers = array(); private $cookiename = 'roundcube_sessauth'; - private $vars = false; + private $vars; private $key; private $now; - private $prev; private $secret = ''; private $ip_check = false; + private $logging = false; private $keep_alive = 0; private $memcache; @@ -50,9 +54,10 @@ */ public function __construct($db, $config) { - $this->db = $db; - $this->start = microtime(true); - $this->ip = $_SERVER['REMOTE_ADDR']; + $this->db = $db; + $this->start = microtime(true); + $this->ip = $_SERVER['REMOTE_ADDR']; + $this->logging = $config->get('log_session', false); $lifetime = $config->get('session_lifetime', 1) * 60; $this->set_lifetime($lifetime); @@ -123,20 +128,19 @@ public function db_read($key) { $sql_result = $this->db->query( - "SELECT vars, ip, changed FROM ".get_table_name('session')." WHERE sess_id = ?", - $key); + "SELECT vars, ip, changed FROM ".get_table_name('session') + ." WHERE sess_id = ?", $key); - if ($sql_arr = $this->db->fetch_assoc($sql_result)) { + if ($sql_result && ($sql_arr = $this->db->fetch_assoc($sql_result))) { $this->changed = strtotime($sql_arr['changed']); $this->ip = $sql_arr['ip']; $this->vars = base64_decode($sql_arr['vars']); $this->key = $key; - if (!empty($this->vars)) - return $this->vars; + return !empty($this->vars) ? (string) $this->vars : ''; } - return false; + return null; } @@ -153,14 +157,19 @@ $ts = microtime(true); $now = $this->db->fromunixtime((int)$ts); + // no session row in DB (db_read() returns false) + if (!$this->key) { + $oldvars = null; + } // use internal data from read() for fast requests (up to 0.5 sec.) - if ($key == $this->key && (!$this->vars || $ts - $this->start < 0.5)) { + else if ($key == $this->key && (!$this->vars || $ts - $this->start < 0.5)) { $oldvars = $this->vars; - } else { // else read data again from DB + } + else { // else read data again from DB $oldvars = $this->db_read($key); } - if ($oldvars !== false) { + if ($oldvars !== null) { $newvars = $this->_fixvars($vars, $oldvars); if ($newvars !== $oldvars) { @@ -190,9 +199,7 @@ */ private function _fixvars($vars, $oldvars) { - $ts = microtime(true); - - if ($oldvars !== false) { + if ($oldvars !== null) { $a_oldvars = $this->unserialize($oldvars); if (is_array($a_oldvars)) { foreach ((array)$this->unsets as $k) @@ -214,13 +221,14 @@ * Handler for session_destroy() * * @param string Session ID + * * @return boolean True on success */ public function db_destroy($key) { - $this->db->query( - sprintf("DELETE FROM %s WHERE sess_id = ?", get_table_name('session')), - $key); + if ($key) { + $this->db->query(sprintf("DELETE FROM %s WHERE sess_id = ?", get_table_name('session')), $key); + } return true; } @@ -260,12 +268,12 @@ $this->vars = $arr['vars']; $this->key = $key; - if (!empty($this->vars)) - return $this->vars; + return !empty($this->vars) ? (string) $this->vars : ''; } - return false; + return null; } + /** * Save session data. @@ -279,29 +287,39 @@ { $ts = microtime(true); + // no session data in cache (mc_read() returns false) + if (!$this->key) + $oldvars = null; // use internal data for fast requests (up to 0.5 sec.) - if ($key == $this->key && (!$this->vars || $ts - $this->start < 0.5)) + else if ($key == $this->key && (!$this->vars || $ts - $this->start < 0.5)) $oldvars = $this->vars; else // else read data again $oldvars = $this->mc_read($key); - $newvars = $oldvars !== false ? $this->_fixvars($vars, $oldvars) : $vars; - + $newvars = $oldvars !== null ? $this->_fixvars($vars, $oldvars) : $vars; + if ($newvars !== $oldvars || $ts - $this->changed > $this->lifetime / 2) return $this->memcache->set($key, serialize(array('changed' => time(), 'ip' => $this->ip, 'vars' => $newvars)), MEMCACHE_COMPRESSED, $this->lifetime); - + return true; } + /** * Handler for session_destroy() with memcache backend * * @param string Session ID + * * @return boolean True on success */ public function mc_destroy($key) { - return $this->memcache->delete($key); + if ($key) { + // #1488592: use 2nd argument + $this->memcache->delete($key, 0); + } + + return true; } @@ -311,20 +329,7 @@ public function gc() { foreach ($this->gc_handlers as $fct) - $fct(); - } - - - /** - * Cleanup session data before saving - */ - public function cleanup() - { - // current compose information is stored in $_SESSION['compose'], move it to $_SESSION['compose_data'] - if ($_SESSION['compose']) { - $_SESSION['compose_data'][$_SESSION['compose']['id']] = $_SESSION['compose']; - $this->remove('compose'); - } + call_user_func($fct); } @@ -333,10 +338,15 @@ * * @param mixed Callback function */ - public function register_gc_handler($func_name) + public function register_gc_handler($func) { - if ($func_name && !in_array($func_name, $this->gc_handlers)) - $this->gc_handlers[] = $func_name; + foreach ($this->gc_handlers as $handler) { + if ($handler == $func) { + return; + } + } + + $this->gc_handlers[] = $func; } @@ -349,7 +359,7 @@ { session_regenerate_id($destroy); - $this->vars = false; + $this->vars = null; $this->key = session_id(); return true; @@ -372,15 +382,32 @@ return true; } - + + /** * Kill this session */ public function kill() { - $this->vars = false; + $this->vars = null; + $this->ip = $_SERVER['REMOTE_ADDR']; // update IP (might have changed) $this->destroy(session_id()); rcmail::setcookie($this->cookiename, '-del-', time() - 60); + } + + + /** + * Re-read session data from storage backend + */ + public function reload() + { + if ($this->key && $this->memcache) + $data = $this->mc_read($this->key); + else if ($this->key) + $data = $this->db_read($this->key); + + if ($data) + session_decode($data); } @@ -498,7 +525,6 @@ // valid time range is now - 1/2 lifetime to now + 1/2 lifetime $now = time(); $this->now = $now - ($now % ($this->lifetime / 2)); - $this->prev = $this->now - ($this->lifetime / 2); } /** @@ -565,13 +591,26 @@ $this->cookie = $_COOKIE[$this->cookiename]; $result = $this->ip_check ? $_SERVER['REMOTE_ADDR'] == $this->ip : true; + if (!$result) + $this->log("IP check failed for " . $this->key . "; expected " . $this->ip . "; got " . $_SERVER['REMOTE_ADDR']); + if ($result && $this->_mkcookie($this->now) != $this->cookie) { - // Check if using id from previous time slot - if ($this->_mkcookie($this->prev) == $this->cookie) - $this->set_auth_cookie(); - else - $result = false; - } + $this->log("Session auth check failed for " . $this->key . "; timeslot = " . date('Y-m-d H:i:s', $this->now)); + $result = false; + + // Check if using id from a previous time slot + for ($i = 1; $i <= 2; $i++) { + $prev = $this->now - ($this->lifetime / 2) * $i; + if ($this->_mkcookie($prev) == $this->cookie) { + $this->log("Send new auth cookie for " . $this->key . ": " . $this->cookie); + $this->set_auth_cookie(); + $result = true; + } + } + } + + if (!$result) + $this->log("Session authentication failed for " . $this->key . "; invalid auth cookie sent; timeslot = " . date('Y-m-d H:i:s', $prev)); return $result; } @@ -598,5 +637,14 @@ $auth_string = "$this->key,$this->secret,$timeslot"; return "S" . (function_exists('sha1') ? sha1($auth_string) : md5($auth_string)); } + + /** + * + */ + function log($line) + { + if ($this->logging) + write_log('session', $line); + } } -- Gitblit v1.9.1