Tuesday, August 11, 2009

WordPress Bugs... A Disturbing Vulnerability

Understanding bugs as they hit the wire has always been somewhat of a fascination with me... so I couldn't help but dig into the new WordPress vulnerability that hit Full-Disclosure earlier today. As I started working through the new WordPress 2..8.x admin-password-reset bug... I started giving myself a headache. Given PHP's bad name for having security issues it's not really hard to pin the "It's PHP's fault" tag on a vulnerability and move on. Not this time... err... not exactly anyway.

If you don't know what I'm going on about yet, go check it out here. Discovery credit goes to Laurent Gaffie, who posted it to Full-Disclosure... WordPress worked quickly to issue a fix here... (more on this below).

It would appear as though you can cause the administrator of any WordPress 2.8.x blog serious discomfort by repeatedly resetting the password to the system-generated random string. While this may not be anything to ring the alarms for - it does get quite annoying if you would like to administer your WordPress blog remotely (which is everybody, right?).

From the disclosure, submitting a query into the server like so
http://domain_name.tld/wp-login.php?action=rp&key[]=
causes it to simply skip verification and just re-set the password... how interesting!

Under ordinary circumstances, a user clicks on "forgot my password" then the server sends a one-time URL complete with random "key" to re-set the password. The thing to remember here is that the field in the user's record remains empty (null) until a password reset is requested! The user then gets an email with a URL that looks something like this:
http://domain_name.tld/wordpress/wp-login.php?action=rp&key=RANDOM0987654321
which he or she will click on. The server will then parse the request and compare the key parameter against the key stored in that user's database record. Easy so far.

Guessing the key is probably not realistic and not worth the attack bandwidth... but if you could just reset passwords without knowing the key then that would be insanely annoying, wouldn't it? The $64k question is... why does submitting a key[] (an empty array) work?

Looking at this issue it's clear there are several things possibly at play, including a little bug that may plague more PHP apps than we give it credit for - casting. Looking at the very first relevant line:
$key = preg_replace('/[^a-z0-9]/i', '', $key);
it appears as though this looks and works just fine... as long as the input in the $key variable is a string the line goes about making sure you only have alpha-numeric values in the $key parameter... golden so far. If you happen to try and stuff an array in there (or worse, an empty array), as evidenced by the key[]= above, this all goes sideways. As you'll now end up with an array in the $key variable... which the rest of the code is clearly not expecting.

Moving on... let's look at the SQL query string that gets built...
$user = $wpdb->get_row($wpdb->prepare("SELECT * FROM $wpdb->users WHERE user_activation_key = %s", $key));
Aha! If you've got an array in the $key variable, and you try to run this select statement then you end up with a very ugly array being flattened and stuffed into the %s (string)... What happens here is that WordPress will flatten the array - take the first value of the key[] which will be null and shove it into %s... and then go off and perform its database query. It ends up ooking something like this:

SELECT * FROM $wpdb->users WHERE user_activation_key = null

Since the administrator has not requested a password change, the value of $key, in the admin record will be null... thus you have a match and a password reset takes place. Makes perfect sense... Whoops.

Now presumably this hits admin every time since this is the first user typically created in the database, but I'm just speculating now since I haven't had a chance to thoroughly test this yet.

Even more interesting is the fix from the WordPress folks...
if ( empty( $key ) || is_array( $key ) )
The "patch" simply replaces the broken if empty statement and adds an is_array, which will check to see if we've accidentally left key blank or passed in an array and bomb if we have. Of course, this patches this bug but ...

Mike Bailey (@mckt_ on Twitter) and I were poking at this and wondered... why wouldn't you use "is_string" instead? Hasty coding? Why take the blacklist vs. whitelist approach?

Obviously, you have to wonder now... how many other bugs are there of this type in other PHP apps? What about other parts of WordPress? Hey... isn't there a rumor going around that Kaminsky and Matasano got taken by an undisclosed WordPress bug? Hrmm... While I agree with Mike's assessment that these types of bugs are rare, PHP certainly makes them more possible, especially when it doesn't have the opportunity to throw errors in a situation like this.

I think Mike's comment sums it up nicely though...
"It's [the source] full of WTF moments... I'm amazed anybody uses WordPress"
Indeed.

