SECURITY: Do not directly redirect to interwikis, but use splash page
authorBrian Wolff <bawolff+wn@gmail.com>
Sun, 7 Feb 2016 13:07:20 +0000 (08:07 -0500)
committerChad Horohoe <chadh@wikimedia.org>
Thu, 6 Apr 2017 20:42:38 +0000 (13:42 -0700)
Directly redirecting based on a url paramter might potentially
be used in a phishing attack to confuse users.

Bug: T109140
Bug: T122209
Change-Id: I6c604439320fa876719933cc7f3a3ff04fb1a6ad

16 files changed:
RELEASE-NOTES-1.29
autoload.php
includes/OutputPage.php
includes/Title.php
includes/specialpage/RedirectSpecialPage.php
includes/specialpage/SpecialPageFactory.php
includes/specials/SpecialChangeCredentials.php
includes/specials/SpecialChangeEmail.php
includes/specials/SpecialGoToInterwiki.php [new file with mode: 0644]
includes/specials/SpecialPageLanguage.php
includes/specials/SpecialPreferences.php
includes/specials/SpecialSearch.php
includes/specials/helpers/LoginHelper.php
languages/i18n/en.json
languages/i18n/qqq.json
languages/messages/MessagesEn.php

index 3d3f9ca..11f961e 100644 (file)
@@ -86,6 +86,8 @@ production.
 * (T157035) "new mw.Uri()" was ignoring options when using default URI.
 * Special:Allpages can no longer be filtered by redirect in miser mode.
 * (T160519) CACHE_ANYTHING will not be CACHE_ACCEL if no accelerator is installed.
 * (T157035) "new mw.Uri()" was ignoring options when using default URI.
 * Special:Allpages can no longer be filtered by redirect in miser mode.
 * (T160519) CACHE_ANYTHING will not be CACHE_ACCEL if no accelerator is installed.
+* (T109140) (T122209) SECURITY: Special:UserLogin and Special:Search allow redirect
+  to interwiki links.
 
 === Action API changes in 1.29 ===
 * Submitting sensitive authentication request parameters to action=login,
 
 === Action API changes in 1.29 ===
 * Submitting sensitive authentication request parameters to action=login,
index 21f75d1..3816485 100644 (file)
@@ -1340,6 +1340,7 @@ $wgAutoloadLocalClasses = [
        'SpecialExpandTemplates' => __DIR__ . '/includes/specials/SpecialExpandTemplates.php',
        'SpecialExport' => __DIR__ . '/includes/specials/SpecialExport.php',
        'SpecialFilepath' => __DIR__ . '/includes/specials/SpecialFilepath.php',
        'SpecialExpandTemplates' => __DIR__ . '/includes/specials/SpecialExpandTemplates.php',
        'SpecialExport' => __DIR__ . '/includes/specials/SpecialExport.php',
        'SpecialFilepath' => __DIR__ . '/includes/specials/SpecialFilepath.php',
+       'SpecialGoToInterwiki' => __DIR__ . '/includes/specials/SpecialGoToInterwiki.php',
        'SpecialImport' => __DIR__ . '/includes/specials/SpecialImport.php',
        'SpecialJavaScriptTest' => __DIR__ . '/includes/specials/SpecialJavaScriptTest.php',
        'SpecialLinkAccounts' => __DIR__ . '/includes/specials/SpecialLinkAccounts.php',
        'SpecialImport' => __DIR__ . '/includes/specials/SpecialImport.php',
        'SpecialJavaScriptTest' => __DIR__ . '/includes/specials/SpecialJavaScriptTest.php',
        'SpecialLinkAccounts' => __DIR__ . '/includes/specials/SpecialLinkAccounts.php',
index d3e1373..5380264 100644 (file)
@@ -2727,7 +2727,9 @@ class OutputPage extends ContextSource {
                } else {
                        $titleObj = Title::newFromText( $returnto );
                }
                } else {
                        $titleObj = Title::newFromText( $returnto );
                }
-               if ( !is_object( $titleObj ) ) {
+               // We don't want people to return to external interwiki. That
+               // might potentially be used as part of a phishing scheme
+               if ( !is_object( $titleObj ) || $titleObj->isExternal() ) {
                        $titleObj = Title::newMainPage();
                }
 
                        $titleObj = Title::newMainPage();
                }
 
