Wikimedia Bugzilla is closed!

Created attachment 10316 [details]
Fix for email confirmation token expiration setting in 1.17.3

The recent patches break email confirmation in 1.17.3 and likely break or degrade it on other versions. Here is the result of attempting to set email in 1.17.3:

Internal error
Non-string key given
Backtrace:
#0 [....]/w/includes/GlobalFunctions.php(781): MessageCache->get(NULL, true, Object(Language))
#1 [....]/w/includes/GlobalFunctions.php(902): wfMsgGetKey(NULL, true, Object(Language), false)
#2 [....]/w/languages/Language.php(513): wfMsgExt(NULL, Array)
#3 [....]/w/languages/Language.php(525): Language->getMessageFromDB(NULL)
#4 [....]/w/languages/Language.php(799): Language->getMonthName(false)
#5 [....]/w/languages/Language.php(1560): Language->sprintfDate('H:i, j F Y', false)
#6 [....]/w/includes/User.php(2966): Language->timeanddate('f4e8c51bf5ef375...', false)
#7 [....]/w/includes/specials/SpecialConfirmemail.php(84): User->sendConfirmationMail()
#8 [....]/w/includes/specials/SpecialConfirmemail.php(58): EmailConfirmation->showRequestForm()

This is because assignment of $expiration incorrectly falls through to the assignment of $token, which is not a valid date:

 function confirmationToken( &$expiration ) {
                $now = time();
                $expires = $now + 7 * 24 * 60 * 60;
                $expiration = 
                $token = MWCryptRand::generateHex( 32 );
                $hash = md5( $token );
                $this->load();
                $this->mEmailToken = $hash;
                $this->mEmailTokenExpires = wfTimestamp( TS_MW, $expires );
                return $token;
        }

The relevant section of the 1.17.3 patch is:

@@ -3005,12 +3009,12 @@
 	function confirmationToken( &$expiration ) {
 		$now = time();
 		$expires = $now + 7 * 24 * 60 * 60;
-		$expiration = wfTimestamp( TS_MW, $expires );
-		$token = self::generateToken( $this->mId . $this->mEmail . $expires );
+		$expiration = 
+		$token = MWCryptRand::generateHex( 32 );
 		$hash = md5( $token );
 		$this->load();
 		$this->mEmailToken = $hash;
-		$this->mEmailTokenExpires = $expiration;
+		$this->mEmailTokenExpires = wfTimestamp( TS_MW, $expires );
 		return $token;
 	}
 

It appears that this is an attempt to incorrectly optimize the setting of $expiration. This is backed up by the patch in 1.18.2, which removes access to the variable from the function:
@@ -3268,12 +3272,11 @@
 		global $wgUserEmailConfirmationTokenExpiry;
 		$now = time();
 		$expires = $now + $wgUserEmailConfirmationTokenExpiry;
-		$expiration = wfTimestamp( TS_MW, $expires );
-		$token = self::generateToken( $this->mId . $this->mEmail . $expires );
-		$hash = md5( $token );
 		$this->load();
+		$token = MWCryptRand::generateHex( 32 );
+		$hash = md5( $token );
 		$this->mEmailToken = $hash;
-		$this->mEmailTokenExpires = $expiration;
+		$this->mEmailTokenExpires = wfTimestamp( TS_MW, $expires );
 		return $token;
 	}
 

I have attached a patch to 1.17.3 that replaces $expiration and uses it to set $this->mEmailTokenExpires again. (It will be offset by a few lines on the branch due to other local patches.)

I do not have later versions around right now, but $expiration is likely still used by the parent function to display the expiry time in the email address, and so this should be fixed in 1.18, 1.19 and SVN. It might break the sending of the email, depending on the impact of $wgLang->{timeanddate|date|time}(null, false).