From dd2e847d7c5150672b063d2d16956373bc659e0b Mon Sep 17 00:00:00 2001
From: Platonides <platonides@users.mediawiki.org>
Date: Sat, 1 May 2010 20:16:13 +0000
Subject: [PATCH] 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.
---
 includes/specials/SpecialUserlogin.php | 93 +++++++++++++++++++++++---
 includes/templates/Userlogin.php       |  1 +
 2 files changed, 83 insertions(+), 11 deletions(-)

diff --git a/includes/specials/SpecialUserlogin.php b/includes/specials/SpecialUserlogin.php
index bfc6c26962..8b8d0e9edd 100644
--- 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 0b5a37b835..60f3376782 100644
--- 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>
-- 
2.20.1