index f16f0c5..f1cf81f 100644 (file)
@@ -1683,6 +1683,33 @@ class Title implements LinkTarget {
                return $url;
        }
 
                return $url;
        }
 
+       /**
+        * Get a url appropriate for making redirects based on an untrusted url arg
+        *
+        * This is basically the same as getFullUrl(), but in the case of external
+        * interwikis, we send the user to a landing page, to prevent possible
+        * phishing attacks and the like.
+        *
+        * @note Uses current protocol by default, since technically relative urls
+        *   aren't allowed in redirects per HTTP spec, so this is not suitable for
+        *   places where the url gets cached, as might pollute between
+        *   https and non-https users.
+        * @see self::getLocalURL for the arguments.
+        * @param array|string $query
+        * @param string $proto Protocol type to use in URL
+        * @return String. A url suitable to use in an HTTP location header.
+        */
+       public function getFullUrlForRedirect( $query = '', $proto = PROTO_CURRENT ) {
+               $target = $this;
+               if ( $this->isExternal() ) {
+                       $target = SpecialPage::getTitleFor(
+                               'GoToInterwiki',
+                               $this->getPrefixedDBKey()
+                       );
+               }
+               return $target->getFullUrl( $query, false, $proto );
+       }
+
        /**
         * Get a URL with no fragment or server name (relative URL) from a Title object.
         * If this page is generated with action=render, however,
        /**
         * Get a URL with no fragment or server name (relative URL) from a Title object.
         * If this page is generated with action=render, however,
index b1ddacf..9b5d5f4 100644 (file)
@@ -41,7 +41,7 @@ abstract class RedirectSpecialPage extends UnlistedSpecialPage {
                $query = $this->getRedirectQuery();
                // Redirect to a page title with possible query parameters
                if ( $redirect instanceof Title ) {
                $query = $this->getRedirectQuery();
                // Redirect to a page title with possible query parameters
                if ( $redirect instanceof Title ) {
-                       $url = $redirect->getFullURL( $query );
+                       $url = $redirect->getFullUrlForRedirect( $query );
                        $this->getOutput()->redirect( $url );
 
                        return $redirect;
                        $this->getOutput()->redirect( $url );
 
                        return $redirect;
index 33e1cc3..ae010f6 100644 (file)
@@ -144,6 +144,7 @@ class SpecialPageFactory {
                'RandomInCategory' => 'SpecialRandomInCategory',
                'Randomredirect' => 'SpecialRandomredirect',
                'Randomrootpage' => 'SpecialRandomrootpage',
                'RandomInCategory' => 'SpecialRandomInCategory',
                'Randomredirect' => 'SpecialRandomredirect',
                'Randomrootpage' => 'SpecialRandomrootpage',
+               'GoToInterwiki' => 'SpecialGoToInterwiki',
 
                // High use pages
                'Mostlinkedcategories' => 'MostlinkedCategoriesPage',
 
                // High use pages
                'Mostlinkedcategories' => 'MostlinkedCategoriesPage',
index 47f8d2f..970a2e2 100644 (file)
@@ -258,7 +258,7 @@ class SpecialChangeCredentials extends AuthManagerSpecialPage {
                }
 
                $title = Title::newFromText( $returnTo );
                }
 
                $title = Title::newFromText( $returnTo );
-               return $title->getFullURL( $returnToQuery );
+               return $title->getFullUrlForRedirect( $returnToQuery );
        }
 
        protected function getRequestBlacklist() {
        }
 
        protected function getRequestBlacklist() {
index 785447f..eb98fe7 100644 (file)
@@ -136,7 +136,7 @@ class SpecialChangeEmail extends FormSpecialPage {
                $query = $request->getVal( 'returntoquery' );
 
                if ( $this->status->value === true ) {
                $query = $request->getVal( 'returntoquery' );
 
                if ( $this->status->value === true ) {
-                       $this->getOutput()->redirect( $titleObj->getFullURL( $query ) );
+                       $this->getOutput()->redirect( $titleObj->getFullUrlForRedirect( $query ) );
                } elseif ( $this->status->value === 'eauth' ) {
                        # Notify user that a confirmation email has been sent...
                        $this->getOutput()->wrapWikiMsg( "<div class='error' style='clear: both;'>\n$1\n</div>",
                } elseif ( $this->status->value === 'eauth' ) {
                        # Notify user that a confirmation email has been sent...
                        $this->getOutput()->wrapWikiMsg( "<div class='error' style='clear: both;'>\n$1\n</div>",
diff --git a/includes/specials/SpecialGoToInterwiki.php b/includes/specials/SpecialGoToInterwiki.php
new file mode 100644 (file)
index 0000000..809a14a
--- /dev/null
@@ -0,0 +1,79 @@
+<?php
+/**
+ * Implements Special:GoToInterwiki
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License along
+ * with this program; if not, write to the Free Software Foundation, Inc.,
+ * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
+ * http://www.gnu.org/copyleft/gpl.html
+ *
+ * @file
+ * @ingroup SpecialPage
+ */
+
+/**
+ * Landing page for non-local interwiki links.
+ *
+ * Meant to warn people that the site they're visiting
+ * is not the local wiki (In case of phishing tricks).
+ * Only meant to be used for things that directly
+ * redirect from url (e.g. Special:Search/google:foo )
+ * Not meant for general interwiki linking (e.g.
+ * [[google:foo]] should still directly link)
+ *
+ * @ingroup SpecialPage
+ */
+class SpecialGoToInterwiki extends UnlistedSpecialPage {
+       public function __construct( $name = 'GoToInterwiki' ) {
+               parent::__construct( $name );
+       }
+
+       public function execute( $par ) {
+               $this->setHeaders();
+               $target = Title::newFromText( $par );
+               // Disallow special pages as a precaution against
+               // possible redirect loops.
+               if ( !$target || $target->isSpecialPage() ) {
+                       $this->getOutput()->setStatusCode( 404 );
+                       $this->getOutput()->addWikiMsg( 'gotointerwiki-invalid' );
+                       return;
+               }
+
+               $url = $target->getFullURL();
+               if ( !$target->isExternal() || $target->isLocal() ) {
+                       // Either a normal page, or a local interwiki.
+                       // just redirect.
+                       $this->getOutput()->redirect( $url, '301' );
+               } else {
+                       $this->getOutput()->addWikiMsg(
+                               'gotointerwiki-external',
+                               $url,
+                               $target->getFullText()
+                       );
+               }
+       }
+
+       /**
+        * @return bool
+        */
+       public function requiresWrite() {
+               return false;
+       }
+
+       /**
+        * @return String
+        */
+       protected function getGroupName() {
+               return 'redirects';
+       }
+}
index db05ebe..2943fd4 100644 (file)
@@ -136,7 +136,7 @@ class SpecialPageLanguage extends FormSpecialPage {
                }
 
                // Url to redirect to after the operation
                }
 
                // Url to redirect to after the operation
