5 ms·

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.

rscale 14y ago
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.
- 8ig8 14y ago
  Would there be much harm in posting the fix for others? If not, could you?
  - rscale 14y ago
    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 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
  - rscale 14y ago
    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 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.
- HorizonXP 14y ago
  +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.
  - deleted 14y ago
    [deleted]
  - chrisguitarguy 14y ago
    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-content http://codex.wordpress.org/Editing_wp-config.php#Moving_wp-c...
- jakejake 14y ago
  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
  - golden_apples 14y ago
    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.
    - jakejake 14y ago
      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.
      - golden_apples 14y ago
        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.
        jakejake 14y ago

rscale 14y ago

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.

8ig8 14y ago
Would there be much harm in posting the fix for others? If not, could you?
- rscale 14y ago
  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 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
- rscale 14y ago
  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 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.
HorizonXP 14y ago
+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.
- deleted 14y ago
  [deleted]
- chrisguitarguy 14y ago
  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-content http://codex.wordpress.org/Editing_wp-config.php#Moving_wp-c...
jakejake 14y ago
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
- golden_apples 14y ago
  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.
  - jakejake 14y ago
    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.
    - golden_apples 14y ago
      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.
      - jakejake 14y ago