Hacker News new | past | comments | ask | show | jobs | submit login

Do you really think sharing this info on HN will do much good? Seems like it is just a "injection attack me" sign for a bunch of Wordpress sites.



I just patched my wootheme'd wordpress sites to address the issue until an official fix is out, so at least one person got positive value from this post.

That said, I really wish the original public report had been a private report.


I just commented it out myself but validating $shortcode might be a better solution.

[update] that wouldn't work either. it would still allow people to insert random shortcodes with whatever params they like. i'm not sure what the point of this function is, I don't think it should allow shortcodes to be passed in at all via _REQUEST


The php code would be inside a shortcode that looked like [php][/php], so no, that wouldn't protect you.

The smart fix is to check for user permissions and nonce before rendering the shortcode preview, which I hope is what the Woo patch does.


cool thanks, yea i just looked at do_shortcode again. this woothemes function seems like a bad design.

Authentication would plug it from random attackers at least. But it seems to me it would still be ripe for a CSRF attack..? That might seem unlikely but I can imagine the attacker could post a comment with a link on the victims blog who had been identified as having a wootheme installed. If the victim clicked the link (likely while authenticated) the attacker's php code would execute.


Nonces, used properly, should be fairly decent protection against the kind of CSRF attack you're describing. They're not bulletproof, but, someone who knows enough about the internals of your site to generate a valid nonce probably has several other possible attack vectors...

I haven't looked at the actual theme in question, but I can imagine that a lot of Woo clientele want to be able to preview their posts with all the shortcodes intact, which is what this function does, and why it has to receive shortcode data through request parameter.


I haven't seen their patch yet. I'd be surprised to see a nonce, my guess is they just call the standard Wordpress function to require auth.

There's surely a better way to do it without accepting code via the query string. Keep the code on the server and have that function only refer to an index or something perhaps?


Would there be much harm in posting the fix for others? If not, could you?


Sorry, I didn't think to post it because it's not a generally useful patch. It eliminates rather than fixes the functionality.

Here's to hoping Woothemes gets this sorted out. As it stands, automatic updating to 5.3.11 is still broken, and instead of stating this directly, it claims that your currently installed (out of date) version is current.

That's a cute side effect of woo_get_fw_version returning the currently installed version # instead of throwing an error if http://www.woothemes.com/updates/functions-changelog.txt isn't available for download (which it isn't, at the moment.)

    --- ./wp-content/themes/inspire/functions/js/shortcode-generator/preview-shortcode-external.php.orig
    +++ ./wp-content/themes/inspire/functions/js/shortcode-generator/preview-shortcode-external.php	
    @@ -58,6 +58,7 @@
    <?php
 
    $shortcode = isset($_REQUEST['shortcode']) ? $_REQUEST['shortcode'] : '';
    +$shortcode = '';
 
    // WordPress automatically adds slashes to quotes
    // http://stackoverflow.com/questions/3812128/although-magic-quotes-are-turned-off-still-escaped-strings


I wrote a simple Nagios check that can be used to watch for WooFramework updates, and published it here:

https://github.com/kway/check_woo

At this point in time it returns 'UNKNOWN' because the WooThemes changelog file isn't fetchable, but it should return CRIT/WARN/OK once they have their shop back in order.

Sharing it here in case it might help somebody notice critical fixes more expediently in the future.


+1, could you post your patch? My budding startup idea's blog is powered by a WooTheme, and I have a conference this week, so I'd rather deal with this sooner than later.


I'm not sure how WooThemes uses this file, but you can require a user to be logged in to access it fairly easily. This might break some part of WooTheme's functionality.

Edit `your-theme/functions/js/shortcode-generator/preview-shortcode-external.php`

Somewhere below the `require_once( $url . '/wp-load.php' );` line:

   if(!is_user_logged_in())
   {
       wp_die(__('Nope'), __('Nope'), array('response' => 403));
   }
If you happened to move your wp-content directory[1] this exploit is not going to work on you. The `require_once` statement above relies on wp-content existing so it can find `wp-load.php`

1. http://codex.wordpress.org/Editing_wp-config.php#Moving_wp-c...




Consider applying for YC's Spring batch! Applications are open till Feb 11.

Guidelines | FAQ | Lists | API | Security | Legal | Apply to YC | Contact

Search: