Patching an XSS Security Bug in "add-comments" Plugin

Published 22 December 2022
Updated 24 July 2023
Robert Rowley
Author at Patchstack
Table of Contents

Accepting your mistakes. The human experience is full of mistakes, failures, and folly. I would say this is "the truth" but this statement itself may be wrong, and I accept that .. but I'm saying this first to make my next point.

Mistakes make the world go round. We can observe a fault and learn from it, improving ourselves in the process. Humans are social creatures too, allowing us to learn from mistakes other make too.

The worst mistake any human can make is to assume infallibility. Assuming you are incapable of making a mistake is to declare yourself above improvement. Claiming you are beyond the humanity of making mistakes, places you in a precarious position of your own making.

Mistakes are what this blog post is about. Patchstack's "Last Patch" is a short series of blog posts where we discuss and patch unpatched security bugs in open source projects.

We do not fault the developers for the lack of a patch. We believe starting an open source project is not a life sentence of providing fixes and support at no cost or remuneration. We understand all people make mistakes, and that most open source project developers are people like us all. People who make mistakes, people in the process of improving, and people who simply need support and help.

We want to help. Help the site owners, help developers, and help hosting providers with rules to protect their customers.

Today's "last patch" is for the add-comments plugin, last updated 10 years ago. Everything seemed fine until November 2022, when an XSS security bug was reported in the plugin. Then, the exploit's proof of concept was publicly shared a few weeks later. The plugin was removed from the WordPress.org plugin repository on the 9th, for the safety of users.

In this post I will review the security bug (e.g... the mistake) that lead to this plugin's disablement. You will see how it could have been patched.

TL;DR Just Share the Patch

For site owners

Open the add-comments/add-comments.php file and find the addCOmments_options_page() function near line 90.

Modify the start of the function to look like the following:

