From f69f0e8f02815d44e218b013c8da92cebabbdcb1 Mon Sep 17 00:00:00 2001 From: Pascal Kriete Date: Tue, 16 Oct 2012 11:54:49 -0400 Subject: Updating the cookie driver to use HMAC authentication on all cookie data. Signed-off-by: Pascal Kriete --- .../libraries/Session/drivers/Session_cookie.php | 45 +++++++++++++--------- 1 file changed, 26 insertions(+), 19 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 5bb1f7aa6..b44c8330e 100755 --- a/system/libraries/Session/drivers/Session_cookie.php +++ b/system/libraries/Session/drivers/Session_cookie.php @@ -372,27 +372,31 @@ class CI_Session_cookie extends CI_Session_driver { return FALSE; } + $len = strlen($session) - 40; + + if ($len < 0) + { + log_message('debug', 'The session cookie was not signed.'); + return FALSE; + } + + // Check cookie authentication + $hmac = substr($session, $len); + $session = substr($session, 0, $len); + + if ($hmac !== hash_hmac('sha1', $session, $this->encryption_key)) + { + log_message('error', 'The session cookie data did not match what was expected. This could be a possible hacking attempt.'); + $this->sess_destroy(); + return FALSE; + } + // Check for encryption if ($this->sess_encrypt_cookie === TRUE) { // Decrypt the cookie data $session = $this->CI->encrypt->decode($session); } - else - { - // Encryption was not used, so we need to check the md5 hash in the last 32 chars - $len = strlen($session)-32; - $hash = substr($session, $len); - $session = substr($session, 0, $len); - - // Does the md5 hash match? This is to prevent manipulation of session data in userspace - if ($hash !== md5($session.$this->encryption_key)) - { - log_message('error', 'The session cookie data did not match what was expected. This could be a possible hacking attempt.'); - $this->sess_destroy(); - return FALSE; - } - } // Unserialize the session array $session = $this->_unserialize($session); @@ -658,10 +662,13 @@ class CI_Session_cookie extends CI_Session_driver { // Serialize the userdata for the cookie $cookie_data = $this->_serialize($cookie_data); - $cookie_data = ($this->sess_encrypt_cookie === TRUE) - ? $this->CI->encrypt->encode($cookie_data) - // if encryption is not used, we provide an md5 hash to prevent userside tampering - : $cookie_data.md5($cookie_data.$this->encryption_key); + if ($this->sess_encrypt_cookie === TRUE) + { + $this->CI->encrypt->encode($cookie_data); + } + + // 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 28dc2023d32e1d997e2b90052f1960f98a255d2c Mon Sep 17 00:00:00 2001 From: Pascal Kriete Date: Wed, 17 Oct 2012 11:27:29 -0400 Subject: Changing session error logging verbiage to be a little less unsettling. Signed-off-by: Pascal Kriete --- 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 b44c8330e..51d94da4e 100755 --- a/system/libraries/Session/drivers/Session_cookie.php +++ b/system/libraries/Session/drivers/Session_cookie.php @@ -386,7 +386,7 @@ class CI_Session_cookie extends CI_Session_driver { if ($hmac !== hash_hmac('sha1', $session, $this->encryption_key)) { - log_message('error', 'The session cookie data did not match what was expected. This could be a possible hacking attempt.'); + log_message('error', 'The session cookie data did not match what was expected.'); $this->sess_destroy(); return FALSE; } -- cgit v1.2.3-24-g4f1b From cf264e0d165647f30efdef1b2d944849bebf4c72 Mon Sep 17 00:00:00 2001 From: Andrey Andreev Date: Thu, 18 Oct 2012 16:14:51 +0300 Subject: Fix Session cookies not being encrypted on creation and sess_destroy() not actually deleting cookies --- system/libraries/Session/drivers/Session_cookie.php | 4 ++-- 1 file changed, 2 insertions(+), 2 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 51d94da4e..8617aec2d 100755 --- a/system/libraries/Session/drivers/Session_cookie.php +++ b/system/libraries/Session/drivers/Session_cookie.php @@ -308,7 +308,7 @@ class CI_Session_cookie extends CI_Session_driver { } // Kill the cookie - $this->_setcookie($this->sess_cookie_name, addslashes(serialize(array())), ($this->now - 31500000), + $this->_setcookie($this->sess_cookie_name, '', ($this->now - 31500000), $this->cookie_path, $this->cookie_domain, 0); // Kill session data @@ -664,7 +664,7 @@ class CI_Session_cookie extends CI_Session_driver { if ($this->sess_encrypt_cookie === TRUE) { - $this->CI->encrypt->encode($cookie_data); + $cookie_data = $this->CI->encrypt->encode($cookie_data); } // Require message authentication -- cgit v1.2.3-24-g4f1b