diff --git a/CHANGELOG.txt b/CHANGELOG.txt index a69cadbd7af6f5682735bb9415551f74a5ef72a9..f0c0aabcaeb1c09626a060541f03c9432ed2f7d5 100644 --- a/CHANGELOG.txt +++ b/CHANGELOG.txt @@ -1,6 +1,8 @@ -Drupal 6.38-dev, xxxx-xx-xx (development release) -------------------------------------------------- +Drupal 6.38, 2016-02-24 - Final release +--------------------------------------- +- Fixed security issues (multiple vulnerabilities). See SA-CORE-2016-001. +- Previously unreleased documentation fixes. Drupal 6.37, 2015-08-19 ----------------------- diff --git a/includes/bootstrap.inc b/includes/bootstrap.inc index aea944dd039ab4537c465cc2084cce573fa984bd..23179ca98d35899bd767f68a2639b89f00d0f07f 100644 --- a/includes/bootstrap.inc +++ b/includes/bootstrap.inc @@ -1196,8 +1196,6 @@ function _drupal_bootstrap($phase) { // because menu_path_is_external() requires the variable system to be // available. if (isset($_GET['destination']) || isset($_REQUEST['destination']) || isset($_REQUEST['edit']['destination'])) { - require_once './includes/menu.inc'; - drupal_load('module', 'filter'); // If the destination is an external URL, remove it. if (isset($_GET['destination']) && menu_path_is_external($_GET['destination'])) { unset($_GET['destination']); @@ -1534,3 +1532,74 @@ function drupal_hmac_base64($data, $key) { // Modify the hmac so it's safe to use in URLs. return strtr($hmac, array('+' => '-', '/' => '_', '=' => '')); } + +/** + * Returns TRUE if a path is external (e.g. http://example.com). + * + * May be used early in bootstrap. + */ +function menu_path_is_external($path) { + // Avoid calling filter_xss_bad_protocol() if there is any slash (/), + // hash (#) or question_mark (?) before the colon (:) occurrence - if any - as + // this would clearly mean it is not a URL. If the path starts with 2 slashes + // then it is always considered an external URL without an explicit protocol + // part. Leading control characters may be ignored or mishandled by browsers, + // so assume such a path may lead to an external location. The range matches + // all UTF-8 control characters, class Cc. + $colonpos = strpos($path, ':'); + // Some browsers treat \ as / so normalize to forward slashes. + $path = str_replace('\\', '/', $path); + return (strpos($path, '//') === 0) || (preg_match('/^[\x00-\x1F\x7F-\x9F]/u', $path) !== 0) + || ($colonpos !== FALSE + && !preg_match('![/?#]!', substr($path, 0, $colonpos)) + && filter_xss_bad_protocol($path, FALSE) == check_plain($path)); +} + +/** + * Processes an HTML attribute value and ensures it does not contain an URL + * with a disallowed protocol (e.g. javascript:) + * + * May be used early in bootstrap. + * + * @param $string + * The string with the attribute value. + * @param $decode + * Whether to decode entities in the $string. Set to FALSE if the $string + * is in plain text, TRUE otherwise. Defaults to TRUE. + * @return + * Cleaned up and HTML-escaped version of $string. + */ +function filter_xss_bad_protocol($string, $decode = TRUE) { + static $allowed_protocols; + if (!isset($allowed_protocols)) { + $allowed_protocols = array_flip(variable_get('filter_allowed_protocols', array('http', 'https', 'ftp', 'news', 'nntp', 'tel', 'telnet', 'mailto', 'irc', 'ssh', 'sftp', 'webcal', 'rtsp'))); + } + + // Get the plain text representation of the attribute value (i.e. its meaning). + if ($decode) { + $string = decode_entities($string); + } + + // 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; + } + // Per RFC2616, section 3.2.3 (URI Comparison) scheme comparison must be case-insensitive + // Check if this is a disallowed protocol. + if (!isset($allowed_protocols[strtolower($protocol)])) { + $string = substr($string, $colonpos + 1); + } + } + } while ($before != $string); + return check_plain($string); +} diff --git a/includes/common.inc b/includes/common.inc index 19798bae1c4eaa5459b5ef0007a482ca0f62aa07..9a28c06cc092059df901c7984dc991bcab606389 100644 --- a/includes/common.inc +++ b/includes/common.inc @@ -142,6 +142,10 @@ function drupal_clear_path_cache() { * * Note: When sending a Content-Type header, always include a 'charset' type, * too. This is necessary to avoid security bugs (e.g. UTF-7 XSS). + * + * Note: No special sanitizing needs to be done to headers. However if a header + * value contains a line break a PHP warning will be thrown and the header + * will not be set. */ function drupal_set_header($header = NULL) { // We use an array to guarantee there are no leading or trailing delimiters. @@ -150,8 +154,15 @@ function drupal_set_header($header = NULL) { static $stored_headers = array(); if (strlen($header)) { - header($header); - $stored_headers[] = $header; + // Protect against header injection attacks if PHP is too old to do that. + if (version_compare(PHP_VERSION, '5.1.2', '<') && (strpos($header, "\n") !== FALSE || strpos($header, "\r") !== FALSE)) { + // Use the same warning message that newer versions of PHP use. + trigger_error('Header may not contain more than a single header, new line detected', E_USER_WARNING); + } + else { + header($header); + $stored_headers[] = $header; + } } return implode("\n", $stored_headers); } @@ -330,14 +341,20 @@ function drupal_goto($path = '', $query = NULL, $fragment = NULL, $http_response if ($destination) { // Do not redirect to an absolute URL originating from user input. - $colonpos = strpos($destination, ':'); - $absolute = strpos($destination, '//') === 0 || ($colonpos !== FALSE && !preg_match('![/?#]!', substr($destination, 0, $colonpos))); - if (!$absolute) { - extract(parse_url(urldecode($destination))); + if (!menu_path_is_external($destination)) { + extract(parse_url($destination)); } } - $url = url($path, array('query' => $query, 'fragment' => $fragment, 'absolute' => TRUE)); + $options = array('query' => $query, 'fragment' => $fragment, 'absolute' => TRUE); + // In some cases modules call drupal_goto($_GET['q']). We need to ensure that + // such a redirect is not to an external URL. + if ($path === $_GET['q'] && menu_path_is_external($path)) { + // Force url() to generate a non-external URL. + $options['external'] = FALSE; + } + + $url = url($path, $options); // Remove newlines from the URL to avoid header injection attacks. $url = str_replace(array("\n", "\r"), '', $url); @@ -672,7 +689,7 @@ function drupal_error_handler($errno, $message, $filename, $line, $context) { return; } - if ($errno & (E_ALL ^ E_DEPRECATED)) { + if ($errno & (E_ALL ^ E_DEPRECATED ^ E_NOTICE)) { $types = array(1 => 'error', 2 => 'warning', 4 => 'parse error', 8 => 'notice', 16 => 'core error', 32 => 'core warning', 64 => 'compile error', 128 => 'compile warning', 256 => 'user error', 512 => 'user warning', 1024 => 'user notice', 2048 => 'strict warning', 4096 => 'recoverable fatal error'); // For database errors, we want the line number/file name of the place that @@ -1480,20 +1497,9 @@ function url($path = NULL, $options = array()) { 'alias' => FALSE, 'prefix' => '' ); - // A duplicate of the code from menu_path_is_external() to avoid needing - // another function call, since performance inside url() is critical. + if (!isset($options['external'])) { - // Return an external link if $path contains an allowed absolute URL. Avoid - // calling filter_xss_bad_protocol() if there is any slash (/), hash (#) or - // question_mark (?) before the colon (:) occurrence - if any - as this - // would clearly mean it is not a URL. If the path starts with 2 slashes - // then it is always considered an external URL without an explicit protocol - // part. - $colonpos = strpos($path, ':'); - $options['external'] = (strpos($path, '//') === 0) - || ($colonpos !== FALSE - && !preg_match('![/?#]!', substr($path, 0, $colonpos)) - && filter_xss_bad_protocol($path, FALSE) == check_plain($path)); + $options['external'] = menu_path_is_external($path); } // May need language dependent rewriting if language.inc is present. diff --git a/includes/form.inc b/includes/form.inc index b55d724a8d7f9a9e9ab18f6743cab97b3da1780a..1f485ef1bcc98b545216d709407c210c5b200c2e 100644 --- a/includes/form.inc +++ b/includes/form.inc @@ -1066,9 +1066,16 @@ function _form_builder_handle_input_element($form_id, &$form, &$form_state, $com $form['#attributes']['disabled'] = 'disabled'; } + // With JavaScript or other easy hacking, input can be submitted even for + // elements with #access=FALSE. For security, these must not be processed. + // For pages with multiple forms, ensure that input is only processed for the + // submitted form. drupal_execute() may bypass these checks and be treated as + // a high privilege user submitting a single form. + $process_input = $form['#programmed'] || ((!isset($form['#access']) || $form['#access']) && isset($form['#post']) && (isset($form['#post']['form_id']) && $form['#post']['form_id'] == $form_id)); + if (!isset($form['#value']) && !array_key_exists('#value', $form)) { $function = !empty($form['#value_callback']) ? $form['#value_callback'] : 'form_type_'. $form['#type'] .'_value'; - if (($form['#programmed']) || ((!isset($form['#access']) || $form['#access']) && isset($form['#post']) && (isset($form['#post']['form_id']) && $form['#post']['form_id'] == $form_id))) { + if ($process_input) { $edit = $form['#post']; foreach ($form['#parents'] as $parent) { $edit = isset($edit[$parent]) ? $edit[$parent] : NULL; @@ -1113,7 +1120,7 @@ function _form_builder_handle_input_element($form_id, &$form, &$form_state, $com // We compare the incoming values with the buttons defined in the form, // and flag the one that matches. We have to do some funky tricks to // deal with Internet Explorer's handling of single-button forms, though. - if (!empty($form['#post']) && isset($form['#executes_submit_callback'])) { + if ($process_input && !empty($form['#post']) && isset($form['#executes_submit_callback'])) { // First, accumulate a collection of buttons, divided into two bins: // those that execute full submit callbacks and those that only validate. $button_type = $form['#executes_submit_callback'] ? 'submit' : 'button'; diff --git a/includes/menu.inc b/includes/menu.inc index ef27d91e7791f958d6fd56416703ed5a004c9469..4788855415d14f3f0443722d7ed3b81bc8b25bbd 100644 --- a/includes/menu.inc +++ b/includes/menu.inc @@ -2469,22 +2469,6 @@ function _menu_router_build($callbacks) { return $menu; } -/** - * Returns TRUE if a path is external (e.g. http://example.com). - */ -function menu_path_is_external($path) { - // Avoid calling filter_xss_bad_protocol() if there is any slash (/), - // hash (#) or question_mark (?) before the colon (:) occurrence - if any - as - // this would clearly mean it is not a URL. If the path starts with 2 slashes - // then it is always considered an external URL without an explicit protocol - // part. - $colonpos = strpos($path, ':'); - return (strpos($path, '//') === 0) - || ($colonpos !== FALSE - && !preg_match('![/?#]!', substr($path, 0, $colonpos)) - && filter_xss_bad_protocol($path, FALSE) == check_plain($path)); -} - /** * Checks whether the site is off-line for maintenance. * diff --git a/includes/xmlrpcs.inc b/includes/xmlrpcs.inc index 049a92b86738c13d9adbfb048ae9d66b23d42abe..18e5ac70e661ca185ff94133b488ec11d5b22115 100644 --- a/includes/xmlrpcs.inc +++ b/includes/xmlrpcs.inc @@ -213,6 +213,10 @@ function xmlrpc_server_call($xmlrpc_server, $methodname, $args) { function xmlrpc_server_multicall($methodcalls) { // See http://www.xmlrpc.com/discuss/msgReader$1208 + // To avoid multicall expansion attacks, limit the number of duplicate method + // calls allowed with a default of 1. Set to -1 for unlimited. + $duplicate_method_limit = variable_get('xmlrpc_multicall_duplicate_method_limit', 1); + $method_count = array(); $return = array(); $xmlrpc_server = xmlrpc_server_get(); foreach ($methodcalls as $call) { @@ -222,10 +226,14 @@ function xmlrpc_server_multicall($methodcalls) { $ok = FALSE; } $method = $call['methodName']; + $method_count[$method] = isset($method_count[$method]) ? $method_count[$method] + 1 : 1; $params = $call['params']; if ($method == 'system.multicall') { $result = xmlrpc_error(-32600, t('Recursive calls to system.multicall are forbidden.')); } + elseif ($duplicate_method_limit > 0 && $method_count[$method] > $duplicate_method_limit) { + $result = xmlrpc_error(-156579, t('Too many duplicate method calls in system.multicall.')); + } elseif ($ok) { $result = xmlrpc_server_call($xmlrpc_server, $method, $params); } diff --git a/modules/filter/filter.module b/modules/filter/filter.module index 6ea05372463bd6283a460b0658e1051a168cdc3c..e603b254f39f4ace10b50402d6fea652f95e3c21 100644 --- a/modules/filter/filter.module +++ b/modules/filter/filter.module @@ -1203,53 +1203,6 @@ function _filter_xss_attributes($attr) { return $attrarr; } -/** - * Processes an HTML attribute value and ensures it does not contain an URL - * with a disallowed protocol (e.g. javascript:) - * - * @param $string - * The string with the attribute value. - * @param $decode - * Whether to decode entities in the $string. Set to FALSE if the $string - * is in plain text, TRUE otherwise. Defaults to TRUE. - * @return - * Cleaned up and HTML-escaped version of $string. - */ -function filter_xss_bad_protocol($string, $decode = TRUE) { - static $allowed_protocols; - if (!isset($allowed_protocols)) { - $allowed_protocols = array_flip(variable_get('filter_allowed_protocols', array('http', 'https', 'ftp', 'news', 'nntp', 'tel', 'telnet', 'mailto', 'irc', 'ssh', 'sftp', 'webcal', 'rtsp'))); - } - - // Get the plain text representation of the attribute value (i.e. its meaning). - if ($decode) { - $string = decode_entities($string); - } - - // 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; - } - // Per RFC2616, section 3.2.3 (URI Comparison) scheme comparison must be case-insensitive - // Check if this is a disallowed protocol. - if (!isset($allowed_protocols[strtolower($protocol)])) { - $string = substr($string, $colonpos + 1); - } - } - } while ($before != $string); - return check_plain($string); -} - /** * @} End of "Standard filters". */ diff --git a/modules/system/system.admin.inc b/modules/system/system.admin.inc index 42fd311bfd572ddf2dce4e8a61eb96767d7f1c09..315390cf21b23dc6d15c3866c1034b5ec87fc98c 100644 --- a/modules/system/system.admin.inc +++ b/modules/system/system.admin.inc @@ -1487,7 +1487,8 @@ function system_rss_feeds_settings() { */ function system_date_time_settings() { drupal_add_js(drupal_get_path('module', 'system') .'/system.js', 'module'); - drupal_add_js(array('dateTime' => array('lookup' => url('admin/settings/date-time/lookup'))), 'setting'); + $ajax_path = 'admin/settings/date-time/lookup'; + drupal_add_js(array('dateTime' => array('lookup' => url($ajax_path, array('query' => array('token' => drupal_get_token($ajax_path)))))), 'setting'); // Date settings: $zones = _system_zonelist(); @@ -1646,6 +1647,11 @@ function system_date_time_settings_submit($form, &$form_state) { * Return the date for a given format string via Ajax. */ function system_date_time_lookup() { + // This callback is protected with a CSRF token because user input from the + // query string is reflected in the output. + if (!isset($_GET['token']) || !drupal_valid_token($_GET['token'], 'admin/settings/date-time/lookup')) { + return MENU_ACCESS_DENIED; + } $result = format_date(time(), 'custom', $_GET['format']); drupal_json($result); } diff --git a/modules/system/system.install b/modules/system/system.install index 33f4c4d3a63c8b6f0e2e7e4e8c6ef25c5a79c1e3..9a4be939603917e54793fd24531573a41ecdbada 100644 --- a/modules/system/system.install +++ b/modules/system/system.install @@ -1061,7 +1061,7 @@ function system_schema() { 'default' => 0), 'session' => array( 'description' => 'The serialized contents of $_SESSION, an array of name/value pairs that persists across page requests by this session ID. Drupal loads $_SESSION from here at the start of each request and saves it at the end.', - 'type' => 'text', + 'type' => 'blob', 'not null' => FALSE, 'size' => 'big') ), @@ -2736,6 +2736,15 @@ function system_update_6055() { return $ret; } +/** + * Convert {session} data storage to blob. + */ +function system_update_6056() { + $ret = array(); + db_change_field($ret, 'sessions', 'session', 'session', array('type' => 'blob', 'not null' => FALSE, 'size' => 'big')); + return $ret; +} + /** * @} End of "defgroup updates-6.x-extra". * The next series of updates should start at 7000. diff --git a/modules/system/system.js b/modules/system/system.js index 48fd016a5f9f878f9c8f86eecfd31cf4e232cea2..24f998ca7d8aa0be7982b0f4078626822ccc13bc 100644 --- a/modules/system/system.js +++ b/modules/system/system.js @@ -101,7 +101,7 @@ Drupal.behaviors.dateTime = function(context) { // Attach keyup handler to custom format inputs. $('input.custom-format:not(.date-time-processed)', context).addClass('date-time-processed').keyup(function() { var input = $(this); - var url = Drupal.settings.dateTime.lookup +(Drupal.settings.dateTime.lookup.match(/\?q=/) ? "&format=" : "?format=") + encodeURIComponent(input.val()); + var url = Drupal.settings.dateTime.lookup +(Drupal.settings.dateTime.lookup.match(/\?/) ? "&format=" : "?format=") + encodeURIComponent(input.val()); $.getJSON(url, function(data) { $("div.description span", input.parent()).html(data); }); diff --git a/modules/system/system.module b/modules/system/system.module index ccc638fc7afb4e965af0e20f999e85ded8c8ca41..37ac1f47831988547737df7927b44f9380fe8cf4 100644 --- a/modules/system/system.module +++ b/modules/system/system.module @@ -8,7 +8,7 @@ /** * The current system version. */ -define('VERSION', '6.38-dev'); +define('VERSION', '6.38'); /** * Core API compatibility. diff --git a/modules/user/user.module b/modules/user/user.module index 00046b6cf7a0d09f6c70ee3a5837e1e61a4b5de2..8b5a37789f5bd421257e6af1d3cd15ef578e9ac5 100644 --- a/modules/user/user.module +++ b/modules/user/user.module @@ -670,8 +670,15 @@ function user_user($type, &$edit, &$account, $category = NULL) { return _user_edit_validate((isset($account->uid) ? $account->uid : FALSE), $edit); } - if ($type == 'submit' && $category == 'account') { - return _user_edit_submit((isset($account->uid) ? $account->uid : FALSE), $edit); + if ($type == 'submit') { + if ($category == 'account') { + return _user_edit_submit((isset($account->uid) ? $account->uid : FALSE), $edit); + } + elseif (isset($edit['roles'])) { + // Filter out roles with empty values to avoid granting extra roles when + // processing custom form submissions. + $edit['roles'] = array_filter($edit['roles']); + } } if ($type == 'categories') { @@ -681,7 +688,7 @@ function user_user($type, &$edit, &$account, $category = NULL) { function user_login_block() { $form = array( - '#action' => url($_GET['q'], array('query' => drupal_get_destination())), + '#action' => url($_GET['q'], array('query' => drupal_get_destination(), 'external' => FALSE)), '#id' => 'user-login-form', '#validate' => user_login_default_validators(), '#submit' => array('user_login_submit'),