* (bug 23076) Fixed login CSRF vulnerability. Logins now require a token to be submit...

diff --git a/includes/User.php b/includes/User.php
index 475f976..ffbce81 100644 (file)
--- 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 903b180..8fb4604 100644 (file)
--- 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 5d5a771..fe9e5cd 100644 (file)
--- 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 f5571cf..0b5a37b 100644 (file)
--- a/includes/templates/Userlogin.php
+++ b/includes/templates/Userlogin.php
@@ -111,6 +111,7 @@ class UserloginTemplate extends QuickTemplate {
                </tr>
        </table>
 <?php if( @$this->haveData( 'uselang' ) ) { ?><input type="hidden" name="uselang" value="<?php $this->text( 'uselang' ); ?>" /><?php } ?>
+<?php if( @$this->haveData( 'token' ) ) { ?><input type="hidden" name="wpLoginToken" value="<?php $this->text( 'token' ); ?>" /><?php } ?>
 </form>
 </div>
 <div id="loginend"><?php $this->msgWiki( 'loginend' ); ?></div>