From 47460043a7a8a87a11c5b58a5ac424aaace9492d Mon Sep 17 00:00:00 2001 From: Reedy Date: Sun, 19 Mar 2017 21:26:26 +0000 Subject: [PATCH] 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 --- includes/AutoLoader.php | 1 + includes/OutputPage.php | 4 +- includes/Title.php | 27 ++++++++++ includes/specialpage/RedirectSpecialPage.php | 2 +- includes/specialpage/SpecialPageFactory.php | 1 + includes/specials/SpecialChangeEmail.php | 2 +- includes/specials/SpecialChangePassword.php | 2 +- includes/specials/SpecialGoToInterwiki.php | 79 ++++++++++++++++++++++++++++ includes/specials/SpecialPreferences.php | 2 +- includes/specials/SpecialUserlogin.php | 2 +- languages/i18n/en.json | 5 +- languages/i18n/qqq.json | 6 ++- languages/messages/MessagesEn.php | 1 + 13 files changed, 126 insertions(+), 8 deletions(-) create mode 100644 includes/specials/SpecialGoToInterwiki.php diff --git a/includes/AutoLoader.php b/includes/AutoLoader.php index 573726c..df75e85 100644 --- a/includes/AutoLoader.php +++ b/includes/AutoLoader.php @@ -1006,6 +1006,7 @@ $wgAutoloadLocalClasses = array( 'SpecialExpandTemplates' => 'includes/specials/SpecialExpandTemplates.php', 'SpecialExport' => 'includes/specials/SpecialExport.php', 'SpecialFilepath' => 'includes/specials/SpecialFilepath.php', + 'SpecialGoToInterwiki' => 'includes/specials/SpecialGoToInterwiki.php', 'SpecialImport' => 'includes/specials/SpecialImport.php', 'SpecialJavaScriptTest' => 'includes/specials/SpecialJavaScriptTest.php', 'SpecialListAdmins' => 'includes/specials/SpecialListusers.php', diff --git a/includes/OutputPage.php b/includes/OutputPage.php index b3e724a..7ba4be4 100644 --- a/includes/OutputPage.php +++ b/includes/OutputPage.php @@ -2535,7 +2535,9 @@ $templates } 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 a54156f..35a2883 100644 --- a/includes/Title.php +++ b/includes/Title.php @@ -1614,6 +1614,33 @@ class Title { } /** + * 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 939fed9..4e43e2d 100644 --- a/includes/specialpage/RedirectSpecialPage.php +++ b/includes/specialpage/RedirectSpecialPage.php @@ -38,7 +38,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 15a71ff..87b6903 100644 --- a/includes/specialpage/SpecialPageFactory.php +++ b/includes/specialpage/SpecialPageFactory.php @@ -132,6 +132,7 @@ class SpecialPageFactory { 'Randompage' => 'RandomPage', 'RandomInCategory' => 'SpecialRandomInCategory', 'Randomredirect' => 'SpecialRandomredirect', + 'GoToInterwiki' => 'SpecialGoToInterwiki', // High use pages 'Mostlinkedcategories' => 'MostlinkedCategoriesPage', diff --git a/includes/specials/SpecialChangeEmail.php b/includes/specials/SpecialChangeEmail.php index e678259..c47f31a 100644 --- a/includes/specials/SpecialChangeEmail.php +++ b/includes/specials/SpecialChangeEmail.php @@ -122,7 +122,7 @@ class SpecialChangeEmail extends UnlistedSpecialPage { $titleObj = Title::newMainPage(); } if ( $type == 'hard' ) { - $this->getOutput()->redirect( $titleObj->getFullURL() ); + $this->getOutput()->redirect( $titleObj->getFullUrlForRedirect() ); } else { $this->getOutput()->addReturnTo( $titleObj ); } diff --git a/includes/specials/SpecialChangePassword.php b/includes/specials/SpecialChangePassword.php index 91d0404..c11136b 100644 --- a/includes/specials/SpecialChangePassword.php +++ b/includes/specials/SpecialChangePassword.php @@ -185,7 +185,7 @@ class SpecialChangePassword extends FormSpecialPage { $titleObj = Title::newMainPage(); } $query = $request->getVal( 'returntoquery' ); - $this->getOutput()->redirect( $titleObj->getFullURL( $query ) ); + $this->getOutput()->redirect( $titleObj->getFullUrlForRedirect( $query ) ); return true; } 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/SpecialPreferences.php b/includes/specials/SpecialPreferences.php index 4cfd445..659d265 100644 --- a/includes/specials/SpecialPreferences.php +++ b/includes/specials/SpecialPreferences.php @@ -88,7 +88,7 @@ class SpecialPreferences extends SpecialPage { $user->resetOptions( 'all', $this->getContext() ); $user->saveSettings(); - $url = $this->getPageTitle()->getFullURL( 'success' ); + $url = $this->getPageTitle()->getFullUrlForRedirect( 'success' ); $this->getOutput()->redirect( $url ); diff --git a/includes/specials/SpecialUserlogin.php b/includes/specials/SpecialUserlogin.php index 6ca7b7a..09b48fa 100644 --- a/includes/specials/SpecialUserlogin.php +++ b/includes/specials/SpecialUserlogin.php @@ -1150,7 +1150,7 @@ class LoginForm extends SpecialPage { } if ( $type == 'successredirect' ) { - $redirectUrl = $returnToTitle->getFullURL( $returnToQuery, false, $proto ); + $redirectUrl = $returnToTitle->getFullUrlForRedirect( $returnToQuery, false, $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 8b674fa..f25b881 100644 --- a/languages/i18n/en.json +++ b/languages/i18n/en.json @@ -3535,5 +3535,8 @@ "expand_templates_generate_rawhtml": "Show raw HTML", "expand_templates_preview": "Preview", "expand_templates_preview_fail_html": "Because {{SITENAME}} has raw HTML enabled and there was a loss of session data, the preview is hidden as a precaution against JavaScript attacks.\n\nIf this is a legitimate preview attempt, please try again.\nIf it still does not work, try [[Special:UserLogout|logging out]] and logging back in.", - "expand_templates_preview_fail_html_anon": "Because {{SITENAME}} has raw HTML enabled and you are not logged in, the preview is hidden as a precaution against JavaScript attacks.\n\nIf this is a legitimate preview attempt, please [[Special:UserLogin|log in]] and try again." + "expand_templates_preview_fail_html_anon": "Because {{SITENAME}} has raw HTML enabled and you are not logged in, the preview is hidden as a precaution against JavaScript attacks.\n\nIf this is a legitimate preview attempt, please [[Special:UserLogin|log in]] and try again.", + "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 4d8fb9b..a75a1c7 100644 --- a/languages/i18n/qqq.json +++ b/languages/i18n/qqq.json @@ -3699,5 +3699,9 @@ n* $1 - the action specified in the url.", "expand_templates_generate_rawhtml": "Used as checkbox label.", "expand_templates_preview": "{{Identical|Preview}}", "expand_templates_preview_fail_html": "Used as error message in Preview section of [[Special:ExpandTemplates]] page.", - "expand_templates_preview_fail_html_anon": "Used as error message in Preview section of [[Special:ExpandTemplates]] page." + "expand_templates_preview_fail_html_anon": "Used as error message in Preview section of [[Special:ExpandTemplates]] 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 6d40d8a..c6ad950 100644 --- a/languages/messages/MessagesEn.php +++ b/languages/messages/MessagesEn.php @@ -413,6 +413,7 @@ $specialPageAliases = array( 'Fewestrevisions' => array( 'FewestRevisions' ), 'FileDuplicateSearch' => array( 'FileDuplicateSearch' ), 'Filepath' => array( 'FilePath' ), + 'GoToInterwiki' => array( 'GoToInterwiki' ), 'Import' => array( 'Import' ), 'Invalidateemail' => array( 'InvalidateEmail' ), 'JavaScriptTest' => array( 'JavaScriptTest' ), -- 2.9.3