We are big believers in having the full details of vulnerabilities, whether they are in WordPress plugins or other software, be disclosed in most instances. That isn’t because that makes our work of compiling data on ones in WordPress plugins easier, but because we see the positive impact that has, as well as the more often emphasized negative impact. One of the important reasons for doing that is that we often find vulnerabilities that were supposed to have been fixed have only been partially fixed or not fixed at all. With more details it makes it easier for others to check to make sure the vulnerability has been fully fixed.
What is important to keep in mind though is that just releasing those details doesn’t mean that they will be checked and any unfixed vulnerabilities will be caught. When it comes to WordPress plugins, that is one of quite a few things that we seem to be the only ones doing. You wouldn’t know that by claims that people make about us, for example in a recent review of the companion for a plugin, which was less a review of the plugin and more someone just bashing us, they wrote:
Why isn’t the author doing more to help with the security community instead of bashing everyone?
Based on one of their other reviews they are a customer of Wordfence, so it wasn’t surprising that they wouldn’t know what we are up to do, since Wordfence is a company that intentionally hid that we had been the discoverer and discloser of a vulnerability they were discussing not too long ago (something they have done not just with us, but others companies as well). We had responded to that review asking for evidence that someone does more than we do and we have yet to get any response.
In another review, which was changed well after it was written (after, it seems the writer didn’t take kindly to us disagreeing with them about something, which seems to be a reoccurring issue with them), the reviewer made this claim:
The WPScan Vulnerbility Database is a valuable resource for WordPress users and developers, but the author has nothing but negative things to say about them, presumably since they do “competing” work. (Even though they are in completely different leagues – WPScan’s resources are far more robust.)
We have repeatedly said that is good resource for a lot of people (since the data can be accessed for free), but also that there are important limitations that people should know about. So claiming that we have “nothing but negative things to say about them” isn’t true. What also wasn’t true was the last part of that “Even though they are in completely different leagues – WPScan’s resources are far more robust.”. There wasn’t any evidence provided that backed that up and it certainly isn’t true for a number of reasons. One of them is that in terms of newly disclosed vulnerabilities, we are adding many more vulnerabilities than them. Another reason that isn’t true is that we actually test out vulnerabilities before adding them to our data set and WPScan doesn’t, which would catch unfixed ones.
Claims of Fixed Vulnerabilities in RegistrationMagic
Last week Rob Carr put out claims that there had been an authenticated SQL injection and a reflected XSS vulnerability that had been in the plugin RegistrationMagic and had been fixed in version 184.108.40.206.
Those claims were then repeated on the WPScan Vulnerability Database:
And also by WPCampus:
Only Accessible by Administrators
When we went to see about adding those vulnerabilities to our data set we first looked at the claim of an authenticated SQL injection vulnerability. While the report contains a lot of detail, it is missing a key detail, what level of user is required to exploit this. That is important since if it was only Administrators that could access this, it wouldn’t really be a vulnerability, since those users would normally be able to do the equivalent of SQL injection.
When we started testing it, we tried the proof of concept URL logged in as Administrator and it worked. We then tried it logged in as a user a level below, an Editor. When we did we got served a page that said:
Sorry, you are not allowed to access this page.
That would seem to indicate that only Administrators could exploit this. To confirm that, we looked at what capability the page that was being visited in the proof of concept required. The code that does is that below, and the fourth parameter in that list the capability required:
add_submenu_page("", RM_UI_Strings::get('ADMIN_MENU_MNG_FIELDS_PT'), RM_UI_Strings::get('ADMIN_MENU_MNG_FIELDS_PT'), "manage_options", "rm_field_manage", array($this->get_controller(), 'run'));
The “manage_options” capability is a capability that normally only Administrator level users have (and normally if lower level users have it they would also have the capability to create new Administrator users). So there isn’t really a vulnerability here, as we see it. It could be that those other sources have a different view of things, but based on looking into the second claimed vulnerability, it seems likely they did even check what users this was limited to.
This Vulnerability Hadn’t Been Fixed
The second vulnerability sounded unusual to us, as the claim was that the SQL injection vulnerability could be used to cause reflected cross-site scripting (XSS). That isn’t something we can recall seeing before.
We first tried out the proof of concept URL with version 220.127.116.11 installed and it worked. We then tried it in 18.104.22.168, which was what the discoverer and the listings on the WPScan Vulnerability Database and WPCampus claimed was the version that fixed it. The proof of concept still worked. We then tried it in the then latest version, 22.214.171.124, and it still worked. At that point we installed version 126.96.36.199 in a new install of WordPress, to make sure that the successful exploration when version 188.8.131.52 was tested didn’t cause the tries with newer versions to only appear to be vulnerable, and the proof of concept still worked.
Considering how easy it was to test this vulnerability, it indicates that those other sources are not doing any testing before making claims as to whether vulnerabilities have been fixed. That is pretty significant limit of the usefulness of their data and getting accurate information is one the important benefits of using our data instead, but one that people likely wouldn’t know about if we were not mentioning it since those sources don’t have a disclaimer that they don’t really know if their data is accurate. It certainly doesn’t help when the public is making further inaccurate claims about other data sources, like in the review mentioned earlier.
At that point we went to look at the changes made in the version that this was supposed to be fixed in, but it didn’t really giving us any idea what might be wrong.
We then looked at the HTML code being served when requesting the proof of concept URL. From that we could see that at least part of the issue is that value of the GET input “rm_form_id” from the proof of concept URL was being output numerous times on the page.
In looking at the underlying code form the plugin we found that one of those instances was generated by the following code:
var loc = "?page=rm_field_add&rm_form_id=<?php echo $data->form_id; ?>&rm_form_page_no=" + curr_form_page + "&rm_field_type";
So at least part of the issue was that the value of $data->form_id was not validated or sanitized and then being output without being escaped. Trying to trace through the code to find where that value came in to the plugin was a bit difficult. That wasn’t really all that surprising once we noticed that the developer was CMSHelpLive, which is company that doesn’t really have a great concern for security (even of their own websites despite offering security services) or for code quality.
We eventual tracked down were the value is brought in to the plugin, it was one three instances of the following line in the file /admin/controllers/class_rm_field_controller.php:
$data->form_id = $request->req['rm_form_id'];
Restricting the value being brought in to integers removed all the instances of cross-site scripting (XSS). Prior to that there were a couple of instances of the XSS that might have come from data passed through SQL statements, but we weren’t able to easily tell where they were coming from.
After we notified the developer they released version 184.108.40.206, which fixes this by restricting the value $request->req[‘rm_form_id’] to an integer by adding the following line to function manage() in the file /admin/controllers/class_rm_field_controller.php: