Funny WordPress Vulnerability

Mike Tracy | January 23rd, 2008 | Filed Under: Uncategorized

There’s an interesting discussion on reddit about a recently “fixed” WordPress vulnerability. tqbf brought it to my attention this evening and it made me laugh… A LOT. It also got me thinking about too many cooks, bad recipe books and dead tasters for no other reason than I was hungry and in search of a metaphor.

Originally reported by Michael Brooks, this simply has to be one of the silliest vulnerabilities ever:

Looking at the code that caused all the ruckus, we find:

return ($wp_query->is_admin || (stripos($_SERVER[‘REQUEST_URI’], ‘wp-admin/’) !== false));

In PHP, the difference between _SERVER[‘PHP_SELF’] and _SERVER[‘REQUEST_URI’] is that the latter also gives you the parameters that were passed to a GET request. For example, getting the url http://www.victim.com/victim_blog/index.php?var=val will set PHP_SELF to “/victim_blog/index.php” and REQUEST_URI to “victim_blog/index.php?var=val”. What does this mean? Well, in the case of the above code from WordPress, if you GET an URL like /victim_blog/index.php?foo=wp-admin/&realvar=realval, is_admin() will return true.

This might sound like the end of the world except that in WordPressLand(tm), the is_admin() function is supposed to be used to find the user’s context and not to determine if the user has admin privileges right? Even though this is pretty shoddy, there’s no real privilege escalation to be had… correct? Hoooold the phone there laddy buck… also in the reddit comments is a code snippet from wp-includes/query.php that looks like:

1172              if ( is_admin() )
1173                  $where .= " OR post_status = ‘future’ OR post_status = ‘draft’ OR post_status = ‘pending’";

Apparently, we’re using the is_admin() call to figure out if a user is administrator after all? This snippet adds to the WHERE clause in the SQL query being built to display a list of posts. I think. &get_posts() has to be the mother of all query building functions. You try reading that code and see if you can make sense of it. The WHERE clause addition adds posts that haven’t been published yet to the list of posts to be displayed. It’s based on whether is_admin() returns true. is_admin() is only supposed to be used to find the user’s context not to determine if a user has ad… well… you get the point.

This kind of thing is academically interesting because it’s the kind of thing that web penetration tests (especially automated ones) will rarely, if ever, find. The Unsafe Use of User Input goblin lurks just around every equals sign, wating to fling your deepest darkest diary entries at unsuspecting passers-by.

More amusing still is the “fix” for the problem. Instead of actually checking if the user is an administrator (using the recommended method whatever that might be) before appending this “invasion of posts I am trying to keep hidden omg!” to the where clause, they just kinda change the is_admin() function to prevent the mishigas described above. In my heart of hearts, I think whoever was doing the fix was afraid to change anything in &get_posts() and perhaps unwittingly unleash SkyNet. The is_admin() function is used to determine… well… ok… nevermind.”

Viewing 10 Comments

Trackbacks

close Reblog this comment
Powered by Disqus · Learn more
blog comments powered by Disqus