Patching a Stored XSS Bug In the "tinymce-custom-styles" Plugin

Welcome back to Patchstack's "Last Patch". This is a special episode, normally these blog posts are lessons in defensive coding tactics using a plugin that has already been disabled due to abandonment.

However, in this post I will share with you the happy story about a plugin author that was able to apply the recommended patch, remediating the reported security vulnerability and avoiding the plugin's disablement! This makes this the first "not the last"-last patch blog post.

What lead to this unique situation? An offer of help.

A short while ago, the Patchstack Alliance received a report on the plugin tinymce-custom-styles. The Alliance triage and review team was able to get in contact with the developer, Tim Reeves via his personal website. To our surprise, the developer's response was a touching note from Tim, asking for a new maintainer.

Here is some of what Tim said:

I am now 68 and retired, and all my time is taken up with other projects, while I still have the energy for them ...

... Disclosure of the problem is scheduled for 18.02.2023. After which I don't know if it will get removed from the repository ...

... Please someone take over maintaining this plugin, or it will get abandoned

After reading this, I reached out to Tim and offered to provide the patch to help avoid the removal of the plugin.

Here is what was patched, and why.

TL;DR - the patch

The patched version of tinymce-custom-styles (1.1.3) is available on the WordPress.org repository and can be upgraded through your websites directly. You can check out the changelog for TinyMCE Custom Styles for more details.

The changes in 1.1.3 can be viewed on the public trac pages. The patch consisted of ensuring user controlled inputs are properly sanitized before they are stored (in this case they were stored in wp_options; this is a database table of key/value pairs for miscellaneous WordPress options.) Then I had to properly escape the values when they were outputted to the browser.

Last Patch: tinymce-custom-styles

What happened? Tinymce-custom-styles was affected by an authenticated (Admin+) stored cross site scripting bug.

This bug carried a very low severity/risk. It requires an administrator user to enter the XSS payload into the plugin's settings, this means someone who already has full access and control of the site (administrators) could technically attack other users. Not ideal, so it deserves a patch.

Ultimately, this is more of a code clean up and improvement task rather than an emergency.

The best was how easy it was to address because WordPress core gives us the functions needed to sanitize inputs and escape outputs with minimal effort.

Verifying the exploit

The confirmation of this exploit was easy. The bug report described a form field in the plugin's settings screen which is vulnerable to XSS.

Submitting this form stores the XSS payload on the plugin's settings screen. Visiting this page in a browser will cause a javascript alert window with the browser's cookie data shown within it. This is more than sufficient to prove the exploitability.

Looking at the HTML source code is a nice way to see how this injection worked. It also provides us with important details for the next step. The name of the input field that we need to patch, tcs_cuslink.

Finding what to patch

Using the evidence we uncovered while verifying the exploit. I can now look at the plugin's source code for that input field being stored.

Looking for tcs_cuslink in the source code I identified it is being stored in the wp_options table without being sanitized, then later outputted to the browser without being escaped. Which makes for two things to patch.

Deciphering the code

Here are the lines where tcs_cuslink is found in the plugin's code:

<?
...
63     $http_path= content_url() . '/' . get_option('tcs_cuslink') . $style_name;
...
81     $server_side_path = WP_CONTENT_DIR . '/' . get_option('tcs_cuslink') . $style_name;
...
361    if ($_POST["tcs_cuslink"]  != get_option('tcs_cuslink'))  update_option("tcs_cuslink",  $_POST['tcs_cuslink']);
...
481        <p><input type="radio" name="tcs_locstyle" value="custom_directory" <?php if (get_option('tcs_locstyle') == "custom_directory") {?>checked="checked" <?php } ?> /> <?php printf(__('Use a custom directory (recommended) at %s/', TCS_TEXTDOMAIN), WP_CONTENT_DIR); ?><input size="30" type="text" name="tcs_cuslink" value="<?php echo get_option('tcs_cuslink'); ?>" />editor-style[-shared].css

We can see it is line 361 that stores the value of $_POST["tcs_cuslink"] to the wp_options table.

On lines 63, 81, and 481 the value of tcs_cuslink from the wp_options table is outputted.

Writing the patch

To sanitize the inputs, we update line 361 to add a call to sanitize_file_name() before the value is stored to wp_options

<?
...
361    if ($_POST["tcs_cuslink"] != get_option('tcs_cuslink')) update_option("tcs_cuslink", sanitize_file_name($_POST['tcs_cuslink']));

Then to address the outputs on lines 63, 81, and 481 I escape the outputs using esc_attr()

<?
...
63     $http_path= content_url() . '/' . esc_attr(get_option('tcs_cuslink')) . $style_name;
...
81      $server_side_path = WP_CONTENT_DIR . '/' . esc_attr(get_option('tcs_cuslink')) . $style_name;
...
481          <p><input type="radio" name="tcs_locstyle" value="custom_directory" <?php if (get_option('tcs_locstyle') == "custom_directory") {?>checked="checked" <?php } ?> /> <?php printf(__('Use a custom directory (recommended) at %s/', TCS_TEXTDOMAIN), WP_CONTENT_DIR); ?><input size="30" type="text" name="tcs_cuslink" value="<?php echo esc_attr(get_option('tcs_cuslink')); ?>" />editor-style[-shared].css

The hardest part of this cleanup was choosing the right sanitization or escape function for the use case. In the code above, it appears the value of tcs_cuslink is a file's path. Which is why I chose sanitize_file_name(). When the variable was being outputted, it appears it is used within an HTML tag, which is why I chose to use esc_attr(). Doing both is correct, to ensure the code is sanitizing on input and escaping on output.

Conclusions

In this post we showed a practical application of the phrase "sanitize every input and escape on output". Adding the functions was easy and choosing the right sanitation or escape function was straightforward.

I only made 6 small changes in tinymce-custom-styles to improve the security of the code. 4 examples were given above, but I added an additional 2 security-related updates just to be thorough. These 6 changes not only made the code more secure, but also saved the plugin from being disabled in the official WP.org repository.

I hope that more plugin developers learn to embrace secure development practices so that their projects may continue to thrive and the end user's sites remain secure.