summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorDavid Rothstein2015-03-18 19:16:59 (GMT)
committer David Rothstein2015-03-18 19:16:59 (GMT)
commit89f56c3bb4a30ef3551ebcf32449c44c40cc1bab (patch)
tree6395322e30343b354f745d11b9b2a987af7f0274
parente1634ecb4b29e2ddabe236fe588d8e1c1071cedd (diff)
parent8ffc5db3c0ab926f3d4b2cf8bc51714c8c0f3c93 (diff)
Merge tag '6.35' into 6.x6.x
6.35 release Conflicts: CHANGELOG.txt modules/system/system.module
-rw-r--r--CHANGELOG.txt6
-rw-r--r--includes/bootstrap.inc75
-rw-r--r--includes/common.inc33
-rw-r--r--includes/menu.inc10
-rw-r--r--modules/system/system.module2
-rw-r--r--modules/user/user.module28
-rw-r--r--modules/user/user.pages.inc2
7 files changed, 142 insertions, 14 deletions
diff --git a/CHANGELOG.txt b/CHANGELOG.txt
index d1dc845..f5a0610 100644
--- a/CHANGELOG.txt
+++ b/CHANGELOG.txt
@@ -1,7 +1,11 @@
-Drupal 6.35-dev, xxxx-xx-xx (development release)
+Drupal 6.36-dev, xxxx-xx-xx (development release)
----------------------
+Drupal 6.35, 2015-03-18
+----------------------
+- Fixed security issues (multiple vulnerabilities). See SA-CORE-2015-001.
+
Drupal 6.34, 2014-11-19
----------------------
- Fixed security issues (session hijacking). See SA-CORE-2014-006.
diff --git a/includes/bootstrap.inc b/includes/bootstrap.inc
index 53fc41e..aea944d 100644
--- a/includes/bootstrap.inc
+++ b/includes/bootstrap.inc
@@ -1185,6 +1185,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.
$cache = $cache_mode == CACHE_DISABLED ? '' : page_get_cache();
@@ -1459,3 +1488,49 @@ function drupal_random_bytes($count) {
$bytes = substr($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('+' => '-', '/' => '_', '=' => ''));
+}
diff --git a/includes/common.inc b/includes/common.inc
index d19429a..19798ba 100644
--- a/includes/common.inc
+++ b/includes/common.inc
@@ -331,7 +331,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', ''));
@@ -1474,12 +1480,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.
@@ -1509,6 +1523,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;
diff --git a/includes/menu.inc b/includes/menu.inc
index 9a686e3..0d1ec25 100644
--- a/includes/menu.inc
+++ b/includes/menu.inc
@@ -2473,8 +2473,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));
}
/**
diff --git a/modules/system/system.module b/modules/system/system.module
index 49df5a0..0a4ac39 100644
--- a/modules/system/system.module
+++ b/modules/system/system.module
@@ -8,7 +8,7 @@
/**
* The current system version.
*/
-define('VERSION', '6.35-dev');
+define('VERSION', '6.36-dev');
/**
* Core API compatibility.
diff --git a/modules/user/user.module b/modules/user/user.module
index 0dea21e..00046b6 100644
--- a/modules/user/user.module
+++ b/modules/user/user.module
@@ -1484,11 +1484,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) {
diff --git a/modules/user/user.pages.inc b/modules/user/user.pages.inc
index 3a52a01..917aa0a 100644
--- a/modules/user/user.pages.inc
+++ b/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));