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.
Once the payload is injected visiting any page where the malicious comment results in a javascript alert box popping up.
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.