/**********
**  Last Patch added by Patchstack (RR)
**********/
function addComments_options_page() { 

    if( $_POST['action'] == 'addcomments' ) {
        echo '<div class="updated settings-error">';
        $id = ( $_POST['post_or_page'] == 'page' ? $_POST['pages_list'] : $_POST['posts_list'] );
        $author = ( isset( $_POST['author_name'] ) ? sanitize_text_field($_POST['author_name']) : 'annonymous' );
        $email = ( isset( $_POST['author_email'] ) ? sanitize_email($_POST['author_email']) : get_bloginfo('admin_email' ) );
        $url = ( isset( $_POST['author_url'] ) ? sanitize_url($_POST['author_url']) : '' );
        $ip = ( isset( $_POST['author_ip'] ) ? $_POST['author_ip'] : '127.0.0.1' );
        echo addComments_add_comments( $id, $author, $email, $url, $ip, sanitize_text_field($_POST['comment']) ) 
        . ' comments added to ' . get_the_title( $id ) . '</div>';
    } ?>
/**********
**  End Last Patch
**********/

This will add sanitize_* functions to $_POST variables found on lines 95, 96, 97, and 99.

Once this change has been made the attempts to inject XSS payloads no longer work for any of the protected fields.

For hosting providers

There is no recommended WAF rule for this XSS bug. A WAF rule would block the request outright, it would be better to sanitize these inputs, making them safe to display.

Using a generic, and robust, XSS protection rule should detect and block these requests, but I would recommend the code based patch outlined above.

Last Patch: add-comments

Verifying the vulnerability

The proof of concept for this security bug was released by WPScan. Reviewing the example I was able to find the page to visit /wp-admin/options-general.php?page=addComments and the variables the XSS payload should exist in: comment.

Verifying this vulnerability was easy knowing the above. Visiting the page in question on my test WordPress site with add-comments installed showed me the "add comment" page, which is just a form to fill out. Using example XSS payloads in the add-comments form's fields required no special tools or programs, the XSS payloads just worked when entered in the comment field as well as the author_name field.

XSS injection points in add-comments

Once the payload is injected visiting any page where the malicious comment results in a javascript alert box popping up.

XSS payload executing in the browser

Finding what to patch

Now that we know the security bug exists and how to test for it. Let's look into the plugin's code. Luckily, this plugin is simple to understand as it only has one file: add-comments.php.

Digging through this one file's 162 lines of code, I found the relevant function on lines 90 - 101

Here is the relevant insecure code:

90    function addComments_options_page() { 
91        
92        if( $_POST['action'] == 'addcomments' ) {
93            echo '<div class="updated settings-error">';
94            $id = ( $_POST['post_or_page'] == 'page' ? $_POST['pages_list'] : $_POST['posts_list'] );
95            $author = ( isset( $_POST['author_name'] ) ? $_POST['author_name'] : 'annonymous' );
96            $email = ( isset( $_POST['author_email'] ) ? $_POST['author_email'] : get_bloginfo('admin_email' ) );
97            $url = ( isset( $_POST['author_url'] ) ? $_POST['author_url'] : '' );
98            $ip = ( isset( $_POST['author_ip'] ) ? $_POST['author_ip'] : '127.0.0.1' );
99            echo addComments_add_comments( $id, $author, $email, $url, $ip, $_POST['comment'] ) 
100            . ' comments added to ' . get_the_title( $id ) . '</div>';
101        } ?>

Can you spot the mistake?

Deciphering the code

The security bug is caused by using $_POST variable values without sanitizing them first. When the developer sends these values to addComments_add_comments() on line 99, this function is used to store the unsanitized values as WordPress comments.

Writing the patch

Luckily, we can keep this simple plugin simple and still make it secure.

All we need to do is remember to sanitize the $_POST variable values before sending them to addComments_add_comments().

90    function addComments_options_page() { 
91        
92        if( $_POST['action'] == 'addcomments' ) {
93            echo '<div class="updated settings-error">';
94            $id = ( $_POST['post_or_page'] == 'page' ? $_POST['pages_list'] : $_POST['posts_list'] );
95            $author = ( isset( $_POST['author_name'] ) ? sanitize_text_field($_POST['author_name']) : 'annonymous' );
96            $email = ( isset( $_POST['author_email'] ) ? sanitize_email($_POST['author_email']) : get_bloginfo('admin_email' ) );
97            $url = ( isset( $_POST['author_url'] ) ? sanitize_url($_POST['author_url']) : '' );
98            $ip = ( isset( $_POST['author_ip'] ) ? $_POST['author_ip'] : '127.0.0.1' );
99            echo addComments_add_comments( $id, $author, $email, $url, $ip, sanitize_text_field($_POST['comment']) ) 
100            . ' comments added to ' . get_the_title( $id ) . '</div>';
101        } ?>

You can see on lines 95, 96, 97, and 99 I add various sanitize functions as we set the values to $author_name, $email, $url, and the $_POST['comment'] variable as it is being sent to addComments_add_comments().

Writing the WAF rule

Web Application Firewalls (WAF) are best suited for blocking malicious requests, cross site scripting is best handled by sanitizing the values. For this reason, there is no recommended WAF rule for this Last Patch. Instead you should patch the code itself.

Conclusions

We learned in this post that the most prevalent WordPress (dare I say most prevalent Web Application) security bug Cross Site Scripting (XSS) can be addressed with a single function call. Simply sanitize the value of user inputs before storing them in the database. This is something every WordPress or Web Application developer needs to do to avoid this mistake.

This most prevalent security bug is best to be addressed in the application itself. It is a mistake to assume the framework or a web application firewall will take care of this for the developer. Instead, we can think that the developer owns the data and can be responsible when storing user inputs.

We also learned that mistakes are OK to make. Mistakes are how we improve ourselves, and fixing mistakes in code is how developers become better developers. The utmost mistake is assuming you will never make one.

Be humble, accept your mistakes and grow as a person.

The latest in Security Advice

Looks like your browser is blocking our support chat widget. Turn off adblockers and reload the page.
crossmenu