Commit b44056d2 authored by David_Rothstein's avatar David_Rothstein

Drupal 7.35

parent 81586d9e
Drupal 7.35, 2015-03-18
----------------------
- Fixed security issues (multiple vulnerabilities). See SA-CORE-2015-001.
Drupal 7.34, 2014-11-19
----------------------
- Fixed security issues (multiple vulnerabilities). See SA-CORE-2014-006.
......
......@@ -8,7 +8,7 @@
/**
* The current system version.
*/
define('VERSION', '7.34');
define('VERSION', '7.35');
/**
* Core API compatibility.
......@@ -2497,6 +2497,26 @@ function _drupal_bootstrap_variables() {
// Load bootstrap modules.
require_once DRUPAL_ROOT . '/includes/module.inc';
module_load_all(TRUE);
// 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. The sanitization happens here because
// url_is_external() requires the variable system to be available.
if (isset($_GET['destination']) || isset($_REQUEST['destination'])) {
require_once DRUPAL_ROOT . '/includes/common.inc';
// If the destination is an external URL, remove it.
if (isset($_GET['destination']) && url_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']) && url_is_external($_REQUEST['destination'])) {
unset($_REQUEST['destination']);
}
}
}
/**
......
......@@ -2214,14 +2214,20 @@ function url($path = NULL, array $options = array()) {
'prefix' => ''
);
// A duplicate of the code from url_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 drupal_strip_dangerous_protocols() if $path contains a ':'
// before any / ? or #. Note: we could use url_is_external($path) here, but
// that would require another function call, and performance inside url() is
// critical.
// Return an external link if $path contains an allowed absolute URL. Avoid
// calling drupal_strip_dangerous_protocols() 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)) && drupal_strip_dangerous_protocols($path) == $path);
$options['external'] = (strpos($path, '//') === 0)
|| ($colonpos !== FALSE
&& !preg_match('![/?#]!', substr($path, 0, $colonpos))
&& drupal_strip_dangerous_protocols($path) == $path);
}
// Preserve the original path before altering or aliasing.
......@@ -2259,6 +2265,11 @@ function url($path = NULL, array $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, $base_secure_url, $base_insecure_url;
// The base_url might be rewritten from the language rewrite in domain mode.
......@@ -2336,10 +2347,15 @@ function url($path = NULL, array $options = array()) {
*/
function url_is_external($path) {
$colonpos = strpos($path, ':');
// Avoid calling drupal_strip_dangerous_protocols() 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.
return $colonpos !== FALSE && !preg_match('![/?#]!', substr($path, 0, $colonpos)) && drupal_strip_dangerous_protocols($path) == $path;
// Avoid calling drupal_strip_dangerous_protocols() 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.
return (strpos($path, '//') === 0)
|| ($colonpos !== FALSE
&& !preg_match('![/?#]!', substr($path, 0, $colonpos))
&& drupal_strip_dangerous_protocols($path) == $path);
}
/**
......@@ -2636,8 +2652,11 @@ function drupal_deliver_html_page($page_callback_result) {
// Keep old path for reference, and to allow forms to redirect to it.
if (!isset($_GET['destination'])) {
// Make sure that the current path is not interpreted as external URL.
if (!url_is_external($_GET['q'])) {
$_GET['destination'] = $_GET['q'];
}
}
$path = drupal_get_normal_path(variable_get('site_404', ''));
if ($path && $path != $_GET['q']) {
......@@ -2665,8 +2684,11 @@ function drupal_deliver_html_page($page_callback_result) {
// Keep old path for reference, and to allow forms to redirect to it.
if (!isset($_GET['destination'])) {
// Make sure that the current path is not interpreted as external URL.
if (!url_is_external($_GET['q'])) {
$_GET['destination'] = $_GET['q'];
}
}
$path = drupal_get_normal_path(variable_get('site_403', ''));
if ($path && $path != $_GET['q']) {
......
......@@ -546,3 +546,85 @@ class BootstrapOverrideServerVariablesTestCase extends DrupalUnitTestCase {
}
}
}
/**
* Tests for $_GET['destination'] and $_REQUEST['destination'] validation.
*/
class BootstrapDestinationTestCase extends DrupalWebTestCase {
public static function getInfo() {
return array(
'name' => 'URL destination validation',
'description' => 'Test that $_GET[\'destination\'] and $_REQUEST[\'destination\'] cannot contain external URLs.',
'group' => 'Bootstrap',
);
}
function setUp() {
parent::setUp('system_test');
}
/**
* Tests that $_GET/$_REQUEST['destination'] only contain internal URLs.
*
* @see _drupal_bootstrap_variables()
* @see system_test_get_destination()
* @see system_test_request_destination()
*/
public function testDestination() {
$test_cases = array(
array(
'input' => 'node',
'output' => 'node',
'message' => "Standard internal example node path is present in the 'destination' parameter.",
),
array(
'input' => '/example.com',
'output' => '/example.com',
'message' => 'Internal path with one leading slash is allowed.',
),
array(
'input' => '//example.com/test',
'output' => '',
'message' => 'External URL without scheme is not allowed.',
),
array(
'input' => 'example:test',
'output' => 'example:test',
'message' => 'Internal URL using a colon is allowed.',
),
array(
'input' => 'http://example.com',
'output' => '',
'message' => 'External URL is not allowed.',
),
array(
'input' => 'javascript:alert(0)',
'output' => 'javascript:alert(0)',
'message' => 'Javascript URL is allowed because it is treated as an internal URL.',
),
);
foreach ($test_cases as $test_case) {
// Test $_GET['destination'].
$this->drupalGet('system-test/get-destination', array('query' => array('destination' => $test_case['input'])));
$this->assertIdentical($test_case['output'], $this->drupalGetContent(), $test_case['message']);
// Test $_REQUEST['destination']. There's no form to submit to, so
// drupalPost() won't work here; this just tests a direct $_POST request
// instead.
$curl_parameters = array(
CURLOPT_URL => $this->getAbsoluteUrl('system-test/request-destination'),
CURLOPT_POST => TRUE,
CURLOPT_POSTFIELDS => 'destination=' . urlencode($test_case['input']),
CURLOPT_HTTPHEADER => array(),
);
$post_output = $this->curlExec($curl_parameters);
$this->assertIdentical($test_case['output'], $post_output, $test_case['message']);
}
// Make sure that 404 pages do not populate $_GET['destination'] with
// external URLs.
variable_set('site_404', 'system-test/get-destination');
$this->drupalGet('http://example.com', array('external' => FALSE));
$this->assertIdentical('', $this->drupalGetContent(), 'External URL is not allowed on 404 pages.');
}
}
......@@ -209,7 +209,16 @@ class CommonURLUnitTest extends DrupalWebTestCase {
// Test that drupal can recognize an absolute URL. Used to prevent attack vectors.
$this->assertTrue(url_is_external($url), 'Correctly identified an external URL.');
// External URL without an explicit protocol.
$url = '//drupal.org/foo/bar?foo=bar&bar=baz&baz#foo';
$this->assertTrue(url_is_external($url), 'Correctly identified an external URL without a protocol part.');
// Internal URL starting with a slash.
$url = '/drupal.org';
$this->assertFalse(url_is_external($url), 'Correctly identified an internal URL with a leading slash.');
// Test the parsing of absolute URLs.
$url = 'http://drupal.org/foo/bar?foo=bar&bar=baz&baz#foo';
$result = array(
'path' => 'http://drupal.org/foo/bar',
'query' => array('foo' => 'bar', 'bar' => 'baz', 'baz' => ''),
......@@ -349,6 +358,17 @@ class CommonURLUnitTest extends DrupalWebTestCase {
$query = array($this->randomName(5) => $this->randomName(5));
$result = url($url, array('query' => $query));
$this->assertEqual($url . '&' . http_build_query($query, '', '&'), $result, 'External URL query string can be extended with a custom query string in $options.');
// Verify that an internal URL does not result in an external URL without
// protocol part.
$url = '/drupal.org';
$result = url($url);
$this->assertTrue(strpos($result, '//') === FALSE, 'Internal URL does not turn into an external URL.');
// Verify that an external URL without protocol part is recognized as such.
$url = '//drupal.org';
$result = url($url);
$this->assertEqual($url, $result, 'External URL without protocol is not altered.');
}
}
......
......@@ -106,6 +106,20 @@ function system_test_menu() {
'type' => MENU_CALLBACK,
);
$items['system-test/get-destination'] = array(
'title' => 'Test $_GET[\'destination\']',
'page callback' => 'system_test_get_destination',
'access callback' => TRUE,
'type' => MENU_CALLBACK,
);
$items['system-test/request-destination'] = array(
'title' => 'Test $_REQUEST[\'destination\']',
'page callback' => 'system_test_request_destination',
'access callback' => TRUE,
'type' => MENU_CALLBACK,
);
return $items;
}
......@@ -420,3 +434,27 @@ function system_test_authorize_init_page($page_title) {
system_authorized_init('system_test_authorize_run', drupal_get_path('module', 'system_test') . '/system_test.module', array(), $page_title);
drupal_goto($authorize_url);
}
/**
* Page callback to print out $_GET['destination'] for testing.
*/
function system_test_get_destination() {
if (isset($_GET['destination'])) {
print $_GET['destination'];
}
// No need to render the whole page, we are just interested in this bit of
// information.
exit;
}
/**
* Page callback to print out $_REQUEST['destination'] for testing.
*/
function system_test_request_destination() {
if (isset($_REQUEST['destination'])) {
print $_REQUEST['destination'];
}
// No need to render the whole page, we are just interested in this bit of
// information.
exit;
}
......@@ -414,7 +414,7 @@ class StatisticsAdminTestCase extends DrupalWebTestCase {
$timestamp = time();
$this->drupalPost(NULL, NULL, t('Cancel account'));
// Confirm account cancellation request.
$this->drupalGet("user/$account->uid/cancel/confirm/$timestamp/" . user_pass_rehash($account->pass, $timestamp, $account->login));
$this->drupalGet("user/$account->uid/cancel/confirm/$timestamp/" . user_pass_rehash($account->pass, $timestamp, $account->login, $account->uid));
$this->assertFalse(user_load($account->uid, TRUE), 'User is not found in the database.');
$this->drupalGet('admin/reports/visitors');
......
......@@ -2825,3 +2825,46 @@ class SystemValidTokenTest extends DrupalUnitTestCase {
return TRUE;
}
}
/**
* Tests confirm form destinations.
*/
class ConfirmFormTest extends DrupalWebTestCase {
protected $admin_user;
public static function getInfo() {
return array(
'name' => 'Confirm form',
'description' => 'Tests that the confirm form does not use external destinations.',
'group' => 'System',
);
}
function setUp() {
parent::setUp();
$this->admin_user = $this->drupalCreateUser(array('administer users'));
$this->drupalLogin($this->admin_user);
}
/**
* Tests that the confirm form does not use external destinations.
*/
function testConfirmForm() {
$this->drupalGet('user/1/cancel');
$this->assertCancelLinkUrl(url('user/1'));
$this->drupalGet('user/1/cancel', array('query' => array('destination' => 'node')));
$this->assertCancelLinkUrl(url('node'));
$this->drupalGet('user/1/cancel', array('query' => array('destination' => 'http://example.com')));
$this->assertCancelLinkUrl(url('user/1'));
}
/**
* Asserts that a cancel link is present pointing to the provided URL.
*/
function assertCancelLinkUrl($url, $message = '', $group = 'Other') {
$links = $this->xpath('//a[normalize-space(text())=:label and @href=:url]', array(':label' => t('Cancel'), ':url' => $url));
$message = ($message ? $message : format_string('Cancel link with url %url found.', array('%url' => $url)));
return $this->assertTrue(isset($links[0]), $message, $group);
}
}
......@@ -2335,7 +2335,7 @@ function user_external_login_register($name, $module) {
*/
function user_pass_reset_url($account) {
$timestamp = REQUEST_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));
}
/**
......@@ -2357,7 +2357,7 @@ function user_pass_reset_url($account) {
*/
function user_cancel_url($account) {
$timestamp = REQUEST_TIME;
return url("user/$account->uid/cancel/confirm/$timestamp/" . user_pass_rehash($account->pass, $timestamp, $account->login), array('absolute' => TRUE));
return url("user/$account->uid/cancel/confirm/$timestamp/" . user_pass_rehash($account->pass, $timestamp, $account->login, $account->uid), array('absolute' => TRUE));
}
/**
......@@ -2377,12 +2377,33 @@ function user_cancel_url($account) {
* A UNIX timestamp, typically REQUEST_TIME.
* @param int $login
* The UNIX timestamp of the user's last login.
* @param int $uid
* The user ID of the user account.
*
* @return
* A string that is safe for use in URLs and SQL statements.
*/
function user_pass_rehash($password, $timestamp, $login) {
return drupal_hmac_base64($timestamp . $login, drupal_get_hash_salt() . $password);
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 = db_query_range('SELECT uid FROM {users} WHERE pass = :password AND login = :login AND uid > 0', 0, 2, array(':password' => $password, ':login' => $login))->fetchCol();
// 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_hash_salt() . $password);
}
/**
......
......@@ -126,7 +126,7 @@ function user_pass_reset($form, &$form_state, $uid, $timestamp, $hashed_pass, $a
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');
}
elseif ($account->uid && $timestamp >= $account->login && $timestamp <= $current && $hashed_pass == user_pass_rehash($account->pass, $timestamp, $account->login)) {
elseif ($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') {
// Set the new user.
......@@ -523,7 +523,7 @@ function user_cancel_confirm($account, $timestamp = 0, $hashed_pass = '') {
// Basic validation of arguments.
if (isset($account->data['user_cancel_method']) && !empty($timestamp) && !empty($hashed_pass)) {
// Validate expiration and hashed password/login.
if ($timestamp <= $current && $current - $timestamp < $timeout && $account->uid && $timestamp >= $account->login && $hashed_pass == user_pass_rehash($account->pass, $timestamp, $account->login)) {
if ($timestamp <= $current && $current - $timestamp < $timeout && $account->uid && $timestamp >= $account->login && $hashed_pass == user_pass_rehash($account->pass, $timestamp, $account->login, $account->uid)) {
$edit = array(
'user_cancel_notify' => isset($account->data['user_cancel_notify']) ? $account->data['user_cancel_notify'] : variable_get('user_mail_status_canceled_notify', FALSE),
);
......
This diff is collapsed.
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment