diff --git a/CHANGELOG.txt b/CHANGELOG.txt index 8f0c8a866788123663fbc1ccd19a5a0766b90a79..38047809a0197c93a5d30d680c56f1c76d340dbe 100644 --- a/CHANGELOG.txt +++ b/CHANGELOG.txt @@ -1,3 +1,9 @@ +Drupal 4.6.10, 2006-10-18 +------------------------ +- fixed security issue (XSS), see SA-2006-024 +- fixed security issue (CSRF), see SA-2006-025 +- fixed security issue (Form action attribute injection), see SA-2006-026 + Drupal 4.6.9, 2006-08-02 ------------------------ - fixed security issue (XSS), see SA-2006-011 diff --git a/includes/common.inc b/includes/common.inc index a8770225c3d985302f4dd1c1db3d09ac3f44a56f..5fee3d6e7486fb43abd0a438b4c11fd590e1b251 100644 --- a/includes/common.inc +++ b/includes/common.inc @@ -911,6 +911,61 @@ function format_name($object) { * @} End of "defgroup format". */ +/** + * Check the form token if there is POST data for an authenticated user to defend against cross site request forgeries. + * + * $_POST will be cleared if the token is absent or incorrect. + * + */ +function drupal_check_token() { + global $user; + if ($user->uid && ($_SERVER['REQUEST_METHOD'] == 'POST') && !(isset($_POST['edit']) && isset($_POST['edit']['token']) && drupal_valid_token($_POST['edit']['token']))) { + drupal_set_message(t('Validation error. Please try again.'), 'error'); + $_POST = array(); + } +} + +/** + * Ensure the private key variable used to generate tokens is set. + * + * @return + * The private key + */ +function drupal_get_private_key() { + if (!($key = variable_get('drupal_private_key', 0))) { + $key = mt_rand(); + variable_set('drupal_private_key', $key); + } + return $key; +} + +/** + * Generate a token based on $value, the current user session and private key. + * + * @param $value + * An additional value to base the token on + */ +function drupal_get_token($value = '') { + $private_key = drupal_get_private_key(); + return md5(session_id() . $value . $private_key); +} + +/** + * Validate a token based on $value, the current user session and private key or penultimate private key. + * + * @param $token + * The token to be validated. + * @param $value + * An additional value to base the token on + * @return + * True for a valid token, False for an invalid token. + */ +function drupal_valid_token($token, $value = '') { + return ($token == md5(session_id() . $value . variable_get('drupal_private_key', ''))); +} + + + /** * @defgroup form Form generation * @{ @@ -921,6 +976,13 @@ function format_name($object) { * must be explicitly generated by modules. */ +/** + * Generate a form token based on the session and the private key to defend against cross site request forgeries. + */ +function form_token() { + return form_hidden('token', drupal_get_token()); +} + /** * Generate a form from a set of form elements. * @@ -939,7 +1001,7 @@ function form($form, $method = 'post', $action = NULL, $attributes = NULL) { if (!$action) { $action = request_uri(); } - return '
\n". $form ."\n
\n"; + return '
\n". $form . form_token() ."\n
\n"; } /** @@ -1600,7 +1662,7 @@ function drupal_xml_parser_create(&$data) { $data = ereg_replace('^(<\?xml[^>]+encoding)="([^"]+)"', '\\1="utf-8"', $out); } else { - watchdog('php', t("Could not convert XML encoding '%s' to UTF-8.", array('%s' => $encoding)), WATCHDOG_WARNING); + watchdog('php', t("Could not convert XML encoding '%s' to UTF-8.", array('%s' => theme('placeholder', $encoding))), WATCHDOG_WARNING); return 0; } } diff --git a/index.php b/index.php index 89f8354f96a9061d142b6a8e30794782e4b5cedb..22d1b689833273bddde7fe4e47061f716a784584 100644 --- a/index.php +++ b/index.php @@ -14,6 +14,7 @@ include_once 'includes/common.inc'; fix_gpc_magic(); +drupal_check_token(); $status = menu_execute_active_handler(); switch ($status) { diff --git a/modules/block.module b/modules/block.module index 290a93e00970d04d64db3bb1b5949fff27f6f675..89afdcee0df04897410cc7cd4753144e9945d87c 100644 --- a/modules/block.module +++ b/modules/block.module @@ -110,6 +110,7 @@ function block_block($op = 'list', $delta = 0, $edit = array()) { } function block_admin_save($edit) { + unset($edit['token']); foreach ($edit as $module => $blocks) { foreach ($blocks as $delta => $block) { db_query("UPDATE {blocks} SET region = %d, status = %d, weight = %d, throttle = %d WHERE module = '%s' AND delta = '%s'", diff --git a/modules/blog.module b/modules/blog.module index 913ecbdfa424ea084823b7ced201afea0268f36e..e6b15952ca643e6c3783ea466293bb70ae673ef0 100644 --- a/modules/blog.module +++ b/modules/blog.module @@ -125,7 +125,7 @@ function blog_page_user($uid) { $account = user_load(array((is_numeric($uid) ? 'uid' : 'name') => $uid, 'status' => 1)); if ($account->uid) { - drupal_set_title($title = t("%name's blog", array('%name' => $account->name))); + drupal_set_title($title = t("%name's blog", array('%name' => check_plain($account->name)))); if (($account->uid == $user->uid) && user_access('edit own blog')) { $output = '
  • '. l(t('Post new blog entry.'), "node/add/blog") .'
    • '; diff --git a/modules/comment.module b/modules/comment.module index c60652d063015c96e4ba48adad3568d565e42413..430cb4cd1842cf9c311d304819fe566ab305560d 100644 --- a/modules/comment.module +++ b/modules/comment.module @@ -761,7 +761,7 @@ function comment_render($node, $cid = 0) { if ((comment_user_can_moderate($node)) && $user->uid != $comment->uid && !(comment_already_moderated($user->uid, $comment->users))) { $output .= '
    '. form_submit(t('Moderate comment')) .'

    '; } - $output .= ''; + $output .= '' . form_token() . ''; } else { // Multiple comment view @@ -863,7 +863,7 @@ function comment_render($node, $cid = 0) { $output .= '
    \n"; $output .= theme('comment_controls', $threshold, $mode, $order, $comments_per_page); $output .= form_hidden('nid', $nid); - $output .= '
    '; + $output .= '' . form_token() . ''; } $output .= '
    \n"; @@ -898,13 +898,13 @@ function comment_render($node, $cid = 0) { $output .= '
    '. form_submit(t('Moderate comments')) .'
    '; } - $output .= '
    '; + $output .= '' . form_token() . ''; if (db_num_rows($result) && (variable_get('comment_controls', 3) == 1 || variable_get('comment_controls', 3) == 2)) { $output .= '
    \n"; $output .= theme('comment_controls', $threshold, $mode, $order, $comments_per_page); $output .= form_hidden('nid', $nid); - $output .= '
    '; + $output .= '' . form_token() . ''; } } diff --git a/modules/contact.module b/modules/contact.module index 28b9fae05659e8526be3f2a168d0d7c5050ed557..628793d771f7b093055efe3e8193dc377ec148b9 100644 --- a/modules/contact.module +++ b/modules/contact.module @@ -55,10 +55,10 @@ function contact_mail_user() { if ($account = user_load(array('uid' => arg(1), 'status' => 1))) { if (!$account->contact && !user_access('administer users')) { - $output = t('%name is not accepting e-mails.', array('%name' => $account->name)); + $output = t('%name is not accepting e-mails.', array('%name' => check_plain($account->name))); } else if (!$user->uid) { - $output = t('Please login or register to send %name a message.', array('%login' => url('user/login'), '%register' => url('user/register'), '%name' => $account->name)); + $output = t('Please login or register to send %name a message.', array('%login' => url('user/login'), '%register' => url('user/register'), '%name' => check_plain($account->name))); } else if (!valid_email_address($user->mail)) { $output = t('You need to provide a valid e-mail address to contact other users. Please edit your user information.', array('%url' => url("user/$user->uid/edit"))); @@ -106,7 +106,7 @@ function contact_mail_user() { // Log the operation: flood_register_event('contact'); - watchdog('mail', t('%name-from sent %name-to an e-mail.', array('%name-from' => $user->name, '%name-to' => $account->name))); + watchdog('mail', t('%name-from sent %name-to an e-mail.', array('%name-from' => theme('placeholder', $user->name), '%name-to' => theme('placeholder', $account->name)))); // Set a status message: drupal_set_message(t('Your message has been sent.')); @@ -119,8 +119,8 @@ function contact_mail_user() { $edit['mail'] = $user->mail; } - $output = form_item(t('From'), $user->name .' <'. $user->mail .'>'); - $output .= form_item(t('To'), $account->name); + $output = form_item(t('From'), check_plain($user->name) .' <'. $user->mail .'>'); + $output .= form_item(t('To'), check_plain($account->name)); $output .= form_textfield(t('Subject'), 'subject', $edit['subject'], 50, 50, NULL, NULL, TRUE); $output .= form_textarea(t('Message'), 'message', $edit['message'], 70, 8, NULL, NULL, TRUE); $output .= form_submit(t('Send e-mail')); diff --git a/modules/filter.module b/modules/filter.module index 654030a3e50feaec37f62023e2132c07ecff8dfa..568cc962a3a3c66bfd798572985bb9e605500f36 100644 --- a/modules/filter.module +++ b/modules/filter.module @@ -1235,15 +1235,21 @@ function filter_xss_bad_protocol($string, $decode = TRUE) { if ($decode) { $string = decode_entities($string); } - // Remove soft hyphen - $string = str_replace(chr(194) . chr(173), '', $string); - // Strip protocols + // Iteratively remove any invalid protocol found. do { $before = $string; $colonpos = strpos($string, ':'); if ($colonpos > 0) { + // We found a colon, possibly a protocol. Verify. $protocol = substr($string, 0, $colonpos); + // If a colon is preceded by a slash, question mark or hash, it cannot + // possibly be part of the URL scheme. This must be a relative URL, + // which inherits the (safe) protocol of the base document. + if (preg_match('![/?#]!', $protocol)) { + break; + } + // Check if this is a disallowed protocol if (!isset($allowed_protocols[$protocol])) { $string = substr($string, $colonpos + 1); } diff --git a/modules/forum.module b/modules/forum.module index 071e8fe533ff6effaaf66dc72bc2d2cad08f2f84..eb5689a42cbdb1a5a4333ec62d2a89f81768d54e 100644 --- a/modules/forum.module +++ b/modules/forum.module @@ -777,7 +777,7 @@ function theme_forum_display($forums, $topics, $parents, $tid, $sortby, $forum_p } } - drupal_set_title($title); + drupal_set_title(check_plain($title)); $breadcrumb[] = array('path' => $_GET['q']); menu_set_location($breadcrumb); diff --git a/modules/locale.module b/modules/locale.module index f5c45ef9f238e6b8f71373b49b735d0147ca0de8..38c2e8b54a8b4510d5d55bfa3c9324c507463a57 100644 --- a/modules/locale.module +++ b/modules/locale.module @@ -430,9 +430,23 @@ function locale_admin_string() { $edit =& $_POST['edit']; switch ($op) { + case t('Delete'): case 'delete': - $output .= _locale_string_delete(db_escape_string(arg(4))); - $output .= _locale_string_seek(); + if($edit['confirm']) { + $output .= _locale_string_delete(db_escape_string(arg(4))); + $output .= _locale_string_seek(); + drupal_goto('admin/locale/string/search'); + } + else { + $string = db_result(db_query("SELECT source FROM {locales_source} WHERE lid = %d", arg(4))); + $output = theme('confirm', + t('Are you sure you want to delete the following string?'), + 'admin/locale/string/search', + t('This action cannot be undone.'), + t('Delete'), + t('Cancel'), + check_plain($string)); + } break; case 'edit': $output .= _locale_string_edit(db_escape_string(arg(4))); diff --git a/modules/menu.module b/modules/menu.module index 26ac4e1278d5b4454c097622c57c8cdfeeb9d434..b29b28590ae378ad64534d3e7e27b4e76fb76362 100644 --- a/modules/menu.module +++ b/modules/menu.module @@ -212,14 +212,26 @@ function menu_delete_item($mid) { * Menu callback; hide a menu item. */ function menu_disable_item($mid) { - $menu = menu_get_menu(); - $type = $menu['items'][$mid]['type']; - $type &= ~MENU_VISIBLE_IN_TREE; - $type &= ~MENU_VISIBLE_IN_BREADCRUMB; - $type |= MENU_MODIFIED_BY_ADMIN; - db_query('UPDATE {menu} SET type = %d WHERE mid = %d', $type, $mid); - drupal_set_message(t('Menu item disabled.')); - drupal_goto('admin/menu'); + $op = $_POST['op']; + $menu = menu_get_menu(); + switch ($op) { + case t('Disable'): + $type = $menu['items'][$mid]['type']; + $type &= ~MENU_VISIBLE_IN_TREE; + $type &= ~MENU_VISIBLE_IN_BREADCRUMB; + $type |= MENU_MODIFIED_BY_ADMIN; + db_query('UPDATE {menu} SET type = %d WHERE mid = %d', $type, $mid); + drupal_set_message(t('Menu item disabled.')); + drupal_goto('admin/menu'); + break; + default: + $output = theme('confirm', + t('Are you sure you want disable %menu-item?', array('%menu-item' => theme('placeholder', $menu['items'][$mid]['title']))), + 'admin/menu', + ' ', + t('Disable')); + print theme('page', $output); + } } /** diff --git a/modules/node.module b/modules/node.module index 6c2a2ccb8d973d7c50f8e691d64d23db7618cc86..fe17353211046d8d88e6af1ac1f231bd38391f33 100644 --- a/modules/node.module +++ b/modules/node.module @@ -705,6 +705,16 @@ function node_menu($may_cache) { 'access' => user_access('administer nodes'), 'weight' => 2, 'type' => MENU_LOCAL_TASK); + $items[] = array('path' => 'node/'. arg(1) .'/rollback-revision', 'title' => t('Revert revision'), + 'callback' => 'node_page', + 'access' => user_access('administer nodes'), + 'weight' => 1, + 'type' => MENU_CALLBACK); + $items[] = array('path' => 'node/'. arg(1) .'/delete-revision', 'title' => t('Delete revision'), + 'callback' => 'node_page', + 'access' => user_access('administer nodes'), + 'weight' => 1, + 'type' => MENU_CALLBACK); } } } @@ -1026,31 +1036,33 @@ function node_revision_create($node) { */ function node_revision_rollback($nid, $revision) { global $user; - if (user_access('administer nodes')) { $node = node_load(array('nid' => $nid)); - // Extract the specified revision: $rev = $node->revisions[$revision]['node']; - - // Inherit all the past revisions: - $rev->revisions = $node->revisions; - - // Save the original/current node: - $rev->revisions[] = array('uid' => $user->uid, 'timestamp' => time(), 'node' => $node); - - // Remove the specified revision: - unset($rev->revisions[$revision]); - - // Save the node: - foreach ($node as $key => $value) { - $filter[] = $key; + if ($_POST['edit']['confirm']) { + // Inherit all the past revisions: + $rev->revisions = $node->revisions; + // Save the original/current node: + $rev->revisions[] = array('uid' => $user->uid, 'timestamp' => time(), 'node' => $node); + // Remove the specified revision: + unset($rev->revisions[$revision]); + // Save the node: + foreach ($node as $key => $value) { + $filter[] = $key; + } + node_save($rev, $filter); + drupal_set_message(t('Rolled back to revision %revision of %title', array('%revision' => "#$revision", '%title' => theme('placeholder', $node->title)))); + drupal_goto('node/'. $nid .'/revisions'); + } + else { + $output = theme('confirm', + t('Are you sure you want to revert %title? to the revision from %revision-date?', array('%title' => theme('placeholder', $node->title), '%revision-date' => theme('placeholder', format_date($node->revisions[$revision]['timestamp'])))), + 'node/'. $nid .'/revisions', + t('This action cannot be undone.'), + t('Revert')); + print theme('page', $output); } - - node_save($rev, $filter); - - drupal_set_message(t('Rolled back to revision %revision of %title', array('%revision' => "#$revision", '%title' => theme('placeholder', $node->title)))); - drupal_goto('node/'. $nid .'/revisions'); } } @@ -1060,18 +1072,27 @@ function node_revision_rollback($nid, $revision) { function node_revision_delete($nid, $revision) { if (user_access('administer nodes')) { $node = node_load(array('nid' => $nid)); + if ($_POST['edit']['confirm']) { + unset($node->revisions[$revision]); - unset($node->revisions[$revision]); + // If the array is empty, replace the array by an empty string, or + // else we'll generate an SQL warning when we try to save the node. + if (count($node->revisions) == 0) { + $node->revisions = ''; + } + node_save($node, array('nid', 'revisions')); - // If the array is empty, replace the array by an empty string, or - // else we'll generate an SQL warning when we try to save the node. - if (count($node->revisions) == 0) { - $node->revisions = ''; + drupal_set_message(t('Deleted revision %revision of %title', array('%revision' => "#$revision", '%title' => theme('placeholder', $node->title)))); + drupal_goto('node/'. $nid . (count($node->revisions) ? '/revisions' : '')); + } + else { + $output = theme('confirm', + t('Are you sure you want to delete the revision of %title from %revision-date?', array('%title' => theme('placeholder', $node->title), '%revision-date' => theme('placeholder', format_date($node->revisions[$revision]['timestamp'])))), + 'node/'. $nid .'/revisions', + t('This action cannot be undone.'), + t('Delete revision')); + print theme('page', $output); } - node_save($node, array('nid', 'revisions')); - - drupal_set_message(t('Deleted revision %revision of %title', array('%revision' => "#$revision", '%title' => theme('placeholder', $node->title)))); - drupal_goto('node/'. $nid . (count($node->revisions) ? '/revisions' : '')); } } @@ -1655,6 +1676,7 @@ function node_page_default() { * Menu callback; dispatches control to the appropriate operation handler. */ function node_page() { + global $user; $op = $_POST['op'] ? $_POST['op'] : arg(1); $edit = $_POST['edit']; @@ -1675,9 +1697,11 @@ function node_page() { case 'revisions': print theme('page', node_revision_overview(arg(1))); break; + case t('Revert'): case 'rollback-revision': node_revision_rollback(arg(1), arg(3)); break; + case t('Delete revision'): case 'delete-revision': node_revision_delete(arg(1), arg(3)); break; diff --git a/modules/path.module b/modules/path.module index 56ee2f644b231fa4b4f3d236d775a846b0fdc214..728748fc8daa8aca6b492108985a37200a2d6081 100644 --- a/modules/path.module +++ b/modules/path.module @@ -107,7 +107,7 @@ function path_admin_edit($pid = 0) { } elseif ($pid) { $alias = path_load($pid); - drupal_set_title($alias['dst']); + drupal_set_title(check_plain($alias['dst'])); $output = path_form(path_load($pid)); } else { diff --git a/modules/profile.module b/modules/profile.module index 91de92598281150908d5e51865327a52ef5e64f7..fd70572332f55fba3ecb2723433f12ca35f5231b 100644 --- a/modules/profile.module +++ b/modules/profile.module @@ -117,7 +117,7 @@ function profile_browse() { } $output .= ''; - drupal_set_title($title); + drupal_set_title(check_plain($title)); print theme('page', $output); } else if ($name && !$field->id) { @@ -491,7 +491,7 @@ function profile_admin_edit($fid) { $data = db_fetch_array(db_query('SELECT * FROM {profile_fields} WHERE fid = %d', $fid)); } - drupal_set_title(t('Edit %type', array('%type' => $data['type']))); + drupal_set_title(t('Edit %type', array('%type' => check_plain($data['type'])))); print theme('page', _profile_field_form($data['type'], $data)); } diff --git a/modules/statistics.module b/modules/statistics.module index 3d9b5cebed0b06b917249d3441781c6d2c91f7c9..413eef3b4d58d2c9a47181a6301d41eb72a6432b 100644 --- a/modules/statistics.module +++ b/modules/statistics.module @@ -211,7 +211,7 @@ function statistics_user_tracker() { $rows[] = array(array('data' => $pager, 'colspan' => '3')); } - drupal_set_title($account->name); + drupal_set_title(check_plain($account->name)); print theme('page', theme('table', $header, $rows)); } else { diff --git a/modules/tracker.module b/modules/tracker.module index 33bdaec1e20e5c65fa0ddf0b957b6aac14f7678b..e68a4dd05a1e09c9782c21202a6e6a554395afc4 100644 --- a/modules/tracker.module +++ b/modules/tracker.module @@ -59,7 +59,7 @@ function tracker_menu($may_cache) { */ function tracker_track_user() { if ($account = user_load(array('uid' => arg(1)))) { - drupal_set_title($account->name); + drupal_set_title(check_plain($account->name)); tracker_page($account->uid); } } diff --git a/modules/user.module b/modules/user.module index 7cfd400b549d387f6b5894911385196872c4a18f..430c691034b95c414e1cbdd86c59f0322ef52f8c 100644 --- a/modules/user.module +++ b/modules/user.module @@ -215,6 +215,18 @@ function user_validate_name($name) { if (substr($name, -1) == ' ') return t('The username cannot end with a space.'); if (ereg(' ', $name)) return t('The username cannot contain multiple spaces in a row.'); if (ereg("[^\x80-\xF7 [:alnum:]@_.-]", $name)) return t('The username contains an illegal character.'); + if (preg_match('/[\x{80}-\x{A0}'. // Non-printable ISO-8859-1 + NBSP + '\x{AD}'. // Soft-hyphen + '\x{2000}-\x{200F}'. // Various space characters + '\x{2028}-\x{202F}'. // Bidirectional text overrides + '\x{205F}-\x{206F}'. // Various text hinting characters + '\x{FEFF}'. // Byte order mark + '\x{FF01}-\x{FF60}'. // Full-width latin + '\x{FFF9}-\x{FFFD}'. // Replacement characters + '\x{0}]/u', // NULL byte + $name)) { + return t('The username contains an illegal character.'); + } if (ereg('@', $name) && !eregi('@([0-9a-z](-?[0-9a-z])*.)+[a-z]{2}([zmuvtg]|fo|me)?$', $name)) return t('The username is not a valid authentication ID.'); if (strlen($name) > 56) return t('The username %name is too long: it must be less than 56 characters.', array('%name' => theme('placeholder', $name))); } @@ -523,7 +535,7 @@ function user_block($op = 'list', $delta = 0, $edit = array()) { case 1: if ($menu = theme('menu_tree')) { - $block['subject'] = $user->uid ? $user->name : t('Navigation'); + $block['subject'] = $user->uid ? check_plain($user->name) : t('Navigation'); $block['content'] = '
    '. $menu .'
    '; } return $block; @@ -1191,7 +1203,7 @@ function user_edit($category = 'account') { } $output = form($output, 'post', 0, array('enctype' => 'multipart/form-data')); - drupal_set_title($account->name); + drupal_set_title(check_plain($account->name)); print theme('page', $output); } @@ -1209,7 +1221,7 @@ function user_view($uid = 0) { } } - drupal_set_title($account->name); + drupal_set_title(check_plain($account->name)); print theme('page', theme('user_profile', $account, $fields)); } else { diff --git a/update.php b/update.php index aae141d4c79b273eaa03134b8a2e0a0c68b1e5be..f332dbda50ef2765f0eb5f2ea6aafcebcd364f3a 100644 --- a/update.php +++ b/update.php @@ -209,6 +209,9 @@ function update_info() { include_once "includes/bootstrap.inc"; include_once "includes/common.inc"; + // Protect against cross site request forgeries + drupal_check_token(); + // Access check: if (($access_check == 0) || ($user->uid == 1)) { update_page();