From 74c5f2668d31f7384ea5f014014356144059cbf3 Mon Sep 17 00:00:00 2001 From: Tyler Brownell Date: Fri, 13 Dec 2013 00:23:12 -0500 Subject: Issue #2763 - Fixes Session GC Probability Calculation This should resolve issue #2763 where the cookie session garbage collection was running every request. --- system/libraries/Session/drivers/Session_cookie.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'system/libraries/Session/drivers/Session_cookie.php') diff --git a/system/libraries/Session/drivers/Session_cookie.php b/system/libraries/Session/drivers/Session_cookie.php index d3d22d03a..cd8074474 100644 --- a/system/libraries/Session/drivers/Session_cookie.php +++ b/system/libraries/Session/drivers/Session_cookie.php @@ -841,7 +841,7 @@ class CI_Session_cookie extends CI_Session_driver { $probability = ini_get('session.gc_probability'); $divisor = ini_get('session.gc_divisor'); - if ((mt_rand(0, $divisor) / $divisor) < $probability) + if (mt_rand(1, $divisor) <= $probability) { $expire = $this->now - $this->sess_expiration; $this->CI->db->delete($this->sess_table_name, 'last_activity < '.$expire); -- cgit v1.2.3-24-g4f1b From 5d6b9c597a9870f55a65bcfcb301d19d83447078 Mon Sep 17 00:00:00 2001 From: Jordan Eldredge Date: Sat, 21 Dec 2013 13:56:41 -0800 Subject: Remove unneeded manual escaping of session data --- .../libraries/Session/drivers/Session_cookie.php | 63 ++-------------------- 1 file changed, 3 insertions(+), 60 deletions(-) (limited to 'system/libraries/Session/drivers/Session_cookie.php') diff --git a/system/libraries/Session/drivers/Session_cookie.php b/system/libraries/Session/drivers/Session_cookie.php index cd8074474..124e0098e 100644 --- a/system/libraries/Session/drivers/Session_cookie.php +++ b/system/libraries/Session/drivers/Session_cookie.php @@ -739,86 +739,29 @@ class CI_Session_cookie extends CI_Session_driver { /** * Serialize an array * - * This function first converts any slashes found in the array to a temporary - * marker, so when it gets unserialized the slashes will be preserved + * This function serializes an array * * @param mixed Data to serialize * @return string Serialized data */ protected function _serialize($data) { - if (is_array($data)) - { - array_walk_recursive($data, array(&$this, '_escape_slashes')); - } - elseif (is_string($data)) - { - $data = str_replace('\\', '{{slash}}', $data); - } - return serialize($data); } // ------------------------------------------------------------------------ - /** - * Escape slashes - * - * This function converts any slashes found into a temporary marker - * - * @param string Value - * @param string Key - * @return void - */ - protected function _escape_slashes(&$val, $key) - { - if (is_string($val)) - { - $val = str_replace('\\', '{{slash}}', $val); - } - } - - // ------------------------------------------------------------------------ - /** * Unserialize * - * This function unserializes a data string, then converts any - * temporary slash markers back to actual slashes + * This function unserializes a data string * * @param mixed Data to unserialize * @return mixed Unserialized data */ protected function _unserialize($data) { - $data = @unserialize(trim($data)); - - if (is_array($data)) - { - array_walk_recursive($data, array(&$this, '_unescape_slashes')); - return $data; - } - - return is_string($data) ? str_replace('{{slash}}', '\\', $data) : $data; - } - - // ------------------------------------------------------------------------ - - /** - * Unescape slashes - * - * This function converts any slash markers back into actual slashes - * - * @param string Value - * @param string Key - * @return void - */ - protected function _unescape_slashes(&$val, $key) - { - if (is_string($val)) - { - $val = str_replace('{{slash}}', '\\', $val); - } + return @unserialize(trim($data)); } // ------------------------------------------------------------------------ -- cgit v1.2.3-24-g4f1b From 5306cad2e40596a3a6fcac787e54689a7095e769 Mon Sep 17 00:00:00 2001 From: Jordan Eldredge Date: Mon, 23 Dec 2013 11:10:51 -0800 Subject: Remove _serialize() and _unserialize() methods Since removing the unneeded manual escaping code, there is no-longer any reason to have the serialization functions abstracted. This also allows us to only suppress errors when unserializing cookie data, and only trim when we are unserializing database data (see commit 6b8312). --- .../libraries/Session/drivers/Session_cookie.php | 38 +++------------------- 1 file changed, 4 insertions(+), 34 deletions(-) (limited to 'system/libraries/Session/drivers/Session_cookie.php') diff --git a/system/libraries/Session/drivers/Session_cookie.php b/system/libraries/Session/drivers/Session_cookie.php index 124e0098e..dc75d8e8e 100644 --- a/system/libraries/Session/drivers/Session_cookie.php +++ b/system/libraries/Session/drivers/Session_cookie.php @@ -397,7 +397,7 @@ class CI_Session_cookie extends CI_Session_driver { } // Unserialize the session array - $session = $this->_unserialize($session); + $session = @unserialize($session); // Is the session data we unserialized an array with the correct format? if ( ! is_array($session) OR ! isset($session['session_id'], $session['ip_address'], $session['user_agent'], $session['last_activity'])) @@ -472,7 +472,7 @@ class CI_Session_cookie extends CI_Session_driver { $row = $query->row(); if ( ! empty($row->user_data)) { - $custom_data = $this->_unserialize($row->user_data); + $custom_data = unserialize(trim($row->user_data)); if (is_array($custom_data)) { @@ -608,7 +608,7 @@ class CI_Session_cookie extends CI_Session_driver { if ( ! empty($userdata)) { // Serialize the custom data array so we can store it - $set['user_data'] = $this->_serialize($userdata); + $set['user_data'] = serialize($userdata); } // Reset query builder values. @@ -696,7 +696,7 @@ class CI_Session_cookie extends CI_Session_driver { : $this->userdata; // Serialize the userdata for the cookie - $cookie_data = $this->_serialize($cookie_data); + $cookie_data = serialize($cookie_data); if ($this->sess_encrypt_cookie === TRUE) { @@ -736,36 +736,6 @@ class CI_Session_cookie extends CI_Session_driver { // ------------------------------------------------------------------------ - /** - * Serialize an array - * - * This function serializes an array - * - * @param mixed Data to serialize - * @return string Serialized data - */ - protected function _serialize($data) - { - return serialize($data); - } - - // ------------------------------------------------------------------------ - - /** - * Unserialize - * - * This function unserializes a data string - * - * @param mixed Data to unserialize - * @return mixed Unserialized data - */ - protected function _unserialize($data) - { - return @unserialize(trim($data)); - } - - // ------------------------------------------------------------------------ - /** * Garbage collection * -- cgit v1.2.3-24-g4f1b From bfb635b276d880336db795f1a603de66ccfc80f6 Mon Sep 17 00:00:00 2001 From: Andrey Andreev Date: Wed, 8 Jan 2014 18:32:05 +0200 Subject: Make newline standardization configurable Added ['standardize_newlines'] Also altered the Session cookie driver, which experienced issues with this feature due to it's HMAC verification failing after the Input class alters newlines in non-encrypted session cookies. Supersedes PR #2470 --- .../libraries/Session/drivers/Session_cookie.php | 25 +++++++++++++++++++++- 1 file changed, 24 insertions(+), 1 deletion(-) (limited to 'system/libraries/Session/drivers/Session_cookie.php') diff --git a/system/libraries/Session/drivers/Session_cookie.php b/system/libraries/Session/drivers/Session_cookie.php index dc75d8e8e..65debcb44 100644 --- a/system/libraries/Session/drivers/Session_cookie.php +++ b/system/libraries/Session/drivers/Session_cookie.php @@ -165,6 +165,8 @@ class CI_Session_cookie extends CI_Session_driver { */ public $now; + // ------------------------------------------------------------------------ + /** * Default userdata keys * @@ -184,6 +186,15 @@ class CI_Session_cookie extends CI_Session_driver { */ protected $data_dirty = FALSE; + /** + * Standardize newlines flag + * + * @var bool + */ + protected $_standardize_newlines; + + // ------------------------------------------------------------------------ + /** * Initialize session driver object * @@ -209,9 +220,11 @@ class CI_Session_cookie extends CI_Session_driver { 'sess_time_to_update', 'time_reference', 'cookie_prefix', - 'encryption_key' + 'encryption_key', ); + $this->_standardize_newlines = (bool) $config['standardize_newlines']; + foreach ($prefs as $key) { $this->$key = isset($this->_parent->params[$key]) @@ -695,6 +708,16 @@ class CI_Session_cookie extends CI_Session_driver { ? array_intersect_key($this->userdata, $this->defaults) : $this->userdata; + // The Input class will do this and since we use HMAC verification, + // unless we standardize here as well, the hash won't match. + if ($this->_standardize_newlines) + { + foreach (array_keys($this->userdata) as $key) + { + $this->userdata[$key] = preg_replace('/(?:\r\n|[\r\n])/', PHP_EOL, $this->userdata[$key]); + } + } + // Serialize the userdata for the cookie $cookie_data = serialize($cookie_data); -- cgit v1.2.3-24-g4f1b From 4ea76cc2216b19bfae38dbbfe7104c21ee278d81 Mon Sep 17 00:00:00 2001 From: Andrey Andreev Date: Wed, 8 Jan 2014 21:49:23 +0200 Subject: Fix 2 errors caused by recent commits --- system/libraries/Session/drivers/Session_cookie.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'system/libraries/Session/drivers/Session_cookie.php') diff --git a/system/libraries/Session/drivers/Session_cookie.php b/system/libraries/Session/drivers/Session_cookie.php index 65debcb44..971dfeabe 100644 --- a/system/libraries/Session/drivers/Session_cookie.php +++ b/system/libraries/Session/drivers/Session_cookie.php @@ -223,7 +223,7 @@ class CI_Session_cookie extends CI_Session_driver { 'encryption_key', ); - $this->_standardize_newlines = (bool) $config['standardize_newlines']; + $this->_standardize_newlines = (bool) config_item('standardize_newlines'); foreach ($prefs as $key) { -- cgit v1.2.3-24-g4f1b From 4a2918a33c756ac7cc9defc2e6acd371e4412af6 Mon Sep 17 00:00:00 2001 From: Andrey Andreev Date: Wed, 5 Feb 2014 01:03:46 +0200 Subject: Integrate CI_Encryption into the framework TODO: Add documentation in user_guide_src/source/libraries/encryption.rst --- .../libraries/Session/drivers/Session_cookie.php | 55 ++++++++++++---------- 1 file changed, 30 insertions(+), 25 deletions(-) (limited to 'system/libraries/Session/drivers/Session_cookie.php') diff --git a/system/libraries/Session/drivers/Session_cookie.php b/system/libraries/Session/drivers/Session_cookie.php index 971dfeabe..5d338fc04 100644 --- a/system/libraries/Session/drivers/Session_cookie.php +++ b/system/libraries/Session/drivers/Session_cookie.php @@ -240,7 +240,7 @@ class CI_Session_cookie extends CI_Session_driver { // Do we need encryption? If so, load the encryption class if ($this->sess_encrypt_cookie === TRUE) { - $this->CI->load->library('encrypt'); + $this->CI->load->library('encryption'); } // Check for database @@ -383,30 +383,33 @@ class CI_Session_cookie extends CI_Session_driver { return FALSE; } - $len = strlen($session) - 40; - - if ($len < 0) + if ($this->sess_encrypt_cookie === TRUE) { - log_message('debug', 'The session cookie was not signed.'); - return FALSE; + $session = $this->CI->encryption->decrypt($session); + if ($session === FALSE) + { + log_message('error', 'Session: Unable to decrypt the session cookie, possibly due to a HMAC mismatch.'); + return FALSE; + } } - - // Check cookie authentication - $hmac = substr($session, $len); - $session = substr($session, 0, $len); - - if ($hmac !== hash_hmac('sha1', $session, $this->encryption_key)) + else { - log_message('error', 'The session cookie data did not match what was expected.'); - $this->sess_destroy(); - return FALSE; - } + if (($len = strlen($session) - 40) <= 0) + { + log_message('error', 'Session: The session cookie was not signed.'); + return FALSE; + } - // Check for encryption - if ($this->sess_encrypt_cookie === TRUE) - { - // Decrypt the cookie data - $session = $this->CI->encrypt->decode($session); + // Check cookie authentication + $hmac = substr($session, $len); + $session = substr($session, 0, $len); + + if ($hmac !== hash_hmac('sha1', $session, $this->encryption_key)) + { + log_message('error', 'Session: HMAC mismatch. The session cookie data did not match what was expected.'); + $this->sess_destroy(); + return FALSE; + } } // Unserialize the session array @@ -723,11 +726,13 @@ class CI_Session_cookie extends CI_Session_driver { if ($this->sess_encrypt_cookie === TRUE) { - $cookie_data = $this->CI->encrypt->encode($cookie_data); + $cookie_data = $this->CI->encryption->encrypt($cookie_data); + } + else + { + // Require message authentication + $cookie_data .= hash_hmac('sha1', $cookie_data, $this->encryption_key); } - - // Require message authentication - $cookie_data .= hash_hmac('sha1', $cookie_data, $this->encryption_key); $expire = ($this->sess_expire_on_close === TRUE) ? 0 : $this->sess_expiration + time(); -- cgit v1.2.3-24-g4f1b From 3aa781a65267d72000009df0fa2feee5cb3bdd8d Mon Sep 17 00:00:00 2001 From: Andrey Andreev Date: Thu, 6 Feb 2014 05:34:19 +0200 Subject: Make CI_Session's HMAC comparison time-attack-safe --- system/libraries/Session/drivers/Session_cookie.php | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) (limited to 'system/libraries/Session/drivers/Session_cookie.php') diff --git a/system/libraries/Session/drivers/Session_cookie.php b/system/libraries/Session/drivers/Session_cookie.php index 971dfeabe..c8dfad6c9 100644 --- a/system/libraries/Session/drivers/Session_cookie.php +++ b/system/libraries/Session/drivers/Session_cookie.php @@ -395,7 +395,15 @@ class CI_Session_cookie extends CI_Session_driver { $hmac = substr($session, $len); $session = substr($session, 0, $len); - if ($hmac !== hash_hmac('sha1', $session, $this->encryption_key)) + // Time-attack-safe comparison + $hmac_check = hash_hmac('sha1', $session, $this->encryption_key); + $diff = 0; + for ($i = 0; $i < 40; $i++) + { + $diff |= ord($hmac[$i]) ^ ord($hmac_check[$i]); + } + + if ($diff !== 0) { log_message('error', 'The session cookie data did not match what was expected.'); $this->sess_destroy(); -- cgit v1.2.3-24-g4f1b From 871754af60251993d640981e107d2def5f2db396 Mon Sep 17 00:00:00 2001 From: darwinel Date: Tue, 11 Feb 2014 17:34:57 +0100 Subject: 2013 > 2014 Update copyright notices from 2013 to 2014. And update one calendar example in user_guide from year 2013/2014 to 2014/2015. --- system/libraries/Session/drivers/Session_cookie.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'system/libraries/Session/drivers/Session_cookie.php') diff --git a/system/libraries/Session/drivers/Session_cookie.php b/system/libraries/Session/drivers/Session_cookie.php index 79712ad94..566c40bd8 100644 --- a/system/libraries/Session/drivers/Session_cookie.php +++ b/system/libraries/Session/drivers/Session_cookie.php @@ -18,7 +18,7 @@ * * @package CodeIgniter * @author EllisLab Dev Team - * @copyright Copyright (c) 2008 - 2013, EllisLab, Inc. (http://ellislab.com/) + * @copyright Copyright (c) 2008 - 2014, EllisLab, Inc. (http://ellislab.com/) * @license http://opensource.org/licenses/OSL-3.0 Open Software License (OSL 3.0) * @link http://codeigniter.com * @since Version 1.0 -- cgit v1.2.3-24-g4f1b