Bug 23371: Fix CSRF similar to r64677 covering the other three execute()

branches. Checks added to mailPassword() and addNewAccountInternal()
(covers addNewAccount & addNewAccountMailPassword).
Paranoia: Use different tokens for login and account creation.

*For wikis allowing public account creation, an attacker could create
many accounts via proxying users, avoiding ip blocks, the anon gets
logged in (wikis using ConfirmEdit to request a captcha for createaccount
are protected from this).

*If the victims were logged users, the attacker could create the
accounts by email and flood innocent parties using the wiki as gateway.

*If the victim was a sysop, the attacker could not only bypass the
captcha protection, but also the username blacklist.

*It also provides a way to bypass the blocks and ping limit for sending
many password resets flooding its targets.

*On private wikis an account creation by targeting a sysop may expose
confidential information.

diff --git a/includes/specials/SpecialUserlogin.php b/includes/specials/SpecialUserlogin.php
index bfc6c26..8b8d0e9 100644 (file)
--- a/includes/specials/SpecialUserlogin.php
+++ b/includes/specials/SpecialUserlogin.php
@@ -72,7 +72,7 @@ class LoginForm {
                $this->mRemember = $request->getCheck( 'wpRemember' );
                $this->mLanguage = $request->getText( 'uselang' );
                $this->mSkipCookieCheck = $request->getCheck( 'wpSkipCookieCheck' );
-               $this->mToken = $request->getVal( 'wpLoginToken' );
+               $this->mToken = ($this->mType == 'signup' ) ? $request->getVal( 'wpCreateaccountToken' ) : $request->getVal( 'wpLoginToken' );
 
                if ( $wgRedirectOnLogin ) {
                        $this->mReturnTo = $wgRedirectOnLogin;
@@ -251,6 +251,25 @@ class LoginForm {
                        return false;
                }
 
+               # Request forgery checks.
+               if ( !self::getCreateaccountToken() ) {
+                       self::setCreateaccountToken();
+                       $this->mainLoginForm( wfMsg( 'sessionfailure' ) );
+                       return false;
+               }
+               
+               # The user didn't pass a createaccount token
+               if ( !$this->mToken ) {
+                       $this->mainLoginForm( wfMsg( 'sessionfailure' ) );
+                       return false;
+               }
+               
+               # Validate the createaccount token
+               if ( $this->mToken !== self::getCreateaccountToken() ) {
+                       $this->mainLoginForm( wfMsg( 'sessionfailure' ) );
+                       return false;
+               }
+
                # Check permissions
                if ( !$wgUser->isAllowed( 'createaccount' ) ) {
                        $this->userNotPrivilegedMessage();
@@ -263,7 +282,7 @@ class LoginForm {
                $ip = wfGetIP();
                if ( $wgUser->isDnsBlacklisted( $ip, true /* check $wgProxyWhitelist */ ) ) {
                        $this->mainLoginForm( wfMsg( 'sorbs_create_account_reason' ) . ' (' . htmlspecialchars( $ip ) . ')' );
-                       return;
+                       return false;
                }
 
                # Now create a dummy user ($u) and check if it is valid
@@ -340,6 +359,7 @@ class LoginForm {
                        return false;
                }
 
+               self::clearCreateaccountToken();                
                return $this->initUser( $u, false );
        }
 
@@ -683,20 +703,33 @@ class LoginForm {
                        return;
                }
 
-               # Check against blocked IPs
-               # fixme -- should we not?
+               # Check against blocked IPs so blocked users can't flood admins 
+               # with password resets
                if( $wgUser->isBlocked() ) {
                        $this->mainLoginForm( wfMsg( 'blocked-mailpassword' ) );
                        return;
                }
                
-               // Check for hooks
+               # Check for hooks
                $error = null;
                if ( ! wfRunHooks( 'UserLoginMailPassword', array( $this->mName, &$error ) ) ) {
                        $this->mainLoginForm( $error );
                        return;
                }
 
+               # If the user doesn't have a login token yet, set one.
+               if ( !self::getLoginToken() ) {
+                       self::setLoginToken();
+                       $this->mainLoginForm( wfMsg( 'sessionfailure' ) );
+                       return;
+               }
+
+               # If the user didn't pass a login token, tell them we need one
+               if ( !$this->mToken ) {
+                       $this->mainLoginForm( wfMsg( 'sessionfailure' ) );
+                       return;
+               }
+               
                # Check against the rate limiter
                if( $wgUser->pingLimiter( 'mailpassword' ) ) {
                        $wgOut->rateLimited();
@@ -717,6 +750,12 @@ class LoginForm {
                        return;
                }
 
+               # Validate the login token
+               if ( $this->mToken !== self::getLoginToken() ) {
+                       $this->mainLoginForm( wfMsg( 'sessionfailure' ) );
+                       return;
+               }
+
                # Check against password throttle
                if ( $u->isPasswordReminderThrottled() ) {
                        global $wgPasswordReminderResendTime;
@@ -732,6 +771,7 @@ class LoginForm {
                        $this->mainLoginForm( wfMsg( 'mailerror', $result->getMessage() ) );
                } else {
                        $this->mainLoginForm( wfMsg( 'passwordsent', $u->getName() ), 'success' );
+                       self::clearLoginToken();
                }
        }
 
@@ -965,11 +1005,18 @@ class LoginForm {
                $template->set( 'canremember', ( $wgCookieExpiration > 0 ) );
                $template->set( 'remember', $wgUser->getOption( 'rememberpassword' ) or $this->mRemember  );
 
-               if ( !self::getLoginToken() ) {
-                       self::setLoginToken();
+               if ( $this->mType == 'signup' ) {
+                       if ( !self::getCreateaccountToken() ) {
+                               self::setCreateaccountToken();
+                       }
+                       $template->set( 'token', self::getCreateaccountToken() );
+               } else {
+                       if ( !self::getLoginToken() ) {
+                               self::setLoginToken();
+                       }
+                       $template->set( 'token', self::getLoginToken() );
                }
-               $template->set( 'token', self::getLoginToken() );
-
+               
                # Prepare language selection links as needed
                if( $wgLoginLanguageSelector ) {
                        $template->set( 'languages', $this->makeLanguageSelector() );
@@ -1034,7 +1081,7 @@ class LoginForm {
        }
        
        /**
-        * Generate a new login token and attach it to the current session
+        * Randomly generate a new login token and attach it to the current session
         */
        public static function setLoginToken() {
                global $wgRequest;
@@ -1046,11 +1093,35 @@ class LoginForm {
        /**
         * Remove any login token attached to the current session
         */
-       public static  function clearLoginToken() {
+       public static function clearLoginToken() {
                global $wgRequest;
                $wgRequest->setSessionData( 'wsLoginToken', null );
        }
 
+       /**
+        * Get the createaccount token from the current session
+        */
+       public static function getCreateaccountToken() {
+               global $wgRequest;
+               return $wgRequest->getSessionData( 'wsCreateaccountToken' );
+       }
+       
+       /**
+        * Randomly generate a new createaccount token and attach it to the current session
+        */
+       public static function setCreateaccountToken() {
+               global $wgRequest;
+               $wgRequest->setSessionData( 'wsCreateaccountToken', User::generateToken() );
+       }
+       
+       /**
+        * Remove any createaccount token attached to the current session
+        */
+       public static function clearCreateaccountToken() {
+               global $wgRequest;
+               $wgRequest->setSessionData( 'wsCreateaccountToken', null );
+       }
+
        /**
         * @private
         */

diff --git a/includes/templates/Userlogin.php b/includes/templates/Userlogin.php
index 0b5a37b..60f3376 100644 (file)
--- a/includes/templates/Userlogin.php
+++ b/includes/templates/Userlogin.php
@@ -315,6 +315,7 @@ class UsercreateTemplate 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="wpCreateaccountToken" value="<?php $this->text( 'token' ); ?>" /><?php } ?>
 </form>
 </div>
 <div id="signupend"><?php $this->msgWiki( 'signupend' ); ?></div>