Wordpress 3.9.2, released August 6th, contained fixes for two closely related vulnerabilities (CVE-2014-5204) in the way it handles Wordpress nonces (CSRF Tokens, essentially) that I reported to the Wordpress Security Team. I’d like to say the delay in my publishing this write-up was to allow people time to patch, but the reality is I’ve just been busy and haven’t gotten around to this.

TL;DR: Wordpress < 3.9.2 generated nonces in a manner that would allow an attacker to generate valid nonces for other users for a small subset of possible actions. Additionally, nonces were compared with ==, leading to a timing attack against nonce comparison. (Although this is very difficult to execute.)

Review of CSRF Protection

A common technique for avoiding Cross Site Request Forgery (CSRF) is to have the server generate a token specific to the current user, include that in the page, and then have the client echo that token back with the request. This way the server can tell that the request was in response to a page from the server, rather than a request triggered on the user’s behalf by an attacker. OWASP calls this the Synchronizer Token Pattern and one of the requirements is that an attacker is not able to predict or determine tokens for another user.

Wordpress Nonces

Wordpress uses what they call “nonces” (but they’re not, in fact, guaranteed to be used only once) for CSRF protection. These nonces include a timestamp, a user identifier, and an action, all of which are part of best practices for CSRF tokens. These values are HMAC’d with a secret key to generate the final token. All of this is in accordance with best practices, and at first blush, the nonce generation code looks good. Here’s how nonces were generated prior to the 3.9.2 fix:

#!php
function wp_create_nonce($action = -1) {
	$user = wp_get_current_user();
	$uid = (int) $user->ID;
	# snipped

	$i = wp_nonce_tick();

	return substr(wp_hash($i . $action . $uid, 'nonce'), -12, 10);
}

wp_nonce_tick returns a monotonically increasing value that increments every 12 hours to provide a timeout on the resulting nonce. $user->ID is the auto-increment id column from the database. wp_hash performs an HMAC-MD5 using a key selected by the 2nd argument, the nonce key in this case. So, we’re esentially getting an HMAC of a string concatenation of the current time, the action value passed in, and the current user’s UID. Assuming HMAC is strong, we’ve got a user, action and time-specific token, right?

Wrong. What if we can figure out a way to collide inputs to the HMAC? Turns out this is pretty easy, actually. Let’s look at some instances where wp_create_nonce is used:

#!php
wp_create_nonce( "approve-comment_$comment->comment_ID" )
wp_create_nonce( 'set_post_thumbnail-' . $post->ID );
wp_create_nonce( 'update-post_' . $attachment->ID );

In more than one case, we see places where nonces are created that end in an ID value (an integer from the database). Note that these action values are immediately before the UID, also an integer. This means that once the concatenation is done, there is no separation between the integer values of the action and the UID, leading to collisions in the hash input, and consequently the same nonce value being generated. Take, for example, an installation where users are privileged to update their own post but not those of other users. Let’s take user 1 and post 32, and user 21 and post 3. What are the respective inputs to wp_hash? (I’m substituting 0 for the timestamp value as it’s the same for all users at the same time.)

$i . 'update-post_32' . 1 => '0update-post_321'
$i . 'update-post_3' . 21 => '0update-post_321'

Despite being two separate users and two separate actions, their nonce values will be the same. While this is fairly limited in what an attacker can do (you can’t pick arbitrary users and values, only “related” users and values), it’s also very easy to fix and completely eliminate the hole: simply add a non-integer separator between the segments of the hash input. Wordpress 3.9.2 now inserts a | between each segment, so now the hash inputs look like this:

$i . '|' . 'update-post_32' . '|' . 1 => '0|update-post_32|1'
$i . '|' . 'update-post_3' . '|' . 21 => '0|update-post_3|21'

No longer will the HMACs collide, so now two distinct nonces are generated, closing the CSRF hole. The implementation also now includes your session token, making it even harder for an attacker to generate a collision, though I can’t think of a specific hole that fixes (it does generate new nonces after a logout/login):

#!php
function wp_create_nonce($action = -1) {
	$user = wp_get_current_user();
	$uid = (int) $user->ID;
	# snipped

	$token = wp_get_session_token();
	$i = wp_nonce_tick();

	return substr( wp_hash( $i . '|' . $action . '|' . $uid . '|' . $token, 'nonce' ), -12, 10 );
}

Timing Attack

Though probably very difficult to exploit on modern systems, using PHP’s == to compare hashes results in a timing attack (not to mention the possibility of running afoul of PHP’s bizarre comparison behavior).

Formerly:

#!php
if ( substr(wp_hash($i . $action . $uid, 'nonce'), -12, 10) === $nonce ) {
  ...

Now:

#!php
$expected = substr( wp_hash( $i . '|' . $action . '|' . $uid . '|' . $token, 'nonce'), -12, 10 );
if ( hash_equals( $expected, $nonce ) ) {
  ...

hash_equals was added in PHP 5.6, but Wordpress provides their own, using a fairly common constant-time comparison pattern, if you don’t have it.

Summary

Even when you include all the right things in your CSRF implementation, it’s still possible to run into trouble if you combine them the wrong way. Much like a hash length extension attack, cryptography won’t save you if you’re putting things together without thinking about how an attacker can alter or vary it.

I’d like to thank the Wordpress security team for their responsiveness when I reported the issues here. I have nothing but positive things to say about the team and my interactions with them.