Fix #4874

bcit-ci · Oct 27, 2016 · 2f76087 · 2f76087 · technoknol · Nov 6, 2017
1 parent 7bc8823
commit 2f76087
Show file tree

Hide file tree

Showing 2 changed files with 59 additions and 9 deletions.
diff --git a/system/libraries/Session/Session.php b/system/libraries/Session/Session.php
@@ -318,35 +318,80 @@ protected function _configure(&$params)
 		ini_set('session.use_cookies', 1);
 		ini_set('session.use_only_cookies', 1);
 
+		$this->_configure_sid_length();
+	}
+
+	// ------------------------------------------------------------------------
+
+	/**
+	 * Configure session ID length
+	 *
+	 * To make life easier, we used to force SHA-1 and 4 bits per
+	 * character on everyone. And of course, someone was unhappy.
+	 *
+	 * Then PHP 7.1 broke backwards-compatibility because ext/session
+	 * is such a mess that nobody wants to touch it with a pole stick,
+	 * and the one guy who does, nobody has the energy to argue with.
+	 *
+	 * So we were forced to make changes, and OF COURSE something was
+	 * going to break and now we have this pile of shit. -- Narf
+	 *
+	 * @return	void
+	 */
+	protected function _configure_sid_length()
+	{
 		if (PHP_VERSION_ID < 70100)
 		{
-			if ((int) ini_get('session.hash_function') === 0)
+			$hash_function = ini_get('session.hash_function');
+			if (ctype_digit($hash_function))
+			{
+				if ($hash_function !== '1')
+				{
+					ini_set('session.hash_function', 1);
+					$bits = 160;
+				}
+			}
+			elseif ( ! in_array($hash_function, hash_algos(), TRUE))
 			{
 				ini_set('session.hash_function', 1);
-				ini_set('session.hash_bits_per_character', $bits_per_character = 4);
+				$bits = 160;
 			}
-			else
+			elseif (($bits = strlen(hash($hash_function, 'dummy', false)) * 4) < 160)
 			{
-				$bits_per_character = (int) ini_get('session.hash_bits_per_character');
+				ini_set('session.hash_function', 1);
+				$bits = 160;
 			}
+
+			$bits_per_character = (int) ini_get('session.hash_bits_per_character');
+			$sid_length         = $bits * $bits_per_character;
 		}
-		elseif ((int) ini_get('session.sid_length') < 40 && ($bits_per_character = (int) ini_get('session.sid_bits_per_character')) === 4)
+		else
 		{
-			ini_set('session.sid_length', 40);
+			$bits_per_character = (int) ini_get('session.sid_bits_per_character');
+			$sid_length         = (int) ini_get('session.sid_length');
+			if (($bits = $sid_length * $bits_per_character) < 160)
+			{
+				// Add as many more characters as necessary to reach at least 160 bits
+				$sid_length += (int) ceil((160 % $bits) / $bits_per_character);
+				ini_set('session.sid_length', $sid_length);
+			}
 		}
 
+		// Yes, 4,5,6 are the only known possible values as of 2016-10-27
 		switch ($bits_per_character)
 		{
 			case 4:
-				$this->_sid_regexp = '[0-9a-f]{40,}';
+				$this->_sid_regexp = '[0-9a-f]';
 				break;
 			case 5:
-				$this->_sid_regexp = '[0-9a-v]{40,}';
+				$this->_sid_regexp = '[0-9a-v]';
 				break;
 			case 6:
-				$this->_sid_regexp = '[0-9a-zA-Z,-]{40,}';
+				$this->_sid_regexp = '[0-9a-zA-Z,-]';
 				break;
 		}
+
+		$this->_sid_regexp .= '{'.$sid_length.'}';
 	}
 
 	// ------------------------------------------------------------------------

diff --git a/user_guide_src/source/changelog.rst b/user_guide_src/source/changelog.rst
@@ -15,6 +15,11 @@ Release Date: Not Released
 
    -  Allowed PHP 4-style constructors (``Mathching_name::Matching_name()`` methods) to be used as routes, if there's a ``__construct()`` to override them.
 
+Bug fixes for 3.1.2
+-------------------
+
+-  Fixed a regression (#4874) - :doc:`Session Library <libraries/sessions>` didn't take into account `session.hash_bits_per_character` when validating session IDs.
+
 Version 3.1.1
 =============