Updating the cookie driver to use HMAC authentication on all cookie data.

Signed-off-by: Pascal Kriete <pascal@pascalkriete.com>

2 files changed, 28 insertions, 19 deletions

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();
 
diff --git a/user_guide_src/source/changelog.rst b/user_guide_src/source/changelog.rst
index 811aa8faf..165ff8ca0 100644
--- a/user_guide_src/source/changelog.rst
+++ b/user_guide_src/source/changelog.rst
@@ -159,6 +159,8 @@ Release Date: Not Released
 	 -  New tempdata feature allows setting user data items with an expiration time.
 	 -  Added default $config['sess_driver'] and $config['sess_valid_drivers'] items to config.php file.
 	 -  Cookie driver now respects php.ini's session.gc_probability and session.gc_divisor
+    -  The Cookie driver now uses HMAC authentication instead of the simple md5 checksum.
+    -  The Cookie driver now also checks authentication on encrypted session data.
 	 -  Changed the Cookie driver to select only one row when using database sessions.
 	 -  Cookie driver now only writes to database at end of request when using database.
 	 -  Cookie driver now uses PHP functions for faster array manipulation when using database.