Remote exploit found in all WooThemes
gist.github.com
gist.github.com
Thanks all for your comments. Regarding responsible disclosure, I'm aware of the concept. I did Google around and looked on the WooThemes site for a security notice address or similar and didn't find one. Even as a paying subscription customer, which I have happily been for the past year, the only way to get support is via their public forum.
That being said, this is only "half" of the equation. I have already seen numerous hosting accounts compromised via a more malicious form of this attack which I have not published. In fact, finding a number of sites running WooThemes all compromised in the past 4 days via the contents of shortcode-generator/ lead me to take a quick look through the code to try to find the attack vector and I found this.
Note that what I found isn't even really an exploit in its current form - just a bug which combined with a lot more work could be used to potentially cause harm. However, since I'm already seeing WooThemes-powered sites that have been compromised via the contents of shortcode-generator/, there is already a more serious problem in the wild. I would hope that WooThemes is aware of that - but again, they don't appear to have any security contact address, or any other transparency about the security of their products to be able to find out.
With regards to security at WooThemes, I won't comment on their attack this week because clearly they are trying to recover from it. However, as one commenter in HackerNews posted, "Does this look like the authors even give a crap about security?" and with that I would tend to agree. For example, a few days prior to their site going down, WooThemes alerted their customers to update their themes for a security vulnerability... via a 5am tweet: https://twitter.com/#!/woothemes/status/192545687051829248 "We found a minor vulnerability in the WooFramework, which we've just fixed. Please update to the latest version ASAP!" Not an email alert, not a blog post with details, just a tweet.
Regarding the person who commented that this issue was already patched; that is incorrect. This has been tested against vanilla WooThemes running the latest version, which (according to the theme's built in updater) is 5.3.7. The WooThemes website only lists a changelog up to version 5.3.1, released almost 2 months ago: http://www.woothemes.com/updates/functions-changelog.txt
Additionally, even if the issue is patched, my link above still works - which means that the patch clearly isn't working or hasn't been applied to WooThemes own servers.
The moral of this story is: WooThemes is a great company and makes a great product, but they have grown to the point where security needs to be a real concern. A proper channel to alert them of these issues, along with prompt and honest email notifications of updates to their customers (free and paid), and a publicly-accessible security/updates site (a la RedHat's RHSA system) are all long overdue. This isn't just a jab at WooThemes either - a review of almost any paid or free theme will surely come up with many issues like this.
Perhaps the best solution is for WordPress to add a built in update notifier that can alert site admins when updates are available, but at this point anything would be helpful.
Thanks again for the comments and thanks to those who posted simple patches to correct this small issue until an official update is released.
http://webcache.googleusercontent.com/search?q=cache:FhfL-lo...
(The changelog that's posted live ends at 3/06/2012. The one in Google's cache lists changes up to 4/23, including the fix for this bug).
Hanlon's razor and all, but its odd.
But just look at the code itself. Does this look like the authors even give a crap about security? Or half decent code in general? Those 3 lines of code alone make me cry, and it pretty much screams "potential security issue".
This kind of crap is endemic in the WordPress ecosystem, and virtually every theme and plugin is a security hole waiting to be exploited.
Now if you're sharing your code freely that's one thing, but these guys run a business and are asking money for this. One of their selling points, the first on their homepage: "Solid Code Framework". Seriously.
(Gist author posts a longer explanation, and mentions this as part of his message.)
Wow, I had no idea that was even possible, but on the surface it seems incredibly stupid to allow an editor to add server-side code to content in a CMS.
+ Marvel at the efficiency where anonymous subscribers are stored in the same database table as all-powerful system admins, which makes it much easier to upgrade anonymous Internet commenters to admins using local privilege escalation.
+ Enjoy at least five different ways to execute arbitrary code against the server from the admin console. For added fun, they're secured independently from each other!
+ Stop wasting time with fine-grained permissions models: all plugins/themes get unrestricted read/write access to the database and arbitrary code execution by default.
+ Experiencing a shortage of qualified engineers? No problem! We've lowered barriers to contribution so much that a significant portion of the community output has been made by people who know just enough PHP to eval($_GET["something"]). You'll find them in our semi-curated plugin/themes lists, sorted by star rating, with easy one-click access for admins to install on your server.
(I actually like Wordpress, but certainly not for its security record.)
I agree with your comment, though. Wordpress has poor coding practices built into the core (arguably like the language it's written in).
Here's the way NOT to do it: Bingo Card Creator has a single users table with a role attribute. Setting role to "admin" makes your account into me. This is dangerous because it increases the damage someone can do with other attacks -- if, for example, I was bitten by the (common to Rails applications) vulnerability of letting users' profile pages reassign roles on update due to a mass assignment vulnerability, that would cough up everything my web app can do. WordPress has the same pattern, which means any of the (historically quite common) ways to convince WP that the logged in user is_admin() mean you can go to town on the software. This is made more problematic because, as noted, WordPress gives admins arbitrary code execution.
There are smarter ways to do this for y'alls own software, by the way. Probably the gold standard is:
+ keep the admin functionality in an entirely separate application at admin.example.com rather than example.com/admin/
+ enforce a strict access policy to the admin application at the network level. Probably the most straightforward is firewalling the admin app from the outside world -- the only way to get to it would be e.g. VPNing into the local network.
That would probably not be a viable option for WordPress, but you're free to not have the security of your businesses informed by WordPress' hard requirement that anybody be able to install the software within 5 minutes and have it work magically.
SQL injection or other auth type vulnerabilities that allow you to alter field values are a lot more common than those that allow you to run a general statement to alter another table
For the people not yet on the latest WooFramework version: You can download the latest version of WooFramework here: http://cl.ly/2a3j1m351C3u2i0t122j (it is 5.3.10)
Do you know how to manually update the framework? This zip file unzips to a "framework" folder, you need to replace to contents of the "functions" folder of your theme. Obviously make a backup of everything before you start.
This exploit is fixed in the latest two stable builds and should have been reported to us (WooThemes team) in the first place. Wonder if this guy has ever heard of the concept of responsible disclosure...
Are you seriously suggesting downloading the fix for vulnerability from some random website?
I never receive notice to upgrade the theme in the dashboard (and the option is activated).
what do i do now ?
Seriously, if you are a representative of woothemes.com, this approach is a major fail. you have a known website, that's where your authority and trust resides. Use it.
This just looks like someone attempting to get people to add a new backdoor to their existing wootheme powered site.
Like I mentioned on GitHub, your updater is broken, so your customers aren't getting this update even if you guys did release it. See my comment for details, but it appears that the file which triggers the Update Framework feature to work has been rolled back to an old version.
Regarding your comment on 'responsible disclosure', where did you responsibly disclose this exploit to your customers and notify them that a critical update was needed to prevent, in your words "a way to cripple many (many, many, many) websites."?
2012.04.29 - version 5.3.11 - critical * - SECURITY UPDATE - Shortcode preview feature removed in previous version due to potential exploit. * admin-functions.php - Make sure PressTrends banner displays only for PressTrends-enabled themes.
I hope that in the future WooThemes (and the WordPress community as a whole) takes security more seriously and notifies their customers more rapidly about these sorts of problems.
Thanks to everyone who commented!
Please see our official blog post here, where we detail the issue and the solution we've implemented: http://www.woothemes.com/2012/04/framework-shortcode-exploit...
This is a very serious exploit. Combined w vulnerabilities in other unsecure but popular plugins[1], this exploit is as damaging as one can possibly be. I have a feeling I'll be cleaning up after this one for a long time (we're still having to clean up hacked sites from timthumb).
Look, mistakes get made. But you need to own up to it, and be honest about the size of the problem. In particular, I find these passages disingenuous:
"Rest assured there is no link and the exploit was actually fixed a few days before our website was hacked." (in bold)
Given that the exploit was live, on your server, this is misleading at best. Yes, you had found the bug and committed it on 4/23. But obviously it hadn't made its way to active WooThemes installations, including your own. In your repo != fixed.
"The potential exploit is such that the shortcode preview allowed users to generate shortcodes using the preview window’s file, without authenticating the user."
More accurate would be: "the exploit allows anyone off the street to run any shortcode from any plugin you have installed. Vulnerabilities that were limited to logged in users before are now available to bots."
The OP should have been more responsible in his disclosure. But so should have WooThemes. Nothing in your public communications reflects the seriousness of this exploit.
For what its worth, you just lost (at least) one customer.
We can assure you that we are aware of the nature of this possible exploit. Security is of paramount importance to us.
The wording on our blog post is such to make known the severity of the possible exploit, while also being sure to ensure our customers that the possible exploit has been patched successfully.
To ensure that we communicate as effectively as possible to as many audiences as possible in possible future exploits, we've setup a new development-specific channel on Twitter and will be making other avenues of communication open to ensure that we inform our customers of security updates as quickly as possible.
That said, I really wish the original public report had been a private report.
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-stringshttps://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.
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));
}
1. http://codex.wordpress.org/Editing_wp-config.php#Moving_wp-c...
[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 smart fix is to check for user permissions and nonce before rendering the shortcode preview, which I hope is what the Woo patch does.
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.
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.
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?
This is precisely why I do not like $_REQUEST, and counsel against it in presentations/classes I do on PHP. This is clearly written to be something which modifies state - GET values should be ignored in this situations. And yes, again, it's not perfect - someone could craft malicious POST requests - I know - but that requires an extra level of effort not currently needed, and it's an extra level of effort some people won't bother with.