SECURITY: Fixed several XSS vulns reported by Secunia, mostly in Private Messaging. Also backported CSRF protection API from 1.1.x, and protected Private Messaging and logout functions.
authorDan Fuhry <dan@enanocms.org>
Tue, 12 Jul 2011 22:13:37 -0400
changeset 343 7e6537fd4730
parent 342 a78b0798a116
child 344 0fa28c5aabe9
SECURITY: Fixed several XSS vulns reported by Secunia, mostly in Private Messaging. Also backported CSRF protection API from 1.1.x, and protected Private Messaging and logout functions.
includes/clientside/static/faders.js
includes/common.php
includes/functions.php
includes/sessions.php
includes/template.php
plugins/PrivateMessages.php
plugins/SpecialPageFuncs.php
plugins/SpecialUpdownload.php
plugins/SpecialUserFuncs.php
--- a/includes/clientside/static/faders.js	Tue Nov 16 12:44:22 2010 -0500
+++ b/includes/clientside/static/faders.js	Tue Jul 12 22:13:37 2011 -0400
@@ -454,7 +454,7 @@
   var mb = new messagebox(MB_YESNO|MB_ICONQUESTION, 'Are you sure you want to log out?', 'If you log out, you will no longer be able to access your user preferences, your private messages, or certain areas of this site until you log in again.');
   mb.onclick['Yes'] = function()
     {
-      window.location = makeUrlNS('Special', 'Logout/' + title);
+      window.location = makeUrlNS('Special', 'Logout/' + csrf_token + '/' + title);
     }
 }
 
--- a/includes/common.php	Tue Nov 16 12:44:22 2010 -0500
+++ b/includes/common.php	Tue Jul 12 22:13:37 2011 -0400
@@ -190,7 +190,7 @@
       unset($_COOKIE['sid']);
       setcookie('sid', '', time()-3600*24, scriptPath);
       setcookie('sid', '', time()-3600*24, scriptPath.'/');
-      die('Session cookie cleared. <a href="'.$_SERVER['PHP_SELF'].'">Continue</a>');
+      die('Session cookie cleared. <a href="'.htmlspecialchars($_SERVER['PHP_SELF']).'">Continue</a>');
       break;
   }
 }
--- a/includes/functions.php	Tue Nov 16 12:44:22 2010 -0500
+++ b/includes/functions.php	Tue Jul 12 22:13:37 2011 -0400
@@ -323,6 +323,92 @@
 
 }
 
+/**
+ * Generates a confirmation form if a CSRF check fails. Will terminate execution.
+ */
+
+function csrf_request_confirm()
+{
+	global $db, $session, $paths, $template, $plugins; // Common objects
+	
+	// If the token was overridden with the correct one, the user confirmed the action using this form. Continue exec.
+	if ( isset($_POST['cstok']) || isset($_GET['cstok']) )
+	{
+		// using the if() check makes sure that the token isn't in a cookie, since $_REQUEST includes $_COOKIE.
+		$token_check =& $_REQUEST['cstok'];
+		if ( $token_check === $session->csrf_token )
+		{
+			// overridden token matches, continue exec
+			return true;
+		}
+	}
+	
+	@ob_end_clean();
+	
+	$template->tpl_strings['PAGE_NAME'] = 'Invalid form confirmation key';
+	$template->header();
+	
+	// initial info
+	echo '<p>Your browser sent an invalid confirmation key for a form. Your session may have expired, or you may have been redirected here from a remote site in an attack known as Cross-Site Request Forgery (CSRF). If you are sure you want to continue with this action, you may click the button below. Otherwise, return to the main page and do not proceed.</p>';
+	
+	// start form
+	$form_method = ( empty($_POST) ) ? 'get' : 'post';
+	echo '<form action="' . htmlspecialchars($_SERVER['REQUEST_URI']) . '" method="' . $form_method . '" enctype="multipart/form-data">';
+	
+	echo '<fieldset enano:expand="closed">';
+	echo '<legend>View request and form data</legend><div>';
+	
+	if ( empty($_POST) )
+	{
+		// GET request
+		echo csrf_confirm_get_recursive();
+	}
+	else
+	{
+		// POST request
+		echo csrf_confirm_post_recursive();
+	}
+	echo '</div></fieldset>';
+	// insert the right CSRF token
+	echo '<input type="hidden" name="cstok" value="' . $session->csrf_token . '" />';
+	echo '<p><input type="submit" value="Continue" /></p>';
+	echo '</form><script type="text/javascript">addOnloadHook(function(){load_component(\'expander\');});</script>';
+	
+	$template->footer();
+	
+	exit;
+}
+
+function csrf_confirm_get_recursive($_inner = false, $pfx = false, $data = false)
+{
+	// make posted arrays work right
+	if ( !$data )
+		( $_inner == 'post' ) ? $data =& $_POST : $data =& $_GET;
+	foreach ( $data as $key => $value )
+	{
+		$pfx_this = ( empty($pfx) ) ? $key : "{$pfx}[{$key}]";
+		if ( is_array($value) )
+		{
+			csrf_confirm_get_recursive(true, $pfx_this, $value);
+		}
+		else if ( empty($value) )
+		{
+			echo htmlspecialchars($pfx_this . " = <nil>") . "<br />\n";
+			echo '<input type="hidden" name="' . htmlspecialchars($pfx_this) . '" value="" />';
+		}
+		else
+		{
+			echo htmlspecialchars($pfx_this . " = " . $value) . "<br />\n";
+			echo '<input type="hidden" name="' . htmlspecialchars($pfx_this) . '" value="' . htmlspecialchars($value) . '" />';
+		}
+	}
+}
+
+function csrf_confirm_post_recursive()
+{
+	csrf_confirm_get_recursive('post');
+}
+
 // Removed wikiFormat() from here, replaced with RenderMan::render
 
 /**
--- a/includes/sessions.php	Tue Nov 16 12:44:22 2010 -0500
+++ b/includes/sessions.php	Tue Jul 12 22:13:37 2011 -0400
@@ -167,6 +167,13 @@
   var $sw_timed_out = false;
   
   /**
+   * Token appended to some important forms to prevent CSRF.
+   * @var string
+   */
+	
+  var $csrf_token = false;
+  
+  /**
    * Switch to track if we're started or not.
    * @access private
    * @var bool
@@ -463,6 +470,8 @@
         $this->real_name =     $userdata['real_name'];
         $this->email =         $userdata['email'];
         $this->unread_pms =    $userdata['num_pms'];
+        // generate an anti-CSRF token
+		$this->csrf_token =    sha1($this->username . $this->sid . $this->user_id);
         if(!$this->compat)
         {
           $this->theme =         $userdata['theme'];
@@ -962,6 +971,9 @@
       $this->style = ( isset($_GET['style']) && file_exists(ENANO_ROOT.'/themes/'.$this->theme . '/css/'.$_GET['style'].'.css' )) ? $_GET['style'] : substr($template->named_theme_list[$this->theme]['default_style'], 0, strlen($template->named_theme_list[$this->theme]['default_style'])-4);
     }
     $this->user_id = 1;
+    
+    // make a CSRF token
+	$this->csrf_token = sha1($_SERVER['REMOTE_ADDR'] . '::' . sha1($this->private_key));
   }
   
   /**
@@ -999,7 +1011,7 @@
                              . '  LEFT JOIN '.table_prefix.'users_extra AS x' . "\n"
                              . '    ON ( u.user_id=x.user_id OR x.user_id IS NULL )' . "\n"
                              . '  LEFT JOIN '.table_prefix.'privmsgs AS p' . "\n"
-                             . '    ON ( p.message_to=u.username AND p.message_read=0 )' . "\n"
+                             . '    ON ( p.message_to=u.username AND p.message_read=0 AND p.folder_name != \'drafts\' )' . "\n"
                              . '  WHERE k.session_key=\''.$keyhash.'\'' . "\n"
                              . '    AND k.salt=\''.$salt.'\'' . "\n"
                              . '  GROUP BY u.user_id,u.username,u.password,u.email,u.real_name,u.user_level,u.theme,u.style,u.signature,u.reg_time,u.account_active,u.activation_key,k.source_ip,k.time,k.auth_level,x.user_id, x.user_aim, x.user_yahoo, x.user_msn, x.user_xmpp, x.user_homepage, x.user_location, x.user_job, x.user_hobbies, x.email_public;');
--- a/includes/template.php	Tue Nov 16 12:44:22 2010 -0500
+++ b/includes/template.php	Tue Jul 12 22:13:37 2011 -0400
@@ -609,7 +609,7 @@
     $parser = $this->makeParserText($tplvars['sidebar_button']);
     
     $parser->assign_vars(Array(
-        'HREF'=>makeUrlNS('Special', 'Logout'),
+        'HREF'=>makeUrlNS('Special', 'Logout/' . $session->csrf_token),
         'FLAGS'=>'onclick="if ( !KILL_SWITCH ) { mb_logout(); return false; }"',
         'TEXT'=>'Log out',
       ));
@@ -681,7 +681,8 @@
             }
           }
       $js_dynamic .= '\';
-      var ENANO_CURRENT_THEME = \''. $session->theme .'\';';
+      var ENANO_CURRENT_THEME = \''. $session->theme .'\';
+      var csrf_token = \'' . $session->csrf_token . '\';';
       foreach($paths->nslist as $k => $c)
       {
         $js_dynamic .= "namespace_list['{$k}'] = '$c';";
@@ -1680,13 +1681,13 @@
     $ob = '<div class="usermessage">'."\n";
     $s = ( $session->unread_pms == 1 ) ? '' : 's';
     $ob .= "  <b>You have $session->unread_pms <a href=" . '"' . makeUrlNS('Special', 'PrivateMessages' ) . '"' . ">unread private message$s</a>.</b><br />\n  Messages: ";
-    $q = $db->sql_query('SELECT message_id,message_from,subject,date FROM '.table_prefix.'privmsgs WHERE message_to=\'' . $session->username . '\' AND message_read=0 ORDER BY date DESC;');
+    $q = $db->sql_query('SELECT message_id,message_from,subject,date FROM '.table_prefix.'privmsgs WHERE message_to=\'' . $session->username . '\' AND message_read=0 AND folder_name != \'drafts\' ORDER BY date DESC;');
     if ( !$q )
       $db->_die();
     $messages = array();
     while ( $row = $db->fetchrow() )
     {
-      $messages[] = '<a href="' . makeUrlNS('Special', 'PrivateMessages/View/' . $row['message_id']) . '" title="Sent ' . date('F d, Y h:i a', $row['date']) . ' by ' . $row['message_from'] . '">' . $row['subject'] . '</a>';
+      $messages[] = '<a href="' . makeUrlNS('Special', 'PrivateMessages/View/' . $row['message_id']) . '" title="Sent ' . date('F d, Y h:i a', $row['date']) . ' by ' . htmlspecialchars($row['message_from']) . '">' . htmlspecialchars($row['subject']) . '</a>';
     }
     $ob .= implode(",\n    " , $messages)."\n";
     $ob .= '</div>'."\n";
--- a/plugins/PrivateMessages.php	Tue Nov 16 12:44:22 2010 -0500
+++ b/plugins/PrivateMessages.php	Tue Jul 12 22:13:37 2011 -0400
@@ -96,6 +96,7 @@
       die_friendly('Message status', '<p>Your message has been moved to the folder "'.$fname.'".</p><p><a href="'.makeUrlNS('Special', 'PrivateMessages/Folder/Inbox').'">Return to inbox</a></p>');
       break;
     case 'Delete':
+    	csrf_request_confirm();
       $id = $argv[1];
       if(!preg_match('#^([0-9]+)$#', $id)) die_friendly('Message error', '<p>Invalid message ID</p>');
       $q = $db->sql_query('SELECT message_to FROM '.table_prefix.'privmsgs WHERE message_id='.$id.'');
@@ -111,6 +112,7 @@
       if($argv[1]=='Send' && isset($_POST['_send']))
       {
         // Check each POST DATA parameter...
+        csrf_request_confirm();
         if(!isset($_POST['to']) || ( isset($_POST['to']) && $_POST['to'] == '')) die_friendly('Sending of message failed', '<p>Please enter the username to which you want to send your message.</p>');
         if(!isset($_POST['subject']) || ( isset($_POST['subject']) && $_POST['subject'] == '')) die_friendly('Sending of message failed', '<p>Please enter a subject for your message.</p>');
         if(!isset($_POST['message']) || ( isset($_POST['message']) && $_POST['message'] == '')) die_friendly('Sending of message failed', '<p>Please enter a message to send.</p>');
@@ -133,6 +135,7 @@
         return;
       } elseif($argv[1]=='Send' && isset($_POST['_savedraft'])) {
         // Check each POST DATA parameter...
+        csrf_request_confirm();
         if(!isset($_POST['to']) || ( isset($_POST['to']) && $_POST['to'] == '')) die_friendly('Sending of message failed', '<p>Please enter the username to which you want to send your message.</p>');
         if(!isset($_POST['subject']) || ( isset($_POST['subject']) && $_POST['subject'] == '')) die_friendly('Sending of message failed', '<p>Please enter a subject for your message.</p>');
         if(!isset($_POST['message']) || ( isset($_POST['message']) && $_POST['message'] == '')) die_friendly('Sending of message failed', '<p>Please enter a message to send.</p>');
@@ -192,11 +195,12 @@
         <br />
         <div class="tblholder"><table border="0" width="100%" cellspacing="1" cellpadding="4">
           <tr><th colspan="2">Compose new private message</th></tr>
-          <tr><td class="row1">To:<br /><small>Separate multiple names with a single comma; you<br />can send this message to up to <b><?php echo (string)MAX_PMS_PER_BATCH; ?></b> users.</small></td><td class="row1"><?php echo $template->username_field('to', (isset($_POST['_savedraft'])) ? $_POST['to'] : $to ); ?></td></tr>
-          <tr><td class="row2">Subject:</td><td class="row2"><input name="subject" type="text" size="30" value="<?php if(isset($_POST['_savedraft'])) echo $_POST['subject']; else echo $subj; ?>" /></td></tr>
-          <tr><td class="row1">Message:</td><td class="row1" style="min-width: 80%;"><textarea rows="20" cols="40" name="message" style="width: 100%;"><?php if(isset($_POST['_savedraft'])) echo $_POST['message']; else echo $text; ?></textarea></td></tr>
+          <tr><td class="row1">To:<br /><small>Separate multiple names with a single comma; you<br />can send this message to up to <b><?php echo (string)MAX_PMS_PER_BATCH; ?></b> users.</small></td><td class="row1"><?php echo $template->username_field('to', (isset($_POST['_savedraft'])) ? htmlspecialchars($_POST['to']) : $to ); ?></td></tr>
+          <tr><td class="row2">Subject:</td><td class="row2"><input name="subject" type="text" size="30" value="<?php if(isset($_POST['_savedraft'])) echo htmlspecialchars($_POST['subject']); else echo $subj; ?>" /></td></tr>
+          <tr><td class="row1">Message:</td><td class="row1" style="min-width: 80%;"><textarea rows="20" cols="40" name="message" style="width: 100%;"><?php if(isset($_POST['_savedraft'])) echo htmlspecialchars($_POST['message']); else echo $text; ?></textarea></td></tr>
           <tr><th colspan="2"><input type="submit" name="_send" value="Send message" />  <input type="submit" name="_savedraft" value="Save as draft" /> <input type="submit" name="_inbox" value="Back to Inbox" /></th></tr>
         </table></div>
+        <input type="hidden" name="cstok" value="<?php echo $session->csrf_token; ?>" />
         <?php
         echo '</form>';
         $template->footer();
@@ -214,6 +218,7 @@
       if(isset($_POST['_send']))
       {
         // Check each POST DATA parameter...
+        csrf_request_confirm();
         if(!isset($_POST['to']) || ( isset($_POST['to']) && $_POST['to'] == '')) die_friendly('Sending of message failed', '<p>Please enter the username to which you want to send your message.</p>');
         if(!isset($_POST['subject']) || ( isset($_POST['subject']) && $_POST['subject'] == '')) die_friendly('Sending of message failed', '<p>Please enter a subject for your message.</p>');
         if(!isset($_POST['message']) || ( isset($_POST['message']) && $_POST['message'] == '')) die_friendly('Sending of message failed', '<p>Please enter a message to send.</p>');
@@ -231,6 +236,7 @@
         return;
       } elseif(isset($_POST['_savedraft'])) {
         // Check each POST DATA parameter...
+        csrf_request_confirm();
         if(!isset($_POST['to']) || ( isset($_POST['to']) && $_POST['to'] == '')) die_friendly('Sending of message failed', '<p>Please enter the username to which you want to send your message.</p>');
         if(!isset($_POST['subject']) || ( isset($_POST['subject']) && $_POST['subject'] == '')) die_friendly('Sending of message failed', '<p>Please enter a subject for your message.</p>');
         if(!isset($_POST['message']) || ( isset($_POST['message']) && $_POST['message'] == '')) die_friendly('Sending of message failed', '<p>Please enter a message to send.</p>');
@@ -251,6 +257,7 @@
         userprefs_show_menu();
         echo '<form action="'.makeUrlNS('Special', 'PrivateMessages/Edit/'.$id).'" method="post">';
         ?>
+        <input type="hidden" name="cstok" value="<?php echo $session->csrf_token; ?>" />
         <br />
         <div class="tblholder"><table border="0" width="100%" cellspacing="1" cellpadding="4">
           <tr><th colspan="2">Edit draft</th></tr>
@@ -317,6 +324,7 @@
           if(!$q) $db->_die('The private message data could not be selected.');
           echo '<form action="'.makeUrlNS('Special', 'PrivateMessages/PostHandler').'" method="post"><div class="tblholder"><table border="0" width="100%" cellspacing="1" cellpadding="4"><tr><th colspan="4" style="text-align: left;">Folder: '.$argv[1].'</th></tr><tr><th class="subhead">';
           if($fname == 'drafts' || $fname == 'Outbox') echo 'To'; else echo 'From';
+          ?><input type="hidden" name="cstok" value="<?php echo $session->csrf_token; ?>" /><?php
           echo '</th><th class="subhead">Subject</th><th class="subhead">Date</th><th class="subhead">Mark</th></tr>';
           if($db->numrows() < 1)
             echo '<tr><td style="text-align: center;" class="row1" colspan="4">No messages in this folder.</td></tr>';
@@ -351,12 +359,16 @@
       $fname = $db->escape(strtolower($_POST['folder']));
       if($fname=='drafts' || $fname=='outbox')
       {
-        $q = $db->sql_query('SELECT p.message_id, p.message_from, p.message_to, p.date, p.subject FROM '.table_prefix.'privmsgs AS p WHERE p.folder_name=\''.$fname.'\' AND p.message_from=\''.$session->username.'\' ORDER BY date DESC;');  
+      	$fname = $fname == 'outbox' ? 'inbox' : $fname;
+      	$readsnip = $fname == 'inbox' ? ' AND message_read = 0' : '';
+        $q = $db->sql_query('SELECT p.message_id, p.message_from, p.message_to, p.date, p.subject FROM '.table_prefix.'privmsgs AS p WHERE p.folder_name=\''.$fname.'\' AND p.message_from=\''.$session->username.'\'' . $readsnip . ' ORDER BY date DESC;');
       } else {
         $q = $db->sql_query('SELECT p.message_id, p.message_from, p.message_to, p.date, p.subject FROM '.table_prefix.'privmsgs AS p WHERE p.folder_name=\''.$fname.'\' AND p.message_to=\''.$session->username.'\' ORDER BY date DESC;');
       }
       if(!$q) $db->_die('The private message data could not be selected.');
-          
+      
+      csrf_request_confirm();
+      
       if(isset($_POST['archive'])) {
         while($row = $db->fetchrow($q))
         {
@@ -373,7 +385,7 @@
           if(isset($_POST['marked_'.$row['message_id']]))
           {
             $e = $db->sql_query('DELETE FROM '.table_prefix.'privmsgs WHERE message_id='.$row['message_id'].';');
-            if(!$e) $db->_die('Message '.$row['message_id'].' was not successfully moved.');
+            if(!$e) $db->_die('Message '.$row['message_id'].' was not successfully removed.');
             $db->free_result();
           }
         }
--- a/plugins/SpecialPageFuncs.php	Tue Nov 16 12:44:22 2010 -0500
+++ b/plugins/SpecialPageFuncs.php	Tue Jul 12 22:13:37 2011 -0400
@@ -88,7 +88,7 @@
     {
       $template->header();
       
-      echo '<h3>The page could not be created.</h3><p>The name "'.$p.'" is invalid.</p>';
+      echo '<h3>The page could not be created.</h3><p>The name "'.htmlspecialchars($p).'" is invalid.</p>';
       
       $template->footer();
       $db->close();
@@ -102,7 +102,7 @@
     {
       $template->header();
       
-      echo '<h3>The page could not be created.</h3><p>The name "'.$paths->nslist[$namespace].$p.'" is invalid.</p>';
+      echo '<h3>The page could not be created.</h3><p>The name "'.$paths->nslist[$namespace].htmlspecialchars($p).'" is invalid.</p>';
       
       $template->footer();
       $db->close();
--- a/plugins/SpecialUpdownload.php	Tue Nov 16 12:44:22 2010 -0500
+++ b/plugins/SpecialUpdownload.php	Tue Jul 12 22:13:37 2011 -0400
@@ -235,7 +235,7 @@
   if ( $db->numrows() < 1 )
   {
     header('HTTP/1.1 404 Not Found');
-    die_friendly('File not found', '<p>The file "'.$filename.'" cannot be found.</p>');
+    die_friendly('File not found', '<p>The file "'.htmlspecialchars($filename).'" cannot be found.</p>');
   }
   $row = $db->fetchrow();
   $db->free_result();
--- a/plugins/SpecialUserFuncs.php	Tue Nov 16 12:44:22 2010 -0500
+++ b/plugins/SpecialUserFuncs.php	Tue Jul 12 22:13:37 2011 -0400
@@ -143,7 +143,7 @@
   }
   if ( $p = $paths->getAllParams() )
   {
-    echo '<input type="hidden" name="return_to" value="'.$p.'" />';
+    echo '<input type="hidden" name="return_to" value="'.htmlspecialchars($p).'" />';
   }
   else if ( isset($_POST['login']) && isset($_POST['return_to']) )
   {
@@ -290,7 +290,7 @@
       if(isset($_POST['return_to']))
       {
         $name = ( isset($paths->pages[$_POST['return_to']]['name']) ) ? $paths->pages[$_POST['return_to']]['name'] : $_POST['return_to'];
-        redirect( makeUrl($_POST['return_to'], false, true), 'Login successful', 'You have successfully logged into the '.getConfig('site_name').' site as "'.$session->username.'". Redirecting to ' . $name . '...' );
+        redirect( makeUrl($_POST['return_to'], false, true), 'Login successful', 'You have successfully logged into the '.getConfig('site_name').' site as "'.$session->username.'". Redirecting to ' . htmlspecialchars($name) . '...' );
       }
       else
       {
@@ -326,11 +326,17 @@
   global $db, $session, $paths, $template, $plugins; // Common objects
   if ( !$session->user_logged_in )
     $paths->main_page();
+
+  $token = $paths->getParam(0);
+  if ( $token !== $session->csrf_token )
+  	  csrf_request_confirm();
+  
+  $target_page = ($p = $paths->getParam(1)) ? $p : getConfig('main_page');
   
   $l = $session->logout();
   if ( $l == 'success' )
   {
-    redirect(makeUrl(getConfig('main_page'), false, true), 'Logged out', 'You have been successfully logged out, and all cookies have been cleared. You will now be transferred to the main page.', 4);
+    redirect(makeUrl($target_page, false, true), 'Logged out', 'You have been successfully logged out, and all cookies have been cleared. You will now be transferred to the main page.', 4);
   }
   $template->header();
   echo '<h3>An error occurred during the logout process.</h3><p>'.$l.'</p>';