Big thanks to Mike Bailey for working through this with me... apparently caring for sleep as much (little) as I do!

10 comments:

Anonymous said...

Thanks Full Disclosure...

zonky said...

Surely if you want to admin any web app remotely, you ip restrict the admin panel components using htaccess and configure an openvpn tunnel advertising routes to the webhost that you use?

Isn't that just common sense?

Jason P. said...

I suppose for users that are sophisticated enough to use open source vpn solutions such as what you suggest (or even a simple L2TP tunnel via windows security) your solution addresses the issue. for everyone else however ip restrictions are only as effective as the fixed nature of the ip you come from.

there are additional plugs for Wordpress that allow you to utilize htaccess authentication in addition to word press auth. tho its a bit more of a pain, it has worked effectively for me, regardless of WP bug. more importantly it doesn't require that i have a vpn tunnel established, or that i am restricted to any one system.

what it does require is that I change my password regularly and use strong passwords.

jason...

zonky said...

Jason, I would venture if these 'simple' tasks are beyond a blog administrator, they shouldn't be managing their own wordpress install. - They should probably be using a hosted mu platform like wordpress.com

If anyone is considering a wordpress install, they should be looking at:

http://blogsecurity.net/projects/WordPress_Whitepaper_rev12.pdf

The problem with the wordpress one click install is that it's inherently weak, and if you ask at the wordpress sites, they'll tell you to leave a lot of content world writable so that the self-upgrader will work.*

All very retarded.

* I use a ftp daemon that listens on localhost only to 'elevate' permissions and upgrade.

jason.p said...

zonky...

I agree with your point. As someone that manages many of my own resources I couldn't agree more with your assertions. My systems maintain a blended range of locked administrative options, based on risk.

My point was that there are already solutions out there that restrict access, and do so without locking you into management from a specific system (or set of systems). I think that many people, even within the reduced audience of those that manage their own WP install, would find that limiting.

I also believe that there are many people that manage their own WP install that don't necessarily have access the system in such a way that they would be able to leverage tunneling. In these cases an alternate solution that uses simple user based Apache configurations would still provide an increased level of security.

I would certainly concede that the .htaccess solution has its own flaws. It is highly likely that any implementation will still be performed over clear text, and therefore still susceptible to exploit.

It isn't that I disagree with your point, as it is something that I have previously recommend to friends and clients. Simply that I think that solution comes with certain hurdles.

re: the ftp...i find that ssh tunneling is my friend for most of my administrative needs. however again it is because I have actual control over the remote server to implement these security controls.

jason...

Ash Searle said...

Strange that this doesn't appear to affect wordpress 2.7.x...

Old wordpress installs end-up generating this SQL:
SELECT * FROM $wpdb->users WHERE user_activation_key = 'Array'

That's not so successful - i.e. there's no vunerability at all.

It looks like this wasn't an issue till they changed their the database layer to accept array arguments to prepare (see wp-db.php prepare())


Final note: the fix deployed in 2.8.4 uses !is_string instead of is_array (i.e. white-list instead of black-list)

JanR. said...

Probably 'nuff said about this subject, but: The fix I added to my blog is on line 373 of wp-login.php, in the resetpass/rp case:

---
if(!is_string($_GET['key'])) { wp_redirect(’wp-login.php?action=lostpassword&error=invalidkey’); exit(); }
---

(I'm not really good at scripting PHP)

My blogpost, in Dutch, about this is on:
http://saotn.nl/2009/08/11/wordpress-2-8-3-remote-admin-password-reset/

Wouldn't this be a good (better?) solution?

jah said...

Thanks for this post! Until reading this I had a hard time understanding how an array containing only an empty string, when passed to prepare(), returns a row from the db. Now I understand!

Tom said...

The is_array() fixed was their first attempt, they modified it heavily just an hour after that commit.

Anonymous said...

In the latest version, they've changed it into a key/login combination.

See http://core.trac.wordpress.org/changeset?old_path=%2Ftrunk%2Fwp-login.php&old=11700&new_path=%2Ftrunk%2Fwp-login.php&new=11803

for the changes

Google+