From bf8cf47c2f82848c091b5430bef30177a0ab897b Mon Sep 17 00:00:00 2001 From: Brian Wolff Date: Sun, 7 Feb 2016 08:07:20 -0500 Subject: [PATCH 1/9] 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 --- RELEASE-NOTES-1.29 | 2 + autoload.php | 1 + includes/OutputPage.php | 4 +- includes/Title.php | 27 +++++++++ includes/specialpage/RedirectSpecialPage.php | 2 +- includes/specialpage/SpecialPageFactory.php | 1 + includes/specials/SpecialChangeCredentials.php | 2 +- includes/specials/SpecialChangeEmail.php | 2 +- includes/specials/SpecialGoToInterwiki.php | 79 ++++++++++++++++++++++++++ includes/specials/SpecialPageLanguage.php | 2 +- includes/specials/SpecialPreferences.php | 2 +- includes/specials/SpecialSearch.php | 2 +- includes/specials/helpers/LoginHelper.php | 2 +- languages/i18n/en.json | 5 +- languages/i18n/qqq.json | 5 +- languages/messages/MessagesEn.php | 1 + 16 files changed, 129 insertions(+), 10 deletions(-) create mode 100644 includes/specials/SpecialGoToInterwiki.php diff --git a/RELEASE-NOTES-1.29 b/RELEASE-NOTES-1.29 index 2371ed3..8690f48 100644 --- a/RELEASE-NOTES-1.29 +++ b/RELEASE-NOTES-1.29 @@ -84,6 +84,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. +* (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, diff --git a/autoload.php b/autoload.php index 956d504..914387a 100644 --- a/autoload.php +++ b/autoload.php @@ -1339,6 +1339,7 @@ $wgAutoloadLocalClasses = [ '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 d3e1373..5380264 100644 --- a/includes/OutputPage.php +++ b/includes/OutputPage.php @@ -2727,7 +2727,9 @@ class OutputPage extends ContextSource { } 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 f16f0c5..f1cf81f 100644 --- a/includes/Title.php +++ b/includes/Title.php @@ -1684,6 +1684,33 @@ class Title implements LinkTarget { } /** + * 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 b1ddacf..9b5d5f4 100644 --- a/includes/specialpage/RedirectSpecialPage.php +++ b/includes/specialpage/RedirectSpecialPage.php @@ -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 ) { - $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 33e1cc3..ae010f6 100644 --- a/includes/specialpage/SpecialPageFactory.php +++ b/includes/specialpage/SpecialPageFactory.php @@ -144,6 +144,7 @@ class SpecialPageFactory { '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 47f8d2f..970a2e2 100644 --- a/includes/specials/SpecialChangeCredentials.php +++ b/includes/specials/SpecialChangeCredentials.php @@ -258,7 +258,7 @@ class SpecialChangeCredentials extends AuthManagerSpecialPage { } $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 @@ class SpecialChangeEmail extends FormSpecialPage { $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( "
\n$1\n
", 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 @@ +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 db05ebe..2943fd4 100644 --- a/includes/specials/SpecialPageLanguage.php +++ b/includes/specials/SpecialPageLanguage.php @@ -136,7 +136,7 @@ class SpecialPageLanguage extends FormSpecialPage { } // Url to redirect to after the operation - $this->goToUrl = $title->getFullURL( + $this->goToUrl = $title->getFullUrlForRedirect( $title->isRedirect() ? [ 'redirect' => 'no' ] : [] ); diff --git a/includes/specials/SpecialPreferences.php b/includes/specials/SpecialPreferences.php index eee5b64..40b50ea 100644 --- a/includes/specials/SpecialPreferences.php +++ b/includes/specials/SpecialPreferences.php @@ -148,7 +148,7 @@ class SpecialPreferences extends SpecialPage { // 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; diff --git a/includes/specials/SpecialSearch.php b/includes/specials/SpecialSearch.php index 9cc0048..139e4f7 100644 --- a/includes/specials/SpecialSearch.php +++ b/includes/specials/SpecialSearch.php @@ -259,7 +259,7 @@ class SpecialSearch extends SpecialPage { return null; } - return $url === null ? $title->getFullURL() : $url; + return $url === null ? $title->getFullUrlForRedirect() : $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 @@ class LoginHelper extends ContextSource { } 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 e87b6fb..0c320e5 100644 --- a/languages/i18n/en.json +++ b/languages/i18n/en.json @@ -4285,5 +4285,8 @@ "restrictionsfield-help": "One IP address or CIDR range per line. To enable everything, use:
0.0.0.0/0\n::/0
", "revid": "revision $1", "pageid": "page ID $1", - "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 f244431..e24226c 100644 --- a/languages/i18n/qqq.json +++ b/languages/i18n/qqq.json @@ -4472,5 +4472,8 @@ "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 <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 b9280ea..c52ec0e 100644 --- a/languages/messages/MessagesEn.php +++ b/languages/messages/MessagesEn.php @@ -427,6 +427,7 @@ $specialPageAliases = [ 'Fewestrevisions' => [ 'FewestRevisions' ], 'FileDuplicateSearch' => [ 'FileDuplicateSearch' ], 'Filepath' => [ 'FilePath' ], + 'GoToInterwiki' => [ 'GoToInterwiki' ], 'Import' => [ 'Import' ], 'Invalidateemail' => [ 'InvalidateEmail' ], 'JavaScriptTest' => [ 'JavaScriptTest' ], -- 2.9.3