# HG changeset patch # User Dan Fuhry # Date 1310523217 14400 # Node ID 7e6537fd473012914d42d12ad614f30e0f404522 # Parent a78b0798a1162d347412321c3e84bacad2aa30a6 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. diff -r a78b0798a116 -r 7e6537fd4730 includes/clientside/static/faders.js --- 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); } } diff -r a78b0798a116 -r 7e6537fd4730 includes/common.php --- 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. Continue'); + die('Session cookie cleared. Continue'); break; } } diff -r a78b0798a116 -r 7e6537fd4730 includes/functions.php --- 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 '

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.

'; + + // start form + $form_method = ( empty($_POST) ) ? 'get' : 'post'; + echo '
'; + + echo '
'; + echo 'View request and form data
'; + + if ( empty($_POST) ) + { + // GET request + echo csrf_confirm_get_recursive(); + } + else + { + // POST request + echo csrf_confirm_post_recursive(); + } + echo '
'; + // insert the right CSRF token + echo ''; + echo '

'; + echo '
'; + + $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 . " = ") . "
\n"; + echo ''; + } + else + { + echo htmlspecialchars($pfx_this . " = " . $value) . "
\n"; + echo ''; + } + } +} + +function csrf_confirm_post_recursive() +{ + csrf_confirm_get_recursive('post'); +} + // Removed wikiFormat() from here, replaced with RenderMan::render /** diff -r a78b0798a116 -r 7e6537fd4730 includes/sessions.php --- 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;'); diff -r a78b0798a116 -r 7e6537fd4730 includes/template.php --- 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 = '
'."\n"; $s = ( $session->unread_pms == 1 ) ? '' : 's'; $ob .= " You have $session->unread_pms unread private message$s.
\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[] = '' . $row['subject'] . ''; + $messages[] = '' . htmlspecialchars($row['subject']) . ''; } $ob .= implode(",\n " , $messages)."\n"; $ob .= '
'."\n"; diff -r a78b0798a116 -r 7e6537fd4730 plugins/PrivateMessages.php --- 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', '

Your message has been moved to the folder "'.$fname.'".

Return to inbox

'); break; case 'Delete': + csrf_request_confirm(); $id = $argv[1]; if(!preg_match('#^([0-9]+)$#', $id)) die_friendly('Message error', '

Invalid message ID

'); $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', '

Please enter the username to which you want to send your message.

'); if(!isset($_POST['subject']) || ( isset($_POST['subject']) && $_POST['subject'] == '')) die_friendly('Sending of message failed', '

Please enter a subject for your message.

'); if(!isset($_POST['message']) || ( isset($_POST['message']) && $_POST['message'] == '')) die_friendly('Sending of message failed', '

Please enter a message to send.

'); @@ -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', '

Please enter the username to which you want to send your message.

'); if(!isset($_POST['subject']) || ( isset($_POST['subject']) && $_POST['subject'] == '')) die_friendly('Sending of message failed', '

Please enter a subject for your message.

'); if(!isset($_POST['message']) || ( isset($_POST['message']) && $_POST['message'] == '')) die_friendly('Sending of message failed', '

Please enter a message to send.

'); @@ -192,11 +195,12 @@
- - - + + +
Compose new private message
To:
Separate multiple names with a single comma; you
can send this message to up to users.
username_field('to', (isset($_POST['_savedraft'])) ? $_POST['to'] : $to ); ?>
Subject:
Message:
To:
Separate multiple names with a single comma; you
can send this message to up to users.
username_field('to', (isset($_POST['_savedraft'])) ? htmlspecialchars($_POST['to']) : $to ); ?>
Subject:
Message:
+ '; $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', '

Please enter the username to which you want to send your message.

'); if(!isset($_POST['subject']) || ( isset($_POST['subject']) && $_POST['subject'] == '')) die_friendly('Sending of message failed', '

Please enter a subject for your message.

'); if(!isset($_POST['message']) || ( isset($_POST['message']) && $_POST['message'] == '')) die_friendly('Sending of message failed', '

Please enter a message to send.

'); @@ -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', '

Please enter the username to which you want to send your message.

'); if(!isset($_POST['subject']) || ( isset($_POST['subject']) && $_POST['subject'] == '')) die_friendly('Sending of message failed', '

Please enter a subject for your message.

'); if(!isset($_POST['message']) || ( isset($_POST['message']) && $_POST['message'] == '')) die_friendly('Sending of message failed', '

Please enter a message to send.

'); @@ -251,6 +257,7 @@ userprefs_show_menu(); echo '
'; ?> +
@@ -317,6 +324,7 @@ if(!$q) $db->_die('The private message data could not be selected.'); echo '
Edit draft
'; if($db->numrows() < 1) echo ''; @@ -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(); } } diff -r a78b0798a116 -r 7e6537fd4730 plugins/SpecialPageFuncs.php --- 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 '

The page could not be created.

The name "'.$p.'" is invalid.

'; + echo '

The page could not be created.

The name "'.htmlspecialchars($p).'" is invalid.

'; $template->footer(); $db->close(); @@ -102,7 +102,7 @@ { $template->header(); - echo '

The page could not be created.

The name "'.$paths->nslist[$namespace].$p.'" is invalid.

'; + echo '

The page could not be created.

The name "'.$paths->nslist[$namespace].htmlspecialchars($p).'" is invalid.

'; $template->footer(); $db->close(); diff -r a78b0798a116 -r 7e6537fd4730 plugins/SpecialUpdownload.php --- 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', '

The file "'.$filename.'" cannot be found.

'); + die_friendly('File not found', '

The file "'.htmlspecialchars($filename).'" cannot be found.

'); } $row = $db->fetchrow(); $db->free_result(); diff -r a78b0798a116 -r 7e6537fd4730 plugins/SpecialUserFuncs.php --- 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 ''; + echo ''; } 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 '

An error occurred during the logout process.

'.$l.'

';
Folder: '.$argv[1].'
'; if($fname == 'drafts' || $fname == 'Outbox') echo 'To'; else echo 'From'; + ?>SubjectDateMark
No messages in this folder.