diff --git a/includes/common.inc b/includes/common.inc index a38d13b..e833ad2 100644 --- a/includes/common.inc +++ b/includes/common.inc @@ -680,8 +680,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']; @@ -704,6 +707,35 @@ 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); + + if (!isset($url_parts['path'])) { + $url_parts['path'] = '/'; + } + + // Force the external path to have a trailing slash for the comparison below, + // since base_path() always has one. + if (substr($url_parts['path'], -1) != '/') { + $url_parts['path'] .= '/'; + } + + // 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); +} + +/** * Delivers a "site is under maintenance" message to the browser. * * Page callback functions wanting to report a "site offline" message should diff --git a/modules/simpletest/tests/common.test b/modules/simpletest/tests/common.test index 6e03253..c145ea2 100644 --- a/modules/simpletest/tests/common.test +++ b/modules/simpletest/tests/common.test @@ -1230,12 +1230,26 @@ class DrupalGotoTest extends DrupalWebTestCase { $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/alternative', array('absolute' => TRUE)); + $this->drupalGet('common-test/drupal_goto/redirect', array('query' => array('destination' => $destination))); + $this->assertText('drupal_goto_alternative', 'Drupal goto redirect with absolute URL destination that points to this Drupal installation succeeded.'); + $this->assertEqual($this->getUrl(), url('common-test/drupal_goto/alternative', 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://example.com'; + $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/modules/simpletest/tests/common_test.module b/modules/simpletest/tests/common_test.module index 674a494..8803022 100644 --- a/modules/simpletest/tests/common_test.module +++ b/modules/simpletest/tests/common_test.module @@ -15,6 +15,12 @@ function common_test_menu() { 'access arguments' => array('access content'), 'type' => MENU_CALLBACK, ); + $items['common-test/drupal_goto/alternative'] = array( + 'title' => 'Drupal Goto', + 'page callback' => 'common_test_drupal_goto_land_alternative', + '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', @@ -77,6 +83,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_alternative() { + print "drupal_goto_alternative"; +} + +/** * Fail landing page for drupal_goto(). */ function common_test_drupal_goto_land_fail() {