jenkins-bot has submitted this change and it was merged. ( https://gerrit.wikimedia.org/r/346848 )
Change subject: SECURITY: Do not directly redirect to interwikis, but use splash page ...................................................................... SECURITY: Do not directly redirect to interwikis, but use splash page 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 --- M RELEASE-NOTES-1.27 M autoload.php M includes/OutputPage.php M includes/Title.php M includes/specialpage/RedirectSpecialPage.php M includes/specialpage/SpecialPageFactory.php M includes/specials/SpecialChangeCredentials.php M includes/specials/SpecialChangeEmail.php A includes/specials/SpecialGoToInterwiki.php M includes/specials/SpecialPageLanguage.php M includes/specials/SpecialPreferences.php M includes/specials/SpecialSearch.php M includes/specials/helpers/LoginHelper.php M languages/i18n/en.json M languages/i18n/qqq.json M languages/messages/MessagesEn.php 16 files changed, 129 insertions(+), 10 deletions(-) Approvals: Chad: Looks good to me, approved jenkins-bot: Verified diff --git a/RELEASE-NOTES-1.27 b/RELEASE-NOTES-1.27 index 3a496f2..3df89e0 100644 --- a/RELEASE-NOTES-1.27 +++ b/RELEASE-NOTES-1.27 @@ -23,6 +23,8 @@ * (T145635) Fix too long index error when installing with MSSQL. * (T156184) $wgRawHtml will no longer apply to internationalization messages. * (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. == MediaWiki 1.27.1 == diff --git a/autoload.php b/autoload.php index f36e613..dbba50d 100644 --- a/autoload.php +++ b/autoload.php @@ -1255,6 +1255,7 @@ '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', diff --git a/includes/OutputPage.php b/includes/OutputPage.php index 1985ab4..e5be2c7 100644 --- a/includes/OutputPage.php +++ b/includes/OutputPage.php @@ -2639,7 +2639,9 @@ } 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(); } diff --git a/includes/Title.php b/includes/Title.php index 589130d..6ba53d6 100644 --- a/includes/Title.php +++ b/includes/Title.php @@ -1682,6 +1682,33 @@ } /** + * 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, * $wgServer is prepended to make an absolute URL. diff --git a/includes/specialpage/RedirectSpecialPage.php b/includes/specialpage/RedirectSpecialPage.php index ea7d783..01787d3 100644 --- a/includes/specialpage/RedirectSpecialPage.php +++ b/includes/specialpage/RedirectSpecialPage.php @@ -41,7 +41,7 @@ $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; diff --git a/includes/specialpage/SpecialPageFactory.php b/includes/specialpage/SpecialPageFactory.php index 4f67da1..f0d7e52 100644 --- a/includes/specialpage/SpecialPageFactory.php +++ b/includes/specialpage/SpecialPageFactory.php @@ -142,6 +142,7 @@ 'RandomInCategory' => 'SpecialRandomInCategory', 'Randomredirect' => 'SpecialRandomredirect', 'Randomrootpage' => 'SpecialRandomrootpage', + 'GoToInterwiki' => 'SpecialGoToInterwiki', // High use pages 'Mostlinkedcategories' => 'MostlinkedCategoriesPage', diff --git a/includes/specials/SpecialChangeCredentials.php b/includes/specials/SpecialChangeCredentials.php index 9bf25ae..b2ffdc6 100644 --- a/includes/specials/SpecialChangeCredentials.php +++ b/includes/specials/SpecialChangeCredentials.php @@ -243,7 +243,7 @@ } $title = Title::newFromText( $returnTo ); - return $title->getFullURL( $returnToQuery ); + return $title->getFullUrlForRedirect( $returnToQuery ); } protected function getRequestBlacklist() { diff --git a/includes/specials/SpecialChangeEmail.php b/includes/specials/SpecialChangeEmail.php index 785447f..eb98fe7 100644 --- a/includes/specials/SpecialChangeEmail.php +++ b/includes/specials/SpecialChangeEmail.php @@ -136,7 +136,7 @@ $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>", diff --git a/includes/specials/SpecialGoToInterwiki.php b/includes/specials/SpecialGoToInterwiki.php new file mode 100644 index 0000000..809a14a --- /dev/null +++ b/includes/specials/SpecialGoToInterwiki.php @@ -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'; + } +} diff --git a/includes/specials/SpecialPageLanguage.php b/includes/specials/SpecialPageLanguage.php index 5322a04..0035829 100644 --- a/includes/specials/SpecialPageLanguage.php +++ b/includes/specials/SpecialPageLanguage.php @@ -141,7 +141,7 @@ ); // Url to redirect to after the operation - $this->goToUrl = $title->getFullURL(); + $this->goToUrl = $title->getFullUrlForRedirect(); // Check if user wants to use default language if ( $data['selectoptions'] == 1 ) { diff --git a/includes/specials/SpecialPreferences.php b/includes/specials/SpecialPreferences.php index 2e7b4cd..2435b08 100644 --- a/includes/specials/SpecialPreferences.php +++ b/includes/specials/SpecialPreferences.php @@ -147,7 +147,7 @@ // Set session data for the success message $this->getRequest()->setSessionData( 'specialPreferencesSaveSuccess', 1 ); - $url = $this->getPageTitle()->getFullURL(); + $url = $this->getPageTitle()->getFullUrlForRedirect(); $this->getOutput()->redirect( $url ); return true; diff --git a/includes/specials/SpecialSearch.php b/includes/specials/SpecialSearch.php index d474ba5..9ba948c 100644 --- a/includes/specials/SpecialSearch.php +++ b/includes/specials/SpecialSearch.php @@ -218,7 +218,7 @@ Hooks::run( 'SpecialSearchGoResult', [ $term, $title, &$url ] ) ) { if ( $url === null ) { - $url = $title->getFullURL(); + $url = $title->getFullUrlForRedirect(); } $this->getOutput()->redirect( $url ); diff --git a/includes/specials/helpers/LoginHelper.php b/includes/specials/helpers/LoginHelper.php index f853f41..cfcbf65 100644 --- a/includes/specials/helpers/LoginHelper.php +++ b/includes/specials/helpers/LoginHelper.php @@ -89,7 +89,7 @@ } 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 ); diff --git a/languages/i18n/en.json b/languages/i18n/en.json index 3203a67..91e0cda 100644 --- a/languages/i18n/en.json +++ b/languages/i18n/en.json @@ -4170,5 +4170,8 @@ "unlinkaccounts": "Unlink accounts", "unlinkaccounts-success": "The account was unlinked.", "authenticationdatachange-ignored": "The authentication data change was not handled. Maybe no provider was configured?", - "rawhtml-notallowed": "<html> tags cannot be used outside of normal pages." + "rawhtml-notallowed": "<html> 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]." } diff --git a/languages/i18n/qqq.json b/languages/i18n/qqq.json index c581d46..70497ed 100644 --- a/languages/i18n/qqq.json +++ b/languages/i18n/qqq.json @@ -4348,5 +4348,8 @@ "unlinkaccounts": "Title of the special page [[Special:UnlinkAccounts]] which allows the user to remove linked remote accounts.", "unlinkaccounts-success": "Account unlinking form success message", "authenticationdatachange-ignored": "Shown when authentication data change was unsuccessful due to configuration problems.", - "rawhtml-notallowed": "Error message given when $wgRawHtml = true; is set and a user uses an <html> 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 <html> 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\")." } diff --git a/languages/messages/MessagesEn.php b/languages/messages/MessagesEn.php index 589144c..6ae2db8 100644 --- a/languages/messages/MessagesEn.php +++ b/languages/messages/MessagesEn.php @@ -426,6 +426,7 @@ 'Fewestrevisions' => [ 'FewestRevisions' ], 'FileDuplicateSearch' => [ 'FileDuplicateSearch' ], 'Filepath' => [ 'FilePath' ], + 'GoToInterwiki' => [ 'GoToInterwiki' ], 'Import' => [ 'Import' ], 'Invalidateemail' => [ 'InvalidateEmail' ], 'JavaScriptTest' => [ 'JavaScriptTest' ], -- To view, visit https://gerrit.wikimedia.org/r/346848 To unsubscribe, visit https://gerrit.wikimedia.org/r/settings Gerrit-MessageType: merged Gerrit-Change-Id: I6c604439320fa876719933cc7f3a3ff04fb1a6ad Gerrit-PatchSet: 1 Gerrit-Project: mediawiki/core Gerrit-Branch: REL1_27 Gerrit-Owner: Chad <ch...@wikimedia.org> Gerrit-Reviewer: Brian Wolff <bawolff...@gmail.com> Gerrit-Reviewer: Chad <ch...@wikimedia.org> Gerrit-Reviewer: Florianschmidtwelzow <florian.schmidt.stargatewis...@gmail.com> Gerrit-Reviewer: Siebrand <siebr...@kitano.nl> Gerrit-Reviewer: TTO <at.li...@live.com.au> Gerrit-Reviewer: jenkins-bot <> _______________________________________________ MediaWiki-commits mailing list MediaWiki-commits@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits