In a recent article, I talked about nonces, what they are and their role in WordPress. This article stressed the importance of using nonces to help prevent XSS and CSRF attacks. Soon after that post was published I read about multiple security vulnerabilities in the extremely popular plugin W3 Total Cache.
One of these vulnerabilities is the result of improper nonce validation. A nonce was not validated using the standard wp_verify_nonce() function. Instead, the nonce was validated using a == comparison. In this article, I’m going to cover string comparisons in PHP. Specifically some of the issues with string comparison, comparing hashes and why normal string comparison is not sufficient.
String Comparisons In A Dynamically Typed Language
Because PHP is a dynamically-typed language, we often need to deal with strings that represent integers or boolean values. For example, the string ‘1’ may represent the integer 1, or the boolean true.
To account for this, PHP and JavaScript provide both === and == comparisons. The === comparison takes into account data type and content, while the == comparison takes into account content only.
Take a look at this example:
<?php
if( 1 === '1' ){
//will not evaluate true
}
if( 1 == '1' ){
//will evaluate true
}
Both conditionals compare the string ‘1’ to the integer 1. The first uses === so it does not evaluate true, but the second uses ==, so it does.
Simple right? Yes, but things get really weird with == quickly. Look at these:
<?php
if( '1a' == 1 ){
//will evaluate true
}
if( 1 == '1a' ){
//will evaluate true
}
These are both going to evaluate to true. That looks silly since 1a and 1 don’t look the same. But, because one of them is an integer, both will be evaluated as an integer. When you think about it that way, it makes a lot of sense. The integer value of the string ‘1a’ is the integer 1, which is what makes it the same as the integer 1a.
Now using a === comparison would have made those comparisons evaluate false. This article isn’t an argument against ever using == comparisons. What makes them so useful is that the type is ignored. My point is to be mindful of this gotcha and think about when it might happen.
For example, consider this test:
<?php
if( 3 === get_post_meta( 42, 'some_key_with_a_number', true ){
}
Let’s say you know that this meta key is equal to 3, you’re going to be annoyed when this test fails because get_post_meta() will return the string 3. So you either need to use a == comparison or pass the meta value through intval() first.
Let’s Talk About Hashes
So, let’s get back to nonces. Here is the security issue that SecuPress found in w3tc:
<?php
//SEE: https://secupress.me/4-new-security-flaws-w3-total-cache-0-9-4-1/
$nonce = W3_Request::get_string('nonce');
$uri = $_SERVER['REQUEST_URI'];
if (wp_hash($uri) == $nonce) {
It looks like w3tc was using their own version of WordPress’ nonce verification. There are two problems with this. The first, as pointed out by SecuPress is that the == comparison allows for type juggling to be used to pass an invalid nonce and have it verify. This makes it easier to orchestrate the social-engineering side of an XSS attack.
Even with a === comparison, they would have still been left open to PHP timing attacks.
One important thing to keep in mind about how PHP compares two strings,1 character at a time, in order. This is like comparing ‘spaceman’ to ‘spacemen,’ which will take slightly longer than it takes to compare ‘spaceman’ to ‘earthlings’.
In the first comparison the first 5 characters are the same, in the second comparison, the first character is not the same. So, in the first case PHP runs 5 comparisons, while in the second case it only runs one.
This seems minor, but it’s not. That tiny difference in time can be used to drastically improve the efficiency of a brute force attack against a hash. This is why PHP introduced the function hash_equals() for comparing hashes with a randomization of the time involved.
For backwards compatibility reasons, WordPress includes this function for PHP 5.5 and earlier. You can see the source here. If you look under the comment ‘// Do not attempt to “optimize” this’ you will see that random timing is introduced to prevent timing attacks.
The function wp_verify_nonce() uses more than just hash_equals() to ensure are safe against timing attacks. This is why you should avoid doing something like w3tc did or this:
<?php
//don't use this:
if( $_GET[ '_wpnonce' ) === wp_create_nonce() ){
}
This will only evaluate true when the nonce is valid, and avoids one of the problems that happened in w3tc. But it is still a shortcut, that would leave you open to the other problem with what they did. It would expose you to timing attacks. By speeding up the process by a few tiny fractions of a second, an important security feature of WordPress has been lost.
Be Wary Of What You Optimize
Funny story, the first time I ever looked at how nonce verification I thought it was poorly done. Look at the source, it doesn’t make a lot of sense if you don’t understand timing attacks and how string comparisons work. I was about to get cute with nonces in a project. Lucky for me, I had a Chris Christoff handy to tell me why I was wrong.
I recently read an article in the New York Times that blamed the recent leak of 500 million Yahoo passwords, to a cultural refusal to prioritize security over user experience or performance. It was a strong reminder that optimizing your code is important, but never at the expense of security.
No Comments