The conjuring discussion regarding security checking on the WordPress wp-hackers mailing list and my subsequent testing have revealed a serious security flaw in Liberated WordPress 1.5.2, the software that runs this blog.

The flaw allows anyone I trust to post drafts, to make a draft post containing images with appropriately forged source hrefs, to subsequently destroy the entire contents of the blog when an administrator views the draft, with no interaction required.

In other words, an automatic nuclear bomb.

Fortunately, the only person entrusted to write posts on this blog is me, so unless I decide to nuke myself, I’m pretty safe. On the other hand, knowing my blog could vapourise at any time gives me the willies. I can’t wait for someone else to issue a repair.

I’m going to have to fix this one all by myself.


Objective

Repair draft image forgery vulnerability in WordPress.

Plan

  1. Assess DB manipulation functions with client-side access control
  2. Move all access control to server-side
  3. Code ‘make_admin_command_href’ and make pluggable, along with ‘check_admin_referer’
  4. Code plugin to implement PIN verification with referer check

10 Comments

  1. Libertus says:



    Assess Access Control

    Some programs within WordPress perform database manipulation (such as DELETE) in response to HTTP GET requests. The only protection is a simple check that the referring URL originates from the blog’s administration interface (get_bloginfo('siteurl').'/wp-admin/'). The draft forgery uses images loaded by the administrator’s browser to satisfy this check and therefore perform the database manipulation.

    Which of the sub-commands change the database in response to GET?

    Scanning around the code brings me to wp-admin/edit.php and the “Delete Post” links with JavaScript confirmations. My guess is that where I find calls to deleteSomething() I will also find a sub-command in need of repair.

    Ignoring the definition of deleteSomething() (not of interest), there are 12 references to the function, all embedded within the generated links to the back-end sub-commands. Just read off the links…

    • post.php action=delete
    • page.php action=delete
    • comment.php action=deletecomment
    • categories.php action=delete
    • link.php action=delete

    Analysing the sub-commands reveals only one surprise; comment.php actually has an ‘Are You Sure?’ form for deleting and approving comments, which is good but is just lip-service to security as the form is submitted by method GET, thereby deliberately creating the hole for the forger to get through and delete comments. All the other sub-commands lack any form of access control other than the client-side JavaScript.

    What a mess.

  2. Libertus says:



    Code Access Control

    First thing is to make these sub-commands safe from forgery by adapting them to only work with data provided using the POST submission method.

    The beginning of the delete sub-command in wp-admin/post.php, out-of-box is:

    case 'delete':
      check_admin_referer();

      $post_id = (isset($_GET['post'])) ? intval($_GET['post']) : intval($_POST['post_ID']);

      $post = & get_post($post_id);

      if ( !current_user_can('edit_post', $post_id) )
        die( __('You are not allowed to delete this post.') );

    The ID of the post to be deleted is first taken from the GET, then from the POST, which is unsafe. I want the command only to operate on POST and show a confirmation dialog on GET, which will marshal the request into a POST if the user clicks “OK”.

    case 'delete':
      check_admin_referer();


      if( $_GET['post'] ) {
        send_admin_confirmation_form(
          "delete post ID $_GET[post]",
          'post.php',
          array(
            'action' => 'delete',
            'post_ID' => $_GET['post']
          )
        );
        break;
      }

      $post_id = intval($_POST['post_ID']);

      $post = & get_post($post_id);

      if ( !current_user_can('delete_post', $post_id) )
        die( __('You are not allowed to delete this post.') );

    Now, the post ID can only be presented via POST. Function send_admin_confirmation_form is in wp-admin/admin-functions.php and is as follows:

    function send_admin_confirmation_form( $message, $program, $params ) {

    $admin_command_url =
      get_settings('siteurl')
      . '/wp-admin/'
      . $program
    ;

    // Assemble parameters as hidden form INPUTs
    if( $params )
      foreach( $params as $key => $value )
        $hidden .=
          "<input type='hidden' id='$key' name='$key' value='$value' />\n";

    echo <<<HTML
    <form method='post' action='$admin_command_url'>
    <p>Are you sure you want to $message?</p>
    <input type='submit' id='submit' name='submit' value='OK' />
    $hidden
    </form>
    HTML;

    }

    OK, so the form is a bit spartan, but it does the job. This change repairs the forgery flaw by disallowing post deletion by GET. Only pages, comments, categories and links to go!

  3. Libertus says:

    Unit Testing

    Having applied the simple confirmation diversion to each of the affected delete sub-commands, I need to test that each works correctly, that the WordPress user interface is unaffected and finally, that the draft forgery nuke is now impotent.

    For the geeks out there, here’s the patch I’m testing, against svn.automattic.com/wordpress/trunk:3730.

    I’m using an old copy of my blog DB as test data. A bit of hassle upgrading the database to the latest, requiring me to set the db_version in my database by hand, but this is alpha software. Hmm… still more hassle – a “cyclic link” of some sort. Bugger this – just start fresh.

    Sigh. OK, blank blog, Zeitgeist installed, ready for testing.

    Step 1. Create three of everything; posts, pages, categories, links and comments. Check except there is no submit comment form for pages. Fix by adding comments_template() to wp-content/themes/default/page.php.

    Here’s the Zeitgeist for my test data.

    Step 2. Delete one of everything using the admin UI; link check, category check, comment check, page check and post check.

    The admin interface checks out.

    Step 3. Attempt to delete one of everything using direct GET. Create a comment with embedded links with hrefs set to delete one of everything. This is easily done by copying the locations from the “Delete” links in the admin UI.

    The constructed comment with forged links is as follows:

    admin Says:

    This is the direct GET delete test comment. Each link below attempts to bypass the admin referer check and delete an item.

    Clicked from outside the admin interface, the correct result is the “Enable Sending Referrers” page.

    Clicked from within the admin interface, the correct result should be that each link leads to a confirmation form that requires an OK click to perform the deletion.

    If any click performs the deletion, the test has failed.

    Link
    Category
    Comment
    Page
    Post

    Clickety-click! First from outside the admin interface. All check.
    Now from inside. All clicks lead to a confirmation form. Good. Can the deletes be completed from the confirmation forms? Link fail – error in deleting, category check, comment fail – delete completes but doesn’t redirect, page check and post check, but redirects to “Write Post” instead of “Manage Posts”.

    Not bad. Two failures and one niggle. I’ll get on to those once I’ve started work.

  4. Libertus says:

    Work

    Zeitgeist One (beta) is desperately in need of an 8th revision so I can’t afford to spend too much of my day frigging around with someone else’s problems. Once the links delete and the redirects are fixed, I’m done.

    Cool stuff like the admin PIN will have to wait until I’m done with my own cooler stuff.

    Link Deletion: OOPS! I sent the command to post.php. Fix check. Bye bye, Ryan!
    Failed Redirects: Fuck it! Who cares? Deleting is done from the UI anyway.

    My Campaign for Nuclear Disarmament is complete!

  5. Libertus says:

    The Ultimate Weapon, For Old Time’s Sake

    I have to be sure that the weapon is harmless. I need to build the ultimate bomb. With PHP, it’s easy and safe.

    I’ve decided to keep it simple and nuke all the posts only, as that also takes out pages and comments.

    Suitably armed, I proceed to the testing range. I have two targets; a local blog running an unrepaired version of Liberated WordPress 1.5.2 and the bleeding-edge blog I have repaired. The plan is to post the nuke on each blog and then view each post. Only one will survive. I hope.

    Liberated WordPress 1.5.2WordPress 2.0.2

    My test data consists of an old copy of my blog database, comprising 200-odd posts. I don’t want to lose this so I hot-copy the data into a new database and update wp-config.php accordingly.

    Please Don’t Nuke Me! (I’m Vulnerable)

    I’m sorry! It’s for the greater good, you know… I paste the nuke. I save the nuke.

    Hmmm… that’s taking a bit longer than I expected… then a surprise! DOH! I’m testing against WordPress 2.0.2 and it’s godforsaken visual rich editor. Ach well, mistakes happen. Set off the nuke…

    Uh… Guys… Where’s The Nuke?

    Nothing! The post is empty. OK, switch off the visual rich editor and try again. Paste the nuke. Save the nuke. Edit the nuke.

    AHHHHHH! Lots of connections being made to localhost. There’s nothing like the sound of a well-crafted explosion (at least in software terms, I’m not at all fond of the physical type).

    The main difference being that you can’t tell if a software explosion has actually happened until you check, and mine was a dud! Bugger. Maybe I didn’t pack it right…

    Make It Properly This Time, You Dolt!

    There’s a world of difference between & and &amp; and that can be confusing. Try again.

    Still nothing. It seems the IFRAME post preview protects WordPress 2.0.2 from my nuke post. That is excellent news. On the other hand, I still want my bomb to go off, so I’ll have to do it by editing a comment.

    Which works! But unfortunately it is less of a BANG and more of a hissssssssss. Deleting the posts from ID 1 upwards takes some time to get to the present day. Besides, I’ve had a better idea. Switch the visual rich editor back on…

    Tsk. A Simple Error.

    Maybe I should use the full URL for each image instead of a relative one? Yes! That’s more like it! I need to update the bomb-making script then I can use the visual rich editor to set it off.

    That’s Rich

    Rich editor off, post the nuke. Rich editor on, edit the nuke, see it briefly, then it goes off. It’s an odd sound… Waiting for localhost…Waiting for localhost… Waiting for localhost….

    Blog’s End

    My God… what have I done? The devastation.

    COOL! AGAIN!!!

  6. Ryan Scheuermann says:

    Nice work Libertus! This post will be a supporting citation to my thesis about a programmer’s predilection towards building bombs.

    Let’s just hope this doesn’t fall into the wrong hands before the patch is made public…

  7. Libertus says:

    Not So Ultimate Now, Are You?

    WordPress 2.0.2, and by extension my derivative Liberated WordPress 1.5.2, are vulnerable. What about the repaired bleeding-edge?

    This is a smaller test – only the first 10 posts are targeted and therefore only the repair to the post delete sub-command. I can test the rest later as I have other work to attend to. Sheesh, time management!

    OK. Rich editor off. Paste the nuke. Post the nuke. Rich editor on. Edit the nuke. Same sound, lots of waiting for localhost but the posts are intact.

    Finally! A reason to stop procrastinating, finish off Liberated WordPress 1.5.0 and release the damned thing. A new security shield. Cool.

  8. Libertus says:

    Hi Ryan!

    Thank you! Feel free to cite this post in any way you enjoy. I have thoroughly enjoyed writing it.

    I don’t think programmers are any more inclined towards destructive behaviour than anyone else. It’s just that the appeal of destroying virtual things is the ability to repeat the experience at will and in great detail. The trouble with destroying real things is the safe distance one has to keep, which takes all the fun away, so making it an entirely pointless exercise of the imagination.

    As to the “wrong hands”, in this particular instance they would be the hands of someone you already trust, which would make them the “right hands”, wouldn’t it?

    Oh, I’m all morally confused now! :)

  9. Yogurt Earl says:

    
    IMG tag , genious.
    
  10. priscilla says:

    aleexit

Leave a Reply

You must be logged in to post a comment.