CMS MADE SIMPLE FORGE

CMS Made Simple Core

 

[#10500] session security check incomplete

avatar
Created By: Fred Polizo (fredp)
Date Submitted: Thu Apr 23 05:49:26 -0400 2015

Assigned To:
Version: 1.11.13
CMSMS Version: 1.12
Severity: Minor
Resolution: None
State: Open
Summary:
session security check incomplete
Detailed Description:
CMSMS versions >= 1.11.13 may lose session variables on certain PHP
configurations. Code introduced in SVN #9825 makes an incorrect assumption about
the valid size of a session ID. This size, in fact, can vary depending on the
values of the session.hash_function and session.hash_bits_per_character PHP
configuration options. Below, I'll explain the problem in more detail and
provide a "proof of concept" of one possible solution.

Based on information from php.net
(http://php.net/manual/en/session.configuration.php#ini.session.hash-function),
if session_hash.function == 0 (MD5) and session.hash_bits_per_character == 4,
then the size of the session ID representation will be 32 digits.  When,
however, session.hash_function == sha512 and session.hash_bits_per_character ==
5, then the size of the session ID representation will be 103 digits. This means
that the current preg_match regex can fail to recognize many valid session IDs. 
Relevant code in ./include.php (introduced in SVN #9825) follows:
+if( isset($_COOKIE[$session_name]) ) {
+    // validate the contents of the cookie.
+    if (!preg_match('/^[a-zA-Z0-9,\-]{22,40}$/', $_COOKIE[$session_name]) ) {
+        session_id( uniquid() );
+        session_start();
+        session_regenerate_id();
+    }
+}
+

On my server where session.hash_function == 'sha256' and
session.hash_bits_per_character == 4, the initial symptom of this problem was
that TinyMCE textareas on Admin console pages (e.g. editcontent) would have
WYSIWYG disabled at page startup and have the sense of their 'Use WYSIWYG
editor' checkboxes reversed--unchecking the checkbox actually enables WYSIWYG
for that textarea! I eventually traced that problem to the fact that the session
variable tiny_live_textareas (set in modules/TinyMCE/TinyMCE.module.php) was
being lost before fetching the template tinyconfig.tpl (in
modules/TinyMCE/action.tinyconfig.php), which caused TinyMCE to be initialized
with an empty 'elements' option parameter and, thus, not activate any textareas
at startup.

Below is a "proof of concept" patch that I wrote which computes the expected
size of the session ID based on the current PHP config settings and uses a
session.hash_bit_per_character specific regex to check for valid hash characters
for the corresponding hash representation.

--- include.php.ORIG    2015-03-29 14:10:11.000000000 -0700
+++ include.php 2015-04-23 02:11:18.508916085 -0700
@@ -49,7 +49,35 @@
 
 if( isset($_COOKIE[$session_name]) ) {
     // validate the contents of the cookie.
-    if (!preg_match('/^[a-zA-Z0-9,\-]{22,40}$/', $_COOKIE[$session_name]) ) {
+    $hashfunc      = ini_get('session.hash_function');
+    $bits_per_char = (int)ini_get('session.hash_bits_per_character');
+    $bits_per_char = (4 <= $bits_per_char && $bits_per_char <= 6) ?
$bits_per_char : 4;
+    switch ("$hashfunc") {
+    case '0':
+        $hashfunc = 'md5';
+       break;
+    case '1':
+        $hashfunc = 'sha1';
+       break;
+    case '':
+        $hashfunc = 'sha512';
+    }
+    $bits = hash($hashfunc, '', true);
+    $nchars = ceil( 8 * strlen($bits) / $bits_per_char );
+
+    switch ($bits_per_char) {
+    case 4:
+           $char_class = '[0-9a-f]';
+           break;
+    case 5:
+           $char_class = '[0-9a-v]';
+           break;
+    case 6:
+           $char_class = '[0-9a-zA-Z,-]';
+           break;
+    }
+    $sid_re = '/^' . $char_class . '{'. $nchars . '}$/';
+    if (!preg_match($sid_re, $_COOKIE[$session_name]) ) {
         session_id( uniqid() );
         session_start();
         session_regenerate_id();


Hope this helps,
---Fred P.


History