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.”

9 Comments so far

  • Hugo

    January 23rd, 2008 2:41 pm

    “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.”

    Perhaps by now you already know your’e wrong. PHP_SELF is as much vulnerable as REQUEST_URI.

    Peace. Lets see what gobless saz :]

  • Mike Tracy

    January 23rd, 2008 3:00 pm

    I’m not sure about “wrong”. All that statement was intended to do was point out that the way the bug was exploited in the snippet at blackhatdomainer.com requires the use of REQUEST_URI. It was not intended to suggest a fix or in any other way condone the coding practices of the WordPress crew.

    I probably should have been more clear in the idea I was expressing but I was too busy snorting a perfectly good bottle of wine out of my nose.

  • Nate

    January 23rd, 2008 6:12 pm

    Oh no, you’re picking up the Thomas style of oblique asides. Trying to sort info from cuteness and failing…

  • Thomas Ptacek

    January 23rd, 2008 6:29 pm

    What does that comment even mean?

  • Mike Tracy

    January 23rd, 2008 6:29 pm

    And why does my blog post all of a sudden become about Tom?

  • ndg

    January 23rd, 2008 7:21 pm

    “Apparently, we’re using the is_admin() call to figure out if a user is administrator after all?”

    No, it’s because pending/draft posts are only displayed in the administration interface (on the “manage posts” page, etc).

    The issue here is that WordPress classifies posts along two axes, published-unpublished and public-private. The exploit reveals (unpublished & public) data only; posts marked as “private” stay private, even though an administrator would be able to see them.

  • Mike Tracy

    January 23rd, 2008 7:43 pm

    ndg: Thank you for the clarification.

  • Doug Steele

    February 17th, 2008 11:01 pm

    Interesting. There are some good ideass presented here. I need to do spend some time reading more about these topics.

  • SEO Ranter

    April 11th, 2008 5:08 am

    Haha, thanks for that, very entertaining. It’s disappointing that no-one’s fixed this rather weak looking bit of coding (e.g. the entirety of wordpress) yet!

    • Leave a reply