summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorcatch2012-10-29 22:46:16 (GMT)
committer catch2012-10-29 22:46:16 (GMT)
commit17288d55b5d7d73a7e29f22a0bf6aea29eb68a82 (patch)
tree9f347426575a0f95f450c5f1a198f76a2c6580f6
parent2d22def9cfbda32b53d1e11ac2d205d75a5f23e1 (diff)
Issue #736172 by fizk: Fixed drupal_goto() should allow absolute destinations that are within the same domain.
-rw-r--r--core/includes/common.inc26
-rw-r--r--core/modules/system/lib/Drupal/system/Tests/Common/GotoTest.php16
-rw-r--r--core/modules/system/tests/modules/common_test/common_test.module15
3 files changed, 54 insertions, 3 deletions
diff --git a/core/includes/common.inc b/core/includes/common.inc
index 9601c76..c13a3cf 100644
--- a/core/includes/common.inc
+++ b/core/includes/common.inc
@@ -683,8 +683,11 @@ function drupal_encode_path($path) {
*/
function drupal_goto($path = '', array $options = array(), $http_response_code = 302) {
// A destination in $_GET always overrides the function arguments.
- // We do not allow absolute URLs to be passed via $_GET, as this can be an attack vector.
- if (isset($_GET['destination']) && !url_is_external($_GET['destination'])) {
+ // We do not allow absolute URLs to be passed via $_GET, as this can be an
+ // attack vector, with the following exception:
+ // - absolute URLs that point to this site (i.e. same base URL and
+ // base path) are allowed
+ if (isset($_GET['destination']) && (!url_is_external($_GET['destination']) || _external_url_is_local($_GET['destination']))) {
$destination = drupal_parse_url($_GET['destination']);
$path = $destination['path'];
$options['query'] = $destination['query'];
@@ -707,6 +710,25 @@ function drupal_goto($path = '', array $options = array(), $http_response_code =
}
/**
+ * Determines if an external URL points to this Drupal installation.
+ *
+ * @param $url
+ * A string containing an external URL, such as "http://example.com/foo".
+ *
+ * @return
+ * TRUE if the URL has the same domain and base path.
+ */
+function _external_url_is_local($url) {
+ $url_parts = parse_url($url);
+ $base_host = parse_url($GLOBALS['base_url'], PHP_URL_HOST);
+
+ // When comparing base paths, we need a trailing slash to make sure a
+ // partial URL match isn't occuring. Since base_path() always returns with
+ // a trailing slash, we don't need to add the trailing slash here.
+ return ($url_parts['host'] == $base_host && stripos($url_parts['path'], base_path()) === 0);
+}
+
+/**
* Performs an HTTP request.
*
* This is a flexible and powerful HTTP client implementation. Correctly
diff --git a/core/modules/system/lib/Drupal/system/Tests/Common/GotoTest.php b/core/modules/system/lib/Drupal/system/Tests/Common/GotoTest.php
index c353e69..edf59a8 100644
--- a/core/modules/system/lib/Drupal/system/Tests/Common/GotoTest.php
+++ b/core/modules/system/lib/Drupal/system/Tests/Common/GotoTest.php
@@ -47,12 +47,26 @@ class GotoTest extends WebTestBase {
$this->assertText('drupal_goto', 'Drupal goto redirect succeeded.');
$this->assertEqual($this->getUrl(), url('common-test/drupal_goto', array('query' => array('foo' => '123'), 'absolute' => TRUE)), 'Drupal goto redirected to expected URL.');
- // Test that drupal_goto() respects ?destination=xxx. Use an complicated URL
+ // Test that drupal_goto() respects ?destination=xxx. Use a complicated URL
// to test that the path is encoded and decoded properly.
$destination = 'common-test/drupal_goto/destination?foo=%2525&bar=123';
$this->drupalGet('common-test/drupal_goto/redirect', array('query' => array('destination' => $destination)));
$this->assertText('drupal_goto', 'Drupal goto redirect with destination succeeded.');
$this->assertEqual($this->getUrl(), url('common-test/drupal_goto/destination', array('query' => array('foo' => '%25', 'bar' => '123'), 'absolute' => TRUE)), 'Drupal goto redirected to given query string destination.');
+
+ // Test that drupal_goto() respects ?destination=xxx with an absolute URL
+ // that points to this Drupal installation.
+ $destination = url('common-test/drupal_goto/alt', array('absolute' => TRUE));
+ $this->drupalGet('common-test/drupal_goto/redirect', array('query' => array('destination' => $destination)));
+ $this->assertText('drupal_goto_alt', 'Drupal goto redirect with absolute URL destination that points to this Drupal installation succeeded.');
+ $this->assertEqual($this->getUrl(), url('common-test/drupal_goto/alt', array('absolute' => TRUE)), 'Drupal goto redirected to given query string destination with absolute URL that points to this Drupal installation.');
+
+ // Test that drupal_goto() fails to respect ?destination=xxx with an absolute URL
+ // that does not point to this Drupal installation.
+ $destination = 'http://pagedoesnotexist';
+ $this->drupalGet('common-test/drupal_goto/redirect', array('query' => array('destination' => $destination)));
+ $this->assertText('drupal_goto', 'Drupal goto fails to redirect with absolute URL destination that does not point to this Drupal installation.');
+ $this->assertNotEqual($this->getUrl(), $destination, 'Drupal goto failed to redirect to given query string destination with absolute URL that does not point to this Drupal installation.');
}
/**
diff --git a/core/modules/system/tests/modules/common_test/common_test.module b/core/modules/system/tests/modules/common_test/common_test.module
index 0634e3b..e5430ff 100644
--- a/core/modules/system/tests/modules/common_test/common_test.module
+++ b/core/modules/system/tests/modules/common_test/common_test.module
@@ -15,6 +15,12 @@ function common_test_menu() {
'access arguments' => array('access content'),
'type' => MENU_CALLBACK,
);
+ $items['common-test/drupal_goto/alt'] = array(
+ 'title' => 'Drupal Goto',
+ 'page callback' => 'common_test_drupal_goto_land_alt',
+ 'access arguments' => array('access content'),
+ 'type' => MENU_CALLBACK,
+ );
$items['common-test/drupal_goto/fail'] = array(
'title' => 'Drupal Goto',
'page callback' => 'common_test_drupal_goto_land_fail',
@@ -85,6 +91,15 @@ function common_test_drupal_goto_land() {
}
/**
+ * Page callback: Provides a landing page for drupal_goto().
+ *
+ * @see common_test_menu()
+ */
+function common_test_drupal_goto_land_alt() {
+ print "drupal_goto_alt";
+}
+
+/**
* Page callback: Provides a failure landing page for drupal_goto().
*
* @see common_test_menu()