Privilege Escalation Vulnerability in Easy Updates Manager
Yesterday we noted how two of the most popular WordPress plugins were insecurely using the function extract(), as they were extracting the $_POST variable, which involves untrusted user input and the documentation for that function warns against:
Warning Do not use extract() on untrusted data, like user input (e.g. $_GET, $_FILES).
We found that usage through a website WPDirectory, which we ran across recently, that makes it easy to check the code of all the plugins in WordPress’ Plugin Directory at once. When we ran a check for plugins doing the exact same thing with the $_REQUEST variable, those plugins were again flagged as well as the plugin Easy Updates Manager:
The code in that plugin turns out to be insecure as well.
Seeing as most of the most popular plugins that are extracting user input like this have vulnerabilities caused by it, we have added a new check to our Plugin Security Checker that flags this, so you can now check plugins you use to see if they might have similar issues with that tool.
That plugin registers the function ajax_handler() to be accessible through WordPress’ AJAX functionality by anyone logged in to WordPress:
42 | add_action('wp_ajax_eum_ajax', array($this, 'ajax_handler')); |
The relevant code in that function is as follows:
122 123 124 125 126 127 128 129 130 131 132 133 134 135 136 137 | public function ajax_handler() { if (empty($_REQUEST)) return; extract($_REQUEST); if (!wp_verify_nonce($nonce, $nonce_key) || empty($subaction)) die('Security check'); $results = array(); $data = isset($data) ? $data : array(); if (!method_exists($this, $subaction)) { do_action('eum_premium_ajax_handler', $subaction, $data); error_log("EUM: ajax_handler: no such command (".$subaction.")"); die('No such command'); } else { $results = call_user_func(array($this, $subaction), $data); |
That code does have some security in place, since it checks for a valid nonce to prevent cross-site request forgery (CSRF), but since user input specifies what nonce is check, access to a valid value for any nonce is good enough to get access to the functions code. Since CSRF involves causing someone else to take an action they didn’t intend, the code is working as intended, but since that is the only security in place the code is not secure.
After getting past that you can run functions specified by user input. In our testing it looks like you are limited to running functions in the MPSUM_Admin_Ajax class, so this wouldn’t necessarily be classified as remote code execution (RCE), but since anyone logged in to WordPress that has access to any valid nonce can run various functionality they are not intended to, it would at least be privilege escalation.
Some of the class’ functionality is further limited with a capabilities check, but others are not, as the proof of concept below for disabling automatic updates for a specified theme shows.
You could think that the explanation for having so poorly thought out code might be explained in part to do with the fact that it started life as very different plugin, as the slug for it is stops-core-theme-and-plugin-updates, which isn’t necessarily something you would expect someone familiar with security would be involved in, but it turns out the code being insecure was a recent situation, which looks to coincide with a premium version coming in to existence based on the changelog entries for the plugin.
The WordPress Tavern’s coverage of the plugin’s sale in January of last year, which preceded the code being vulnerable, included this line:
Without proper vetting, selling established plugins to individuals or companies can be harmful to sites and tarnish its reputation. Because UpdraftPlus is a well established company, Huereca didn’t have to do a lot of research.
Due to the moderators of the WordPress Support Forum’s continued inappropriate behavior we are full disclosing vulnerabilities in protest until WordPress gets that situation cleaned up, so we are releasing this post and then only trying to notify the developer through the WordPress Support Forum. You can notify the developer of this issue on the forum as well. Hopefully the moderators will finally see the light and clean up their act soon, so these full disclosures will no longer be needed (we hope they end soon). You would think they would have already done that since a previously full disclosed vulnerability was quickly on hackers’ radar, but it appears those moderators have such disdain for the rest of the WordPress community that their continued ability to act inappropriate is more important that what is best for the rest of the community.
Proof of Concept
The following proof of concept will disable automatic updates for the theme Twenty Nineteen, when logged in to WordPress. That can be confirmed on the page /wp-admin/index.php?page=mpsum-update-options&tab=themes.
Make sure to replace “[path to WordPress]” with the location of WordPress and “[heartbeat nonce]” with the value right after ‘var heartbeatSettings = {“nonce”:”‘ in the source code of admin pages.
<html> <body> <form action="http://[path to WordPress]/wp-admin/admin-ajax.php?action=eum_ajax" method="POST"> <input type="hidden" name="nonce_key" value="heartbeat-nonce" /> <input type="hidden" name="nonce" value="[heartbeat nonce]" /> <input type="hidden" name="subaction" value="save_themes_update_options" /> <input type="hidden" name="data[themes][twentynineteen]" value="false" /> <input type="submit" value="Submit" /> </form> </body> </html>
Hi,
Thank you for your research. We were tipped off about this a few hours ago by another source, and released a fix.
BTW, are you aware that you’re running WooCommerce 2.6.14 on your website, an insecure and ancient release – from over 2 years ago with a long list of known security defects and subsequent security releases? Including object injection and customer name disclosure and others (which, cough cough, “isn’t necessarily something you would expect someone familiar with security would be involved in”). Anyway, I’m glad the issue was discovered.
Best wishes,
David
The changes made, while fixing the vulnerability, don’t properly resolve the security weakness. The better course would be to directly register the AJAX accessible functions instead of running them through another function like you are now.
Cough cough, did you consider that the version of WooCommerce we use isn’t actually insecure? Also, you bought the plugin long after its purpose was changed, so you are missing the point there.
Why don’t the changes resolve the weakness? They now require the user to both a) produce a specific nonce, which is only given to admins and b) pass a `current_user_can()` test on any change-inducing action. The proof of concept in your post no longer works.
I do agree that it would be ultimately best to entirely eliminate the `extract()`. But, since you published to the world before telling us, there’s the practical consideration of getting a working fix out to sites before any real-world damage is done. Being safe matters to website owners.
To be honest, no, I haven’t bothered to investigate whether all the WooCommerce security defects found in the last two years exist in the ancient 2.6.14 version or not, or if they were all introduced subsequently, or if it’s OK to run a version that was only ever tested up to WP 4.7 on WP 5.1 and if that unsupported combination exposes new problems or not. If you’ve done that, well done to you (sounds like a lot of work). It’s just that on your blog you’ve previously said that lots and lots of things like “if people were doing the security basic step of keeping software up to date…” (https://www.pluginvulnerabilities.com/2018/02/21/the-failure-to-update-vulnerable-plugin-is-reminder-of-security-industrys-apparent-lack-of-interest-in-making-sure-websites-are-secure/), and I agree with that sentiment; running very seriously out-of-date software is bad practice, and that’s basic, not rocket science.
I hope you understand my point, of course. I don’t mean that because you made a mistake, therefore I didn’t. I did. I reviewed that line personally in the merge commit, and should have spotted the obvious blunder, but failed to. You found it, and I am in your debt – thank you again. My point isn’t that because you made a mistake, my one is excused. I’m rather getting at the fact that the rhetoric in your blogs is sometimes over-the-top, and you risk your own credibility by it. You don’t need to do that; the quality of your work can stand without it.
David
As we said before, the vulnerability is fixed, so it should go without say the proof of concept for the vulnerability won’t work.
Here is what the nonce documentation states:
Protecting only some of the functions with current_user_can() doesn’t solve the security weakness in the two stage process, which is why we suggested instead of having a two stage process, which doesn’t have that in the first stage, just use a one stage process.
We didn’t say that version 2.6.14 contain doesn’t contain security defects, what we said did you consider that the version of WooCommerce we use isn’t actually insecure, which is different. Also, again, you bought the plugin long after its purpose was changed, so you are missing the point there. We were just noting that the plugin’s slug indicated something, which didn’t have anything to do with your time as the owner.
You are not in our debt for us finding a vulnerability in your plugin.
Thank you for all your advice. I entirely agree with what would be optimal to prevent accidentally introducing new vulnerabilities in that area, given more time, and it’s certainly something we’ll want to do if users’ continued interest in Easy Updates Manager justifies further development. (We’re currently in a phase of seeing how the market responds to the Premium offering, which we hope will allow ongoing maintenance and improvement of quality free + Premium offerings).