Drupal 6.38 (SA-CORE-2016-001) by agerard, Alan Evans, benjy, catch, chx, dalin, Damien Tournoud, DamienMcKenna, Dave Cohen, Dave Reid, David Jardin, David_Rothstein, dmitrig01, dsnopek, effulgentsia, fgm, greggles, Gábor Hojtsy, Harry Taheem, Heine, John Morahan, Juho Nurminen 2NS, klausi, larowlan, nagba, Pere Orga, plach, pwolanin, quicksketch, rickmanelius, scor, sun, Tarpinder Grewal, YesCT6.386.x

12 files changed, 148 insertions, 97 deletions

diff --git a/CHANGELOG.txt b/CHANGELOG.txt
index a69cadbd7af..f0c0aabcaeb 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 aea944dd039..23179ca98d3 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 19798bae1c4..9a28c06cc09 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 b55d724a8d7..1f485ef1bcc 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 ef27d91e779..4788855415d 100644
--- a/includes/menu.inc
+++ b/includes/menu.inc
@@ -2470,22 +2470,6 @@ function _menu_router_build($callbacks) {
 }
 
 /**
- * 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.
  *
  * This function will log the current user out and redirect to front page
diff --git a/includes/xmlrpcs.inc b/includes/xmlrpcs.inc
index 049a92b8673..18e5ac70e66 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 6ea05372463..e603b254f39 100644
--- a/modules/filter/filter.module
+++ b/modules/filter/filter.module
@@ -1204,52 +1204,5 @@ function _filter_xss_attributes($attr) {
 }
 
 /**
- * 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 42fd311bfd5..315390cf21b 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 33f4c4d3a63..9a4be939603 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')
       ),
@@ -2737,6 +2737,15 @@ function system_update_6055() {
 }
 
 /**
+ * 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 48fd016a5f9..24f998ca7d8 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 ccc638fc7af..37ac1f47831 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 00046b6cf7a..8b5a37789f5 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'),