From 4be85fc229bae7ba5e722840a6eefab12d731793 Mon Sep 17 00:00:00 2001 From: Brian Wolff Date: Sun, 7 Feb 2016 13:28:13 -0500 Subject: [PATCH] [SECURITY] [API BREAKING CHANGE] Require logout actions to have a token. Special:Userlogout now requires a token Api action=logout requires a csrf token and the request to be POSTed Bug: T25227 Change-Id: If758b2a9e7a7efb50caca8448ff74be30c195c6b --- RELEASE-NOTES-1.27 | 1 + includes/api/ApiLogout.php | 14 +++++++++++++- includes/skins/SkinTemplate.php | 14 +++++++------- includes/specials/SpecialUserlogout.php | 22 ++++++++++++++++++++++ languages/i18n/en.json | 4 +++- languages/i18n/qqq.json | 4 +++- 6 files changed, 49 insertions(+), 10 deletions(-) diff --git a/RELEASE-NOTES-1.27 b/RELEASE-NOTES-1.27 index 5b9b2b8..b9b1334 100644 --- a/RELEASE-NOTES-1.27 +++ b/RELEASE-NOTES-1.27 @@ -198,6 +198,7 @@ production. merely need to change the username and password used after setting up a bot password. * action=upload no longer understands statuskey, asyncdownload or leavemessage. +* (T25227) action=logout now requires to be posted and have a csrf token. === Action API internal changes in 1.27 === * ApiQueryORM removed. diff --git a/includes/api/ApiLogout.php b/includes/api/ApiLogout.php index b40f5a3..8bb7fc3 100644 --- a/includes/api/ApiLogout.php +++ b/includes/api/ApiLogout.php @@ -56,13 +56,25 @@ class ApiLogout extends ApiBase { return false; } + public function mustBePosted() { + return true; + } + + public function needsToken() { + return 'csrf'; + } + protected function getExamplesMessages() { return array( - 'action=logout' + 'action=logout&token=123ABC' => 'apihelp-logout-example-logout', ); } + protected function getWebUITokenSalt( Array $params ) { + return 'logoutToken'; + } + public function getHelpUrls() { return 'https://www.mediawiki.org/wiki/API:Logout'; } diff --git a/includes/skins/SkinTemplate.php b/includes/skins/SkinTemplate.php index 1328870..e012a01 100644 --- a/includes/skins/SkinTemplate.php +++ b/includes/skins/SkinTemplate.php @@ -568,16 +568,15 @@ class SkinTemplate extends Skin { $page = Title::newFromText( $request->getVal( 'title', '' ) ); } $page = $request->getVal( 'returnto', $page ); - $a = array(); + $returnto = array(); if ( strval( $page ) !== '' ) { - $a['returnto'] = $page; + $returnto['returnto'] = $page; $query = $request->getVal( 'returntoquery', $this->thisquery ); if ( $query != '' ) { - $a['returntoquery'] = $query; + $returnto['returntoquery'] = $query; } } - $returnto = wfArrayToCgi( $a ); if ( $this->loggedin ) { $personal_urls['userpage'] = array( 'text' => $this->username, @@ -635,9 +634,10 @@ class SkinTemplate extends Skin { $personal_urls['logout'] = array( 'text' => $this->msg( 'pt-userlogout' )->text(), 'href' => self::makeSpecialUrl( 'Userlogout', - // userlogout link must always contain an & character, otherwise we might not be able + // Note: userlogout link must always contain an & character, otherwise we might not be able // to detect a buggy precaching proxy (bug 17790) - $title->isSpecial( 'Preferences' ) ? 'noreturnto' : $returnto + ( $title->isSpecial( 'Preferences' ) ? array() : $returnto ) + + array( 'logoutToken' => $this->getUser()->getEditToken( 'logoutToken', $this->getRequest() ) ) ), 'active' => false ); @@ -656,7 +656,7 @@ class SkinTemplate extends Skin { ); $createaccount_url = array( 'text' => $this->msg( 'pt-createaccount' )->text(), - 'href' => self::makeSpecialUrl( 'Userlogin', "$returnto&type=signup" ), + 'href' => self::makeSpecialUrl( 'Userlogin', $returnto + array( "type" => "signup" ) ), 'active' => $title->isSpecial( 'Userlogin' ) && $is_signup, ); diff --git a/includes/specials/SpecialUserlogout.php b/includes/specials/SpecialUserlogout.php index 6e34690..e6bdc89 100644 --- a/includes/specials/SpecialUserlogout.php +++ b/includes/specials/SpecialUserlogout.php @@ -48,6 +48,28 @@ class SpecialUserlogout extends UnlistedSpecialPage { $this->setHeaders(); $this->outputHeader(); + $out = $this->getOutput(); + $user = $this->getUser(); + $request = $this->getRequest(); + + $logoutToken = $request->getVal( 'logoutToken' ); + $urlParams = array( + 'logoutToken' => $user->getEditToken( 'logoutToken', $request ) + ) + $request->getValues(); + unset( $urlParams['title'] ); + $continueLink = $this->getFullTitle()->getFullUrl( $urlParams ); + + if ( $logoutToken === null ) { + $this->getOutput()->addWikiMsg( 'userlogout-continue', $continueLink ); + return; + } + if ( !$this->getUser()->matchEditToken( + $logoutToken, 'logoutToken', $this->getRequest(), 24*60*60 + ) ) { + $this->getOutput()->addWikiMsg( 'userlogout-sessionerror', $continueLink ); + return; + } + // Make sure it's possible to log out $session = MediaWiki\Session\SessionManager::getGlobalSession(); if ( !$session->canSetUser() ) { diff --git a/languages/i18n/en.json b/languages/i18n/en.json index 2e1df41..e6fffd8 100644 --- a/languages/i18n/en.json +++ b/languages/i18n/en.json @@ -4033,5 +4033,7 @@ "sessionprovider-generic": "$1 sessions", "sessionprovider-mediawiki-session-cookiesessionprovider": "cookie-based sessions", "sessionprovider-nocookies": "Cookies may be disabled. Ensure you have cookies enabled and start again.", - "randomrootpage": "Random root page" + "randomrootpage": "Random root page", + "userlogout-continue": "If you wish to log out please [$1 continue to the log out page].", + "userlogout-sessionerror": "Log out failed due to session error. Please [$1 try again]." } diff --git a/languages/i18n/qqq.json b/languages/i18n/qqq.json index fec3079..1deb3ad 100644 --- a/languages/i18n/qqq.json +++ b/languages/i18n/qqq.json @@ -4208,5 +4208,7 @@ "sessionprovider-generic": "Used to create a generic session type description when one isn't provided via the proper message. Should be phrased to make sense when added to a message such as {{msg-mw|cannotloginnow-text}}.\n\nParameters:\n* $1 - PHP classname.", "sessionprovider-mediawiki-session-cookiesessionprovider": "Description of the sessions provided by the CookieSessionProvider class, which use HTTP cookies. Should be phrased to make sense when added to a message such as {{msg-mw|cannotloginnow-text}}.", "sessionprovider-nocookies": "Used to inform the user that sessions may be missing due to lack of cookies.", - "randomrootpage": "{{doc-special|RandomRootPage}}" + "randomrootpage": "{{doc-special|RandomRootPage}}", + "userlogout-continue": "Shown if user attempted to log out without a token specified. Probably the user clicked on an old link that hasn't been updated to use the new system. $1 - url that user should click on in order to log out.", + "userlogout-sessionerror": "Shown when a user tries to log out with an invalid token. $1 is url with correct token that user should click." } -- 2.0.1