From 916b176594bcf175417423f33711ac0cbb4082e7 Mon Sep 17 00:00:00 2001
From: Andrey Andreev <narf@devilix.net>
Date: Sat, 31 May 2014 21:00:05 +0300
Subject: Backport HMAC authentication for CI_Session

---
 system/libraries/Session.php | 56 +++++++++++++++++++++++++++-----------------
 user_guide/changelog.html    |  4 +++-
 2 files changed, 37 insertions(+), 23 deletions(-)

diff --git a/system/libraries/Session.php b/system/libraries/Session.php
index 53b914fbd..89c699765 100644
--- a/system/libraries/Session.php
+++ b/system/libraries/Session.php
@@ -144,24 +144,36 @@ class CI_Session {
 			return FALSE;
 		}
 
-		// Decrypt the cookie data
-		if ($this->sess_encrypt_cookie == TRUE)
+		// HMAC authentication
+		if (($len = strlen($session) - 40) <= 0)
 		{
-			$session = $this->CI->encrypt->decode($session);
+			log_message('error', 'Session: The session cookie was not signed.');
+			return FALSE;
 		}
-		else
+
+		// Check cookie authentication
+		$hmac = substr($session, $len);
+		$session = substr($session, 0, $len);
+
+		// Time-attack-safe comparison
+		$hmac_check = hash_hmac('sha1', $session, $this->encryption_key);
+		$diff = 0;
+		for ($i = 0; $i < 40; $i++)
 		{
-			// encryption was not used, so we need to check the md5 hash
-			$hash	 = substr($session, strlen($session)-32); // get last 32 chars
-			$session = substr($session, 0, strlen($session)-32);
+			$diff |= ord($hmac[$i]) ^ ord($hmac_check[$i]);
+		}
 
-			// 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;
-			}
+		if ($diff !== 0)
+		{
+			log_message('error', 'Session: HMAC mismatch. The session cookie data did not match what was expected.');
+			$this->sess_destroy();
+			return FALSE;
+		}
+
+		// Decrypt the cookie data
+		if ($this->sess_encrypt_cookie == TRUE)
+		{
+			$session = $this->CI->encrypt->decode($session);
 		}
 
 		// Unserialize the session array
@@ -659,20 +671,20 @@ class CI_Session {
 		else
 		{
 			// if encryption is not used, we provide an md5 hash to prevent userside tampering
-			$cookie_data = $cookie_data.md5($cookie_data.$this->encryption_key);
+			$cookie_data .= hash_hmac('sha1', $cookie_data, $this->encryption_key);
 		}
 
 		$expire = ($this->sess_expire_on_close === TRUE) ? 0 : $this->sess_expiration + time();
 
 		// Set the cookie
 		setcookie(
-					$this->sess_cookie_name,
-					$cookie_data,
-					$expire,
-					$this->cookie_path,
-					$this->cookie_domain,
-					$this->cookie_secure
-				);
+			$this->sess_cookie_name,
+			$cookie_data,
+			$expire,
+			$this->cookie_path,
+			$this->cookie_domain,
+			$this->cookie_secure
+		);
 	}
 
 	// --------------------------------------------------------------------
diff --git a/user_guide/changelog.html b/user_guide/changelog.html
index 4b3145754..ff6603d8b 100644
--- a/user_guide/changelog.html
+++ b/user_guide/changelog.html
@@ -63,7 +63,8 @@ Change Log
 <ul>
 	<li>General Changes
 		<ul>
-			<li><b>Security:</b> The <samp>xor_encode()</samp> method in the Encyption Class has been removed. The Encryption Class now requires the Mcrypt extension to be installed.</li>
+			<li><b>Security:</b> The <samp>xor_encode()</samp> method in the Encrypt Class has been removed. The Encrypt Class now requires the Mcrypt extension to be installed.</li>
+			<li><b>Security:</b> The <a href="libraries/sessions.html">Session Library</a> now uses HMAC authentication instead of a simple MD5 checksum.</li>
 		</ul>
 	</li>
 </ul>
@@ -74,6 +75,7 @@ Change Log
 	<li>Fixed a bug (#696) - make <samp>oci_execute()</samp> calls inside <samp>num_rows()</samp> non-committing, since they are only there to reset which row is next in line for oci_fetch calls and thus don't need to be committed.</li>
 	<li>Fixed a bug (#2689) - <a href="database/forge.html">Database Forge Class</a> methods <samp>create_table()</samp>, <samp>drop_table()</samp> and <samp>rename_table()</samp> produced broken SQL for tge 'sqlsrv' driver.</li>
 	<li>Fixed a bug (#2427) - PDO <a href="database/index.html">Database driver</a> didn't properly check for query failures.</li>
+	<li>Fixed a bug in the <a href="libraries/sessions.html">Session Library</a> where authentication was not performed for encrypted cookies.</li>
 </ul>
 
 <h2>Version 2.1.4</h2>
-- 
cgit v1.2.3-24-g4f1b