The great open-source bazaar. This is the idea to bring as many vendors (open source developers) under one roof (or repository) to share their wares with whoever may be interested. This sounds well and good, but this bazaar has one big secret. Look closely at some of the offerings and you may expose concerns about safety and security for the users.
Many of the vendors appreciate the help, some seek it out. They embrace the openness of open source and are quick to improve the quality of their offerings. There is an exception to this, the unsettling number of booths that have been long abandoned. Dust is collecting on their neglected products which are still offered at no cost but also with no support and no one around to make a fix when a flaw is pointed out. This unseen abandonment problem is a dark side of open source's great bazaar.
Enter Patchstack's "Last Patch". A short series of blog posts where we will be discussing abandoned open source projects found in the WordPress.org plugin repository. The process will be to identify abandoned projects with public security bugs reported in them. We will then take the time to review the bug, write an example patch, and share what was learned.
We do not fault the developers for the lack of a patch. The honest truth is some open source projects get abandoned. Life's priorities can change. The project's developers have moved on and that is OK. Starting an open-source project is not a life sentence of unpaid servitude.
The goal of these posts is to do a little good, and share what was learned. So that other developers can see what effort is needed to write security patches, to make something good out of an unfortunate event. With that said, today let's talk about the plugin: account-manager-woocommerce.
The account-manager-woocommerce plugin was officially abandoned by its developers on June 1st, 2022. This was clearly communicated by the developers in a support post. I applaud these developers for their honesty and transparency.
The only problem is, a security bug was found and publicly reported in the plugin months later in October 2022. The Broken access control in Account Manager for Woocommerce bug could lead to the personal identifying information of users being leaked. This is not a critical bug by any means, but it is still a good candidate for one last patch.
TL;DR Just Share the Patch
For site owners
Open the account-manager-woocommerce/helper/class-zacctmgr-core-admin.php file and look for the ajax_get_eligible_managers() function declared on lines 1332-1354
Change this function to look like the following:
1332 public function ajax_get_eligible_managers() {
1333 $roles = [];
1334
1335 /**********
1336 * Last Patch provided by Patchstack (RR)
1337 ***********/
1338 if (!current_user_can('list_users')) { wp_die('Current user can not list users'); }
1339 /**********
1340 * End Last Patch
1341 **********/
1342 if ( isset( $_REQUEST['roles'] ) && $_REQUEST['roles'] != '' ) {
1343 $roles = explode( ',', $_REQUEST['roles'] );
1344 }
1345
1346 $users = zacctmgr_get_em_users( $roles );
1347 $data = [];
1348
1349 if ( $users && count( $users ) > 0 ) {
1350 foreach ( $users as $user ) {
1351 $data[] = [
1352 'ID' => $user->ID,
1353 'first_name' => $user->first_name,
1354 'last_name' => $user->last_name
1355 ];
1356 }
1357 }
1358
1359 exit( json_encode( $data ) );
1360 wp_die();
1361 }For hosting providers
Not all security bugs can be protected via a WAF. In this case, a WAF rule would risk breaking the functionality of the plugin. So no WAF rule will be provided.
Read on to understand why.
Last Patch: account-manager-woocommerce
Security bugs reported through the Patchstack Alliance do not have their proof of concepts published by Patchstack directly. The security researchers reporting bugs through the alliance are free to publish their findings, but in this case, they chose not to.
So … I need to put in a little more effort, and I will not be sharing with you any proof of concepts this time around.
Finding the insecure code
The only information I have to go off of from the description is "Broken Access Control", "Requires subscriber or higher role", and a little bird told me that attackers could "export of sensitive information (user id, first name, last name)"
This was enough to point me in the right direction.
The "Requires subscriber or higher role" is a tell-tail sign of a WordPress AJAX endpoint that was created without Authorization checks. So I searched for an AJAX endpoint that exported user IDs, first names, and last names.
Finding what to patch
I found exactly what I was looking for sooner than expected. An AJAX endpoint that calls a function named "ajax_get_eligible_managers()" which returned a JSON blob of user IDs, first name, and last name.
This function is created in the file: helper/class-zacctmgr-core-admin.php on lines 1332-1354
1332 public function ajax_get_eligible_managers() {
1333 $roles = [];
1334
1335 if ( isset( $_REQUEST['roles'] ) && $_REQUEST['roles'] != '' ) {
1336 $roles = explode( ',', $_REQUEST['roles'] );
1337 }
1338
1339 $users = zacctmgr_get_em_users( $roles );
1340 $data = [];
1341
1342 if ( $users && count( $users ) > 0 ) {
1343 foreach ( $users as $user ) {
1344 $data[] = [
1345 'ID' => $user->ID,
1346 'first_name' => $user->first_name,
1347 'last_name' => $user->last_name
1348 ];
1349 }
1350 }
1351
1352 exit( json_encode( $data ) );
1353 wp_die();
1354 }Deciphering the code
The ajax_get_eligible_managers() function's purpose is to return a list of names of users who are shop managers (which includes administrators.) This is a simple function that returns the values as a JSON blob. Likely this is used somewhere to fill out a form field or table in the admin interface as it is only available to logged in users.
The problem is, the function lacks authorization checks. So it is available to any user (including subscribers, or authors.) to get a list of administrator and shop manager users.
It really sounds like a very low risk security bug. The impact is only the leaking of PII (personally identifiable information) of a limited set of users. But, privacy is a serious concern so, let's fix it.
Writing the patch
It is common to find AJAX endpoints being created and lacking authorization checks. This security bug is a great reminder that when you create AJAX endpoints in WordPress, you should always check authorization before performing sensitive tasks.
The patch is simple. To add an authorization check, we use current_user_can() with the appropriate capability to check. In this case, I felt that confirming that the current user can "list_users" made the most sense.
I added this check with just one line of code.
if (!current_user_can('list_users')) { wp_die('Current user can not list users'); }Here it is within the ajax_get_eligible_managers() function.
1332 public function ajax_get_eligible_managers() {
1333 $roles = [];
1334
1335 /**********
1336 * Last Patch provided by Patchstack (RR)
1337 ***********/
1338 if (!current_user_can('list_users')) { wp_die('Current user can not list users'); }
1339 /**********
1340 * End Last Patch
1341 **********/
1342 if ( isset( $_REQUEST['roles'] ) && $_REQUEST['roles'] != '' ) {
1343 $roles = explode( ',', $_REQUEST['roles'] );
1344 }
1345
1346 $users = zacctmgr_get_em_users( $roles );
1347 $data = [];
1348
1349 if ( $users && count( $users ) > 0 ) {
1350 foreach ( $users as $user ) {
1351 $data[] = [
1352 'ID' => $user->ID,
1353 'first_name' => $user->first_name,
1354 'last_name' => $user->last_name
1355 ];
1356 }
1357 }
1358
1359 exit( json_encode( $data ) );
1360 wp_die();
1361 }Writing the WAF rule
This security bug was caused by a lack of authorization checks. Which makes it a bad candidate for a web application firewall (WAF) rule. The patch is best written within WordPress so we can run a capability check, web servers like Apache, Nginx, etc… simply do not understand the context of user roles within the web application. Therefore a firewall rule can not be written at the web server layer without risking breaking functionality.
This is a reminder that not all security bugs can be patched with a WAF.
Conclusions
We learned the importance of adding authorization checks to AJAX endpoints. It is important to protect your AJAX API endpoints that perform sensitive tasks. This can be done quickly with a single call to current_user_can() for WordPress websites.
I also hopefully showed you that even the smallest and lower significance security bugs deserve a patch. This plugin is still active on WordPress.org, but it is clearly not supported. The developer has made this clear with their post about no longer supporting the plugin, and the point is driven home because this bug was not addressed. If one of the plugins you rely on has a security bug, even an insignificant one that is not been patched. That may be a sign the developer is not interested in improving their offerings, and you should keep looking for other vendors who care more. After all, it is a great bazaar full of many open source projects. You will find one that cares if you continue to look.
