Merge pull request pressflow#15 from dawehner/drupal-636

Update to 6.36

CHANGELOG.txt

@@ -1,18 +1,10 @@
-Drupal 6.30, 2014-01-15
-----------------------
-- Fixed security issues (multiple vulnerabilities), see SA-CORE-2014-001.
-
-Drupal 6.29, 2013-11-20
-----------------------
-- Fixed security issues (multiple vulnerabilities), see SA-CORE-2013-003.
-
-Drupal 6.31, 2014-04-16
-----------------------
-- Fixed security issues (information disclosure). See SA-CORE-2014-002.
+Drupal 6.36, 2015-06-17
+-----------------------
+- Fixed security issues (OpenID impersonation). See SA-CORE-2015-002.
 
-Drupal 6.30, 2014-01-15
+Drupal 6.35, 2015-03-18
 ----------------------
-- Fixed security issues (multiple vulnerabilities), see SA-CORE-2014-001.
+- Fixed security issues (multiple vulnerabilities). See SA-CORE-2015-001.
 
 Drupal 6.34, 2014-11-19
 ----------------------

includes/bootstrap.inc

@@ -1858,7 +1858,35 @@ function _drupal_bootstrap($phase) {
     case DRUPAL_BOOTSTRAP_LATE_PAGE_CACHE:
       // Initialize configuration variables, using values from settings.php if available.
       $conf = variable_init(isset($conf) ? $conf : array());
-
+
+      // Sanitize the destination parameter (which is often used for redirects)
+      // to prevent open redirect attacks leading to other domains. Sanitize
+      // both $_GET['destination'] and $_REQUEST['destination'] to protect code
+      // that relies on either, but do not sanitize $_POST to avoid interfering
+      // with unrelated form submissions. $_REQUEST['edit']['destination'] is
+      // also sanitized since drupal_goto() will sometimes rely on it, and
+      // other code might therefore use it too. The sanitization happens here
+      // 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']);
+          unset($_REQUEST['destination']);
+        }
+        // If there's still something in $_REQUEST['destination'] that didn't
+        // come from $_GET, check it too.
+        if (isset($_REQUEST['destination']) && (!isset($_GET['destination']) || $_REQUEST['destination'] != $_GET['destination']) && menu_path_is_external($_REQUEST['destination'])) {
+          unset($_REQUEST['destination']);
+        }
+        // Check $_REQUEST['edit']['destination'] separately.
+        if (isset($_REQUEST['edit']['destination']) && menu_path_is_external($_REQUEST['edit']['destination'])) {
+          unset($_REQUEST['edit']['destination']);
+        }
+      }
+
       $cache_mode = variable_get('cache', CACHE_DISABLED);
       // Get the page from the cache, unless the cache is disabled or external.
       if ($cache_mode != CACHE_DISABLED && $cache_mode != CACHE_EXTERNAL) {
@@ -2380,3 +2408,49 @@ function drupal_random_bytes($count) {
   return $output;
 }
 
+/**
+ * Calculates a hexadecimal encoded sha-1 hmac.
+ *
+ * @param string $data
+ *   String to be validated with the hmac.
+ * @param string $key
+ *   A secret string key.
+ *
+ * See RFC2104 (http://www.ietf.org/rfc/rfc2104.txt). Note, the result of this
+ * must be identical to using hash_hmac('sha1', $data, $key);  We don't use
+ * that function since PHP can be missing it if it was compiled with the
+ * --disable-hash switch.
+ *
+ * @return string
+ *   A hexadecimal encoded sha-1 hmac.
+ */
+function drupal_hash_hmac_sha1($data, $key) {
+  // Keys longer than the 64 byte block size must be hashed first.
+  if (strlen($key) > 64) {
+    $key = pack("H*", sha1($key));
+  }
+  return sha1((str_pad($key, 64, chr(0x00)) ^ (str_repeat(chr(0x5c), 64))) . pack("H*", sha1((str_pad($key, 64, chr(0x00)) ^ (str_repeat(chr(0x36), 64))) . $data)));
+}
+
+/**
+ * Calculates a base-64 encoded, URL-safe sha-1 hmac.
+ *
+ * @param string $data
+ *   String to be validated with the hmac.
+ * @param string $key
+ *   A secret string key.
+ *
+ * @return string
+ *   A base-64 encoded sha-1 hmac, with + replaced with -, / with _ and
+ *   any = padding characters removed.
+ */
+function drupal_hmac_base64($data, $key) {
+  // Casting $data and $key to strings here is necessary to avoid empty string
+  // results of the hash function if they are not scalar values. As this
+  // function is used in security-critical contexts like token validation it is
+  // important that it never returns an empty string.
+  $hmac = base64_encode(pack("H*", drupal_hash_hmac_sha1((string) $data, (string) $key)));
+  // Modify the hmac so it's safe to use in URLs.
+  return strtr($hmac, array('+' => '-', '/' => '_', '=' => ''));
+}
+

includes/common.inc

@@ -328,7 +328,7 @@ 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 = ($colonpos !== FALSE && !preg_match('![/?#]!', substr($destination, 0, $colonpos)));
+    $absolute = strpos($destination, '//') === 0 || ($colonpos !== FALSE && !preg_match('![/?#]!', substr($destination, 0, $colonpos)));
     if (!$absolute) {
       extract(parse_url(urldecode($destination)));
     }
@@ -380,7 +380,10 @@ function drupal_not_found() {
 
   // Keep old path for reference, and to allow forms to redirect to it.
   if (!isset($_REQUEST['destination'])) {
-    $_REQUEST['destination'] = $_GET['q'];
+    // Make sure that the current path is not interpreted as external URL.
+    if (!menu_path_is_external($_GET['q'])) {
+      $_REQUEST['destination'] = $_GET['q'];
+    }
   }
 
   $path = drupal_get_normal_path(variable_get('site_404', ''));
@@ -410,7 +413,10 @@ function drupal_access_denied() {
 
   // Keep old path for reference, and to allow forms to redirect to it.
   if (!isset($_REQUEST['destination'])) {
-    $_REQUEST['destination'] = $_GET['q'];
+    // Make sure that the current path is not interpreted as external URL.
+    if (!menu_path_is_external($_GET['q'])) {
+      $_REQUEST['destination'] = $_GET['q'];
+    }
   }
 
   $path = drupal_get_normal_path(variable_get('site_403', ''));
@@ -1468,12 +1474,20 @@ 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.
-    // Only call the slow filter_xss_bad_protocol if $path contains a ':' before
-    // any / ? or #.
+    // 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'] = ($colonpos !== FALSE && !preg_match('![/?#]!', substr($path, 0, $colonpos)) && filter_xss_bad_protocol($path, FALSE) == check_plain($path));
+    $options['external'] = (strpos($path, '//') === 0)
+      || ($colonpos !== FALSE
+        && !preg_match('![/?#]!', substr($path, 0, $colonpos))
+        && filter_xss_bad_protocol($path, FALSE) == check_plain($path));
   }
 
   // May need language dependent rewriting if language.inc is present.
@@ -1503,6 +1517,11 @@ function url($path = NULL, $options = array()) {
     return $path . $options['fragment'];
   }
 
+  // Strip leading slashes from internal paths to prevent them becoming external
+  // URLs without protocol. /example.com should not be turned into
+  // //example.com.
+  $path = ltrim($path, '/');
+
   global $base_url;
   static $script;
 

includes/menu.inc

@@ -2486,8 +2486,16 @@ 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 $colonpos !== FALSE && !preg_match('![/?#]!', substr($path, 0, $colonpos)) && filter_xss_bad_protocol($path, FALSE) == check_plain($path);
+  return (strpos($path, '//') === 0)
+    || ($colonpos !== FALSE
+      && !preg_match('![/?#]!', substr($path, 0, $colonpos))
+      && filter_xss_bad_protocol($path, FALSE) == check_plain($path));
 }
 
 /**

modules/openid/openid.module

@@ -243,10 +243,34 @@ function openid_complete($response = array()) {
         if (openid_verify_assertion($service, $response)) {
           // If the returned claimed_id is different from the session claimed_id,
           // then we need to do discovery and make sure the op_endpoint matches.
-          if ($service['version'] == 2 && $response['openid.claimed_id'] != $claimed_id) {
-            $disco = openid_discovery($response['openid.claimed_id']);
-            if ($disco[0]['uri'] != $service['uri']) {
-              return $response;
+          if ($service['version'] == 2) {
+            // Returned Claimed Identifier could contain unique fragment
+            // identifier to allow identifier recycling so we need to preserve
+            // it in the response.
+            $response_claimed_id = _openid_normalize($response['openid.claimed_id']);
+
+            if ($response_claimed_id != $claimed_id || $response_claimed_id != $response['openid.identity']) {
+              $disco = openid_discovery($response['openid.claimed_id']);
+
+              if ($disco[0]['uri'] != $service['uri']) {
+                return $response;
+              }
+
+              if (!empty($disco[0]['localid'])) {
+                $identity = $disco[0]['localid'];
+              }
+              else if (!empty($disco[0]['delegate'])) {
+                $identity = $disco[0]['delegate'];
+              }
+              else {
+                $identity = FALSE;
+              }
+
+              // The OP-Local Identifier (if different than the Claimed
+              // Identifier) must be present in the XRDS document.
+              if ($response_claimed_id != $response['openid.identity'] && (!$identity || $identity != $response['openid.identity'])) {
+                return $response;
+              }
             }
           }
           else {

modules/system/system.module

@@ -8,7 +8,7 @@
 /**
  * The current system version.
  */
-define('VERSION', '6.34');
+define('VERSION', '6.36');
 
 /**
  * Core API compatibility.

modules/user/user.module

@@ -1592,11 +1592,33 @@ function user_external_login_register($name, $module) {
  */
 function user_pass_reset_url($account) {
   $timestamp = time();
-  return url("user/reset/$account->uid/$timestamp/". user_pass_rehash($account->pass, $timestamp, $account->login), array('absolute' => TRUE));
+  return url("user/reset/$account->uid/$timestamp/". user_pass_rehash($account->pass, $timestamp, $account->login, $account->uid), array('absolute' => TRUE));
 }
 
-function user_pass_rehash($password, $timestamp, $login) {
-  return md5($timestamp . $password . $login);
+function user_pass_rehash($password, $timestamp, $login, $uid) {
+  // Backwards compatibility: Try to determine a $uid if one was not passed.
+  // (Since $uid is a required parameter to this function, a PHP warning will
+  // be generated if it's not provided, which is an indication that the calling
+  // code should be updated. But the code below will try to generate a correct
+  // hash in the meantime.)
+  if (!isset($uid)) {
+    $uids = array();
+    $result = db_query_range("SELECT uid FROM {users} WHERE pass = '%s' AND login = '%s' AND uid > 0", $password, $login, 0, 2);
+    while ($row = db_fetch_array($result)) {
+      $uids[] = $row['uid'];
+    }
+    // If exactly one user account matches the provided password and login
+    // timestamp, proceed with that $uid.
+    if (count($uids) == 1) {
+      $uid = reset($uids);
+    }
+    // Otherwise there is no safe hash to return, so return a random string
+    // that will never be treated as a valid token.
+    else {
+      return drupal_random_key();
+    }
+  }
+  return drupal_hmac_base64($timestamp . $login . $uid, drupal_get_private_key() . $password);
 }
 
 function user_edit_form(&$form_state, $uid, $edit, $register = FALSE) {

modules/user/user.pages.inc

@@ -106,7 +106,7 @@ function user_pass_reset(&$form_state, $uid, $timestamp, $hashed_pass, $action =
         drupal_set_message(t('You have tried to use a one-time login link that has expired. Please request a new one using the form below.'));
         drupal_goto('user/password');
       }
-      else if ($account->uid && $timestamp > $account->login && $timestamp < $current && $hashed_pass == user_pass_rehash($account->pass, $timestamp, $account->login)) {
+      else if ($account->uid && $timestamp > $account->login && $timestamp < $current && $hashed_pass == user_pass_rehash($account->pass, $timestamp, $account->login, $account->uid)) {
         // First stage is a confirmation form, then login
         if ($action == 'login') {
           watchdog('user', 'User %name used one-time login link at time %timestamp.', array('%name' => $account->name, '%timestamp' => $timestamp));