From 7bb661a0d216f4dad2dc44da5d55450e1cb12464 Mon Sep 17 00:00:00 2001 From: Tim Starling Date: Wed, 7 Apr 2010 00:05:33 +0000 Subject: [PATCH] * (bug 23076) Fixed login CSRF vulnerability. Logins now require a token to be submitted along with the user name and password. Patch by Roan Kattouw. --- includes/User.php | 6 +-- includes/api/ApiLogin.php | 18 +++++++- includes/specials/SpecialUserlogin.php | 63 +++++++++++++++++++++++++- includes/templates/Userlogin.php | 1 + 4 files changed, 81 insertions(+), 7 deletions(-) diff --git a/includes/User.php b/includes/User.php index 475f976e40..ffbce8151a 100644 --- a/includes/User.php +++ b/includes/User.php @@ -2850,7 +2850,7 @@ class User { return EDIT_TOKEN_SUFFIX; } else { if( !isset( $_SESSION['wsEditToken'] ) ) { - $token = $this->generateToken(); + $token = self::generateToken(); $_SESSION['wsEditToken'] = $token; } else { $token = $_SESSION['wsEditToken']; @@ -2868,7 +2868,7 @@ class User { * @param $salt \string Optional salt value * @return \string The new random token */ - function generateToken( $salt = '' ) { + public static function generateToken( $salt = '' ) { $token = dechex( mt_rand() ) . dechex( mt_rand() ); return md5( $token . $salt ); } @@ -2967,7 +2967,7 @@ class User { $now = time(); $expires = $now + 7 * 24 * 60 * 60; $expiration = wfTimestamp( TS_MW, $expires ); - $token = $this->generateToken( $this->mId . $this->mEmail . $expires ); + $token = self::generateToken( $this->mId . $this->mEmail . $expires ); $hash = md5( $token ); $this->load(); $this->mEmailToken = $hash; diff --git a/includes/api/ApiLogin.php b/includes/api/ApiLogin.php index 903b180802..8fb4604d77 100644 --- a/includes/api/ApiLogin.php +++ b/includes/api/ApiLogin.php @@ -58,6 +58,7 @@ class ApiLogin extends ApiBase { 'wpName' => $params['name'], 'wpPassword' => $params['password'], 'wpDomain' => $params['domain'], + 'wpLoginToken' => $params['token'], 'wpRemember' => '' ) ); @@ -86,6 +87,15 @@ class ApiLogin extends ApiBase { $result['cookieprefix'] = $wgCookiePrefix; $result['sessionid'] = session_id(); break; + + case LoginForm::NEED_TOKEN: + $result['result'] = 'NeedToken'; + $result['token'] = $loginForm->getLoginToken(); + break; + + case LoginForm::WRONG_TOKEN: + $result['result'] = 'WrongToken'; + break; case LoginForm::NO_NAME: $result['result'] = 'NoName'; @@ -146,7 +156,8 @@ class ApiLogin extends ApiBase { return array( 'name' => null, 'password' => null, - 'domain' => null + 'domain' => null, + 'token' => null, ); } @@ -154,7 +165,8 @@ class ApiLogin extends ApiBase { return array( 'name' => 'User Name', 'password' => 'Password', - 'domain' => 'Domain (optional)' + 'domain' => 'Domain (optional)', + 'token' => 'Login token obtained in first request', ); } @@ -170,6 +182,8 @@ class ApiLogin extends ApiBase { public function getPossibleErrors() { return array_merge( parent::getPossibleErrors(), array( + array( 'code' => 'NeedToken', 'info' => 'You need to resubmit your login with the specified token. See https://bugzilla.wikimedia.org/show_bug.cgi?id=23076' ), + array( 'code' => 'WrongToken', 'info' => 'You specified an invalid token' ), array( 'code' => 'NoName', 'info' => 'You didn\'t set the lgname parameter' ), array( 'code' => 'Illegal', 'info' => ' You provided an illegal username' ), array( 'code' => 'NotExists', 'info' => ' The username you provided doesn\'t exist' ), diff --git a/includes/specials/SpecialUserlogin.php b/includes/specials/SpecialUserlogin.php index 5d5a771046..fe9e5cd176 100644 --- a/includes/specials/SpecialUserlogin.php +++ b/includes/specials/SpecialUserlogin.php @@ -35,11 +35,13 @@ class LoginForm { const CREATE_BLOCKED = 9; const THROTTLED = 10; const USER_BLOCKED = 11; + const NEED_TOKEN = 12; + const WRONG_TOKEN = 13; var $mName, $mPassword, $mRetype, $mReturnTo, $mCookieCheck, $mPosted; var $mAction, $mCreateaccount, $mCreateaccountMail, $mMailmypassword; var $mLoginattempt, $mRemember, $mEmail, $mDomain, $mLanguage; - var $mSkipCookieCheck, $mReturnToQuery; + var $mSkipCookieCheck, $mReturnToQuery, $mToken; private $mExtUser = null; @@ -70,6 +72,7 @@ class LoginForm { $this->mRemember = $request->getCheck( 'wpRemember' ); $this->mLanguage = $request->getText( 'uselang' ); $this->mSkipCookieCheck = $request->getCheck( 'wpSkipCookieCheck' ); + $this->mToken = $request->getVal( 'wpLoginToken' ); if ( $wgRedirectOnLogin ) { $this->mReturnTo = $wgRedirectOnLogin; @@ -395,6 +398,21 @@ class LoginForm { return self::NO_NAME; } + // We require a login token to prevent login CSRF + // Handle part of this before incrementing the throttle so + // token-less login attempts don't count towards the throttle + // but wrong-token attempts do. + + // If the user doesn't have a login token yet, set one. + if ( !self::getLoginToken() ) { + self::setLoginToken(); + return self::NEED_TOKEN; + } + // If the user didn't pass a login token, tell them we need one + if ( !$this->mToken ) { + return self::NEED_TOKEN; + } + global $wgPasswordAttemptThrottle; $throttleCount = 0; @@ -413,6 +431,11 @@ class LoginForm { return self::THROTTLED; } } + + // Validate the login token + if ( $this->mToken !== self::getLoginToken() ) { + return self::WRONG_TOKEN; + } // Load $wgUser now, and check to see if we're logging in as the same // name. This is necessary because loading $wgUser (say by calling @@ -575,6 +598,7 @@ class LoginForm { $wgUser->invalidateCache(); } $wgUser->setCookies(); + self::clearLoginToken(); // Reset the throttle $key = wfMemcKey( 'password-throttle', wfGetIP(), md5( $this->mName ) ); @@ -593,7 +617,11 @@ class LoginForm { return $this->cookieRedirectCheck( 'login' ); } break; - + + case self::NEED_TOKEN: + case self::WRONG_TOKEN: + $this->mainLoginForm( wfMsg( 'sessionfailure' ) ); + break; case self::NO_NAME: case self::ILLEGAL: $this->mainLoginForm( wfMsg( 'noname' ) ); @@ -937,6 +965,11 @@ class LoginForm { $template->set( 'canremember', ( $wgCookieExpiration > 0 ) ); $template->set( 'remember', $wgUser->getOption( 'rememberpassword' ) or $this->mRemember ); + if ( !self::getLoginToken() ) { + self::setLoginToken(); + } + $template->set( 'token', self::getLoginToken() ); + # Prepare language selection links as needed if( $wgLoginLanguageSelector ) { $template->set( 'languages', $this->makeLanguageSelector() ); @@ -991,6 +1024,32 @@ class LoginForm { global $wgDisableCookieCheck, $wgRequest; return $wgDisableCookieCheck ? true : $wgRequest->checkSessionCookie(); } + + /** + * Get the login token from the current session + */ + public static function getLoginToken() { + global $wgRequest; + return $wgRequest->getSessionData( 'wsLoginToken' ); + } + + /** + * Generate a new login token and attach it to the current session + */ + public static function setLoginToken() { + global $wgRequest; + // Use User::generateToken() instead of $user->editToken() + // because the latter reuses $_SESSION['wsEditToken'] + $wgRequest->setSessionData( 'wsLoginToken', User::generateToken() ); + } + + /** + * Remove any login token attached to the current session + */ + public static function clearLoginToken() { + global $wgRequest; + $wgRequest->setSessionData( 'wsLoginToken', null ); + } /** * @private diff --git a/includes/templates/Userlogin.php b/includes/templates/Userlogin.php index f5571cf790..0b5a37b835 100644 --- a/includes/templates/Userlogin.php +++ b/includes/templates/Userlogin.php @@ -111,6 +111,7 @@ class UserloginTemplate extends QuickTemplate { haveData( 'uselang' ) ) { ?> +haveData( 'token' ) ) { ?>
msgWiki( 'loginend' ); ?>
-- 2.20.1