Issue #736172 by fizk: Fixed drupal_goto() should allow absolute destinations...

Issue #736172 by fizk: Fixed drupal_goto() should allow absolute destinations that are within the same domain.

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'];
@@ -706,6 +709,25 @@ function drupal_goto($path = '', array $options = array(), $http_response_code =
   drupal_exit($url);
 }
 
+/**
+ * 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.
  *

core/modules/system/lib/Drupal/system/Tests/Common/GotoTest.php

@@ -47,12 +47,26 @@ function testDrupalGoto() {
     $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.');
   }
 
   /**

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',
@@ -84,6 +90,15 @@ function common_test_drupal_goto_land() {
   print "drupal_goto";
 }
 
+/**
+ * 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().
  *