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 | 89 +++++++++++++++++++++++++++----------------- 1 files changed, 54 insertions(+), 35 deletions(-) diff --git a/program/include/rcube_session.php b/program/include/rcube_session.php index 1f5290c..e00401f 100644 --- a/program/include/rcube_session.php +++ b/program/include/rcube_session.php @@ -5,7 +5,7 @@ | program/include/rcube_session.php | | | | This file is part of the Roundcube Webmail client | - | Copyright (C) 2005-2011, The Roundcube Dev Team | + | Copyright (C) 2005-2012, The Roundcube Dev Team | | Copyright (C) 2011, Kolab Systems AG | | | | Licensed under the GNU General Public License version 3 or | @@ -40,10 +40,9 @@ 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; @@ -138,11 +137,10 @@ $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; } @@ -161,7 +159,7 @@ // no session row in DB (db_read() returns false) if (!$this->key) { - $oldvars = false; + $oldvars = null; } // use internal data from read() for fast requests (up to 0.5 sec.) else if ($key == $this->key && (!$this->vars || $ts - $this->start < 0.5)) { @@ -171,7 +169,7 @@ $oldvars = $this->db_read($key); } - if ($oldvars !== false) { + if ($oldvars !== null) { $newvars = $this->_fixvars($vars, $oldvars); if ($newvars !== $oldvars) { @@ -201,7 +199,7 @@ */ private function _fixvars($vars, $oldvars) { - if ($oldvars !== false) { + if ($oldvars !== null) { $a_oldvars = $this->unserialize($oldvars); if (is_array($a_oldvars)) { foreach ((array)$this->unsets as $k) @@ -223,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; } @@ -269,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. @@ -290,30 +289,37 @@ // no session data in cache (mc_read() returns false) if (!$this->key) - $oldvars = false; + $oldvars = null; // use internal data for fast requests (up to 0.5 sec.) 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; } @@ -332,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; } @@ -348,7 +359,7 @@ { session_regenerate_id($destroy); - $this->vars = false; + $this->vars = null; $this->key = session_id(); return true; @@ -371,13 +382,15 @@ 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); } @@ -512,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); } /** @@ -583,15 +595,22 @@ $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(); + $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; + } } - else { - $result = false; - $this->log("Session authentication failed for " . $this->key . "; invalid auth cookie sent"); - } - } + } + + 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; } -- Gitblit v1.9.1