-               $this->goToUrl = $title->getFullURL(
+               $this->goToUrl = $title->getFullUrlForRedirect(
                        $title->isRedirect() ? [ 'redirect' => 'no' ] : []
                );
 
                        $title->isRedirect() ? [ 'redirect' => 'no' ] : []
                );
 
index eee5b64..40b50ea 100644 (file)
@@ -148,7 +148,7 @@ class SpecialPreferences extends SpecialPage {
                // Set session data for the success message
                $this->getRequest()->getSession()->set( 'specialPreferencesSaveSuccess', 1 );
 
                // Set session data for the success message
                $this->getRequest()->getSession()->set( 'specialPreferencesSaveSuccess', 1 );
 
-               $url = $this->getPageTitle()->getFullURL();
+               $url = $this->getPageTitle()->getFullUrlForRedirect();
                $this->getOutput()->redirect( $url );
 
                return true;
                $this->getOutput()->redirect( $url );
 
                return true;
index 9cc0048..139e4f7 100644 (file)
@@ -259,7 +259,7 @@ class SpecialSearch extends SpecialPage {
                        return null;
                }
 
                        return null;
                }
 
-               return $url === null ? $title->getFullURL() : $url;
+               return $url === null ? $title->getFullUrlForRedirect() : $url;
        }
 
        /**
        }
 
        /**
index f853f41..cfcbf65 100644 (file)
@@ -89,7 +89,7 @@ class LoginHelper extends ContextSource {
                }
 
                if ( $type === 'successredirect' ) {
                }
 
                if ( $type === 'successredirect' ) {
-                       $redirectUrl = $returnToTitle->getFullURL( $returnToQuery, false, $proto );
+                       $redirectUrl = $returnToTitle->getFullUrlForRedirect( $returnToQuery, $proto );
                        $this->getOutput()->redirect( $redirectUrl );
                } else {
                        $this->getOutput()->addReturnTo( $returnToTitle, $returnToQuery, null, $options );
                        $this->getOutput()->redirect( $redirectUrl );
                } else {
                        $this->getOutput()->addReturnTo( $returnToTitle, $returnToQuery, null, $options );
index a49a4b4..9c4753c 100644 (file)
        "restrictionsfield-help": "One IP address or CIDR range per line. To enable everything, use:<pre>0.0.0.0/0\n::/0</pre>",
        "revid": "revision $1",
        "pageid": "page ID $1",
        "restrictionsfield-help": "One IP address or CIDR range per line. To enable everything, use:<pre>0.0.0.0/0\n::/0</pre>",
        "revid": "revision $1",
        "pageid": "page ID $1",
-       "rawhtml-notallowed": "&lt;html&gt; tags cannot be used outside of normal pages."
+       "rawhtml-notallowed": "&lt;html&gt; tags cannot be used outside of normal pages.",
+       "gotointerwiki": "Leaving {{SITENAME}}",
+       "gotointerwiki-invalid": "The specified title was invalid.",
+       "gotointerwiki-external": "You are about to leave {{SITENAME}} to visit [[$2]] which is a separate website.\n\n[$1 Click here to continue on to $1]."
 }
 }
index 01a8fe7..ba99a55 100644 (file)
        "restrictionsfield-help": "Placeholder text displayed in restriction fields (e.g. on Special:BotPassword).",
        "revid": "Used to format a revision ID number in text. Parameters:\n* $1 - Revision ID number.\n{{Identical|Revision}}",
        "pageid": "Used to format a page ID number in text. Parameters:\n* $1 - Page ID number.",
        "restrictionsfield-help": "Placeholder text displayed in restriction fields (e.g. on Special:BotPassword).",
        "revid": "Used to format a revision ID number in text. Parameters:\n* $1 - Revision ID number.\n{{Identical|Revision}}",
        "pageid": "Used to format a page ID number in text. Parameters:\n* $1 - Page ID number.",
-       "rawhtml-notallowed": "Error message given when $wgRawHtml = true; is set and a user uses an &lt;html&gt; tag in a system message or somewhere other than a normal page."
+       "rawhtml-notallowed": "Error message given when $wgRawHtml = true; is set and a user uses an &lt;html&gt; tag in a system message or somewhere other than a normal page.",
+       "gotointerwiki": "{{doc-special|GoToInterwiki}}\n\nSpecial:GoToInterwiki is a warning page displayed before redirecting users to external interwiki links. Its triggered by people going to something like [[Special:Search/google:foo]].",
+       "gotointerwiki-invalid": "Message shown on Special:GoToInterwiki if given an invalid title.",
+       "gotointerwiki-external": "Message shown on Special:GoToInterwiki if given a external interwiki link (e.g. [[Special:GoToInterwiki/Google:Foo]]). $1 is the full url the user is trying to get to. $2 is the text of the interwiki link (e.g. \"Google:foo\")."
 }
 }
index b9280ea..c52ec0e 100644 (file)
@@ -427,6 +427,7 @@ $specialPageAliases = [
        'Fewestrevisions'           => [ 'FewestRevisions' ],
        'FileDuplicateSearch'       => [ 'FileDuplicateSearch' ],
        'Filepath'                  => [ 'FilePath' ],
        'Fewestrevisions'           => [ 'FewestRevisions' ],
        'FileDuplicateSearch'       => [ 'FileDuplicateSearch' ],
        'Filepath'                  => [ 'FilePath' ],
+       'GoToInterwiki'             => [ 'GoToInterwiki' ],
        'Import'                    => [ 'Import' ],
        'Invalidateemail'           => [ 'InvalidateEmail' ],
        'JavaScriptTest'            => [ 'JavaScriptTest' ],
        'Import'                    => [ 'Import' ],
        'Invalidateemail'           => [ 'InvalidateEmail' ],
        'JavaScriptTest'            => [ 'JavaScriptTest' ],