diff options
-rw-r--r-- | system/libraries/Session.php | 56 | ||||
-rw-r--r-- | 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> |