One of the ways we are very different from the other sources of vulnerability data on WordPress plugins that we are aware of, is that we actually review each reported vulnerability when preparing to add it to our data. That means that we can provide higher quality data than other sources, so you don’t get told among other things that an unfixed vulnerability has been fixed and don’t get warned about vulnerabilities that don’t exist as you do with other sources. That also means we have looked at numerous reports and we have seen the good and the bad of handling of security in WordPress plugins. Something we ran across recently managed to surprise us and is a good reminder that even popular plugins can have glaring security failures that are left in place for years.

The plugin Form Maker (also referred to as Form Maker by WD) as you might guess from the name handles forms on website and is fairly popular, with 90,000+ active installs according to wordpress.org. Last Wednesday a new version, 1.11.2 was released, which had as one of its changelog entries “Fixed: Security issue”. After noticing that the new version was indicated to have a security fix we looked around to see if there had been a report released on the issue, when we didn’t find any we started to take a look at the changes made in that version to see if the fixed security issue was a vulnerability that we should be adding to our data set.

What looked to be the relevant change was running a variable through the escaping function esc_html() in the file /admin/views/FMViewSubmissions_fm.php:

627

$temp[$g]->element_value = esc_html($temp[$g]->element_value);

Based on the name of file it seemed possible that this was related the page that would show the contents of submitted forms. It seems hard to believe that the plugin wouldn’t be properly handling the security of user input coming through forms submissions. Not only is that very basic when it comes to security, but if the developer had missed doing that you would think that for plugin used on some many websites someone else would have checked on that.

We then tried to see if there was previously a persistent cross-site scripting (XSS) vulnerability that change indicates that there might have been. We were immediately able to cause a persistent XSS to occur, though with further checking we found that we had gotten a bit lucky.

What we found is that if you changed the value of a selected radio button in a form to JavaScript code that code be saved unchanged in the database as part of the form submitting and the JavaScript would be output and run on the Submissions page in the WordPress admin area. The escaping added in version 1.11.2 prevents the JavaScript code from running.

When we went to do further testing we found that when we tried to submit JavaScript code in text input in a form that it was being saved to the database in escaped form and was still escaped when out Submissions page in the WordPress admin area. Clearly there was something different happening in the plugin’s processing of those two types of inputs.

In what might explain why the vulnerability had been in the plugin and had been there for a long time we had a bit of trouble figuring out where the code for saving form submissions that was involved here was located (it was located in the function save_db() in the file /frontend/models/FMModelForm_maker.php ). Once we found that the reason for this discrepancy between the handling of those inputs was obvious.

Here is the line for saving a radio input:

$value = isset($_POST['wdform_'.$i."_element".$id]) ? $_POST['wdform_'.$i."_element".$id] : "";

By comparison here is the text input’s line:

$value = isset($_POST['wdform_'.$i."_element".$id]) ? esc_html($_POST['wdform_'.$i."_element".$id]) : "";

In that the value is passed esc_html(), unlike the radio input.

Going back through older versions we found that the escaping that occurs for some inputs was not there when the code was originally added, instead in occurred in June of 2014, so before that the other inputs were also available to use for persistent XSS. For the version that changed occurred in, 1.7.10,  the changelog entry is “security issue fixed”. Its odd that the others were not changed in the same way at the time and that the more recent fix did not involve doing that as they are still stored in the database in potentially unsafe way (if say, another plugin were to display their data without doing their own escaping). We have just sent a message to the developer suggesting they do that.

That previous incomplete fix seems like another indication that having more eyes review security changes could leave to further improvements. We do plenty of that type work now, but it could be made easier for us and others to do that. It would be particularly helpful if developers provided more information on security fixes, so that others could easily review them. Instead of something like this where we had to dig through the changes made in a new version to try to figure out what is going on. While it is way down the list of things WordPress could do to improve security, them putting in place a formal process where developers could submit information on security fixes and it could be reviewed by others looks like it could lead to better security.