Awesome Motive’s Easy Digital Downloads is Still Lacking Basic Security Despite Contrary Claim by Patchstack

Most days we see what appears to be a hacker probing for the usage of a single WordPress plugin with a recently disclosed vulnerability through a single request for a file on each of our websites. Yesterday, we saw them doubling up both on the files they were requesting and the IP addresses being used. The plugin they were looking for was Easy Digital Downloads. It wasn’t hard to guess why, as Patchstack had disclosed how to exploit a serious vulnerability that had been fixed the day before. While reviewing this, we found that there are still security issues that run counter to a central claim made by Patchstack.

Before we get to that, it’s important to note who the developer of the plugin is. That is Awesome Motive. That would be the Awesome Motive that has a chief security officer (CSO) who is also the “security reviewer” on the team running the WordPress Plugin Directory. That would be the Awesome Motive that took two months to fix a publicly known vulnerability in a plugin with 3+ millions installs. They frequently acquire existing WordPress plugins, which is how they came to be the developer of this plugin. The vulnerability that was fixed was introduced six months after they had acquired the plugin.

Patchstack made this claim about the vulnerability:

This vulnerability makes it possible for any user, regardless of their current authentication and authorization, to execute any action registered with the prefix edd_.

And claimed that the issue had been resolved:

This issue occurs because we are able to call any action registered with the edd_ prefix. The patch can be seen here.

If you look at what they are describing as the patch for the vulnerability, it doesn’t address what they are claiming is the vulnerability. Instead, the change made fixes a vulnerability where anyone could reset a user’s password, which relied in part on anyone being able to execute any action registered with the prefix edd_.

In the new version, it is still possible for anyone to execute any action registered with the prefix edd_. That occurs in the file /includes/actions.php with this code:

function edd_get_actions() {
	$key = ! empty( $_GET['edd_action'] ) ? sanitize_key( $_GET['edd_action'] ) : false;
 
	$is_delayed_action = edd_is_delayed_action( $key );
 
	if ( $is_delayed_action ) {
		return;
	}
 
	if ( ! empty( $key ) ) {
		do_action( "edd_{$key}" , $_GET );
	}
}
add_action( 'init', 'edd_get_actions' );

That file wasn’t changed in the version that Patchstack claims fixed that. We can’t come up with a plausible explain for why they are claiming otherwise.

That access to those functions isn’t a vulnerability, but it could be seen as a bad design choice. The problem is that plenty of code that is accessible through that still isn’t properly secured, leading to vulnerabilities.

Lack of Capabilities Check

As an example of the insecurity that runs counter to Patchstack’s claim, in the file /includes/admin/import/import-functions.php the function edd_do_ajax_import_file_upload(), which allows uploading import files, is registered to be accessible through what Patchstack was referencing:

91	add_action( 'edd_upload_import_file', 'edd_do_ajax_import_file_upload' );

The function includes a nonce check to prevent cross-site request forgery (CSRF), but doesn’t include a capabilities check to limit who has access:

function edd_do_ajax_import_file_upload() {
 
	if ( ! function_exists( 'wp_handle_upload' ) ) {
		require_once( ABSPATH . 'wp-admin/includes/file.php' );
	}
 
	require_once EDD_PLUGIN_DIR . 'includes/admin/import/class-batch-import.php';
 
	if( ! wp_verify_nonce( $_REQUEST['edd_ajax_import'], 'edd_ajax_import' ) ) {
		wp_send_json_error( array( 'error' => __( 'Nonce verification failed', 'easy-digital-downloads' ) ) );
	}
 
	if( empty( $_POST['edd-import-class'] ) ) {
		wp_send_json_error( array( 'error' => __( 'Missing import parameters. Import class must be specified.', 'easy-digital-downloads' ), 'request' => $_REQUEST ) );
	}
 
	if( empty( $_FILES['edd-import-file'] ) ) {
		wp_send_json_error( array( 'error' => __( 'Missing import file. Please provide an import file.', 'easy-digital-downloads' ), 'request' => $_REQUEST ) );
	}
 
	if ( empty( $_FILES['edd-import-file']['type'] ) || ! in_array( strtolower( $_FILES['edd-import-file']['type'] ), edd_importer_accepted_mime_types(), true ) ) {
		wp_send_json_error( array( 'error' => __( 'The file you uploaded does not appear to be a CSV file.', 'easy-digital-downloads' ), 'request' => $_REQUEST ) );
	}
 
	if( ! file_exists( $_FILES['edd-import-file']['tmp_name'] ) ) {
		wp_send_json_error( array( 'error' => __( 'Something went wrong during the upload process, please try again.', 'easy-digital-downloads' ), 'request' => $_REQUEST ) );
	}
 
	// Let WordPress import the file. We will remove it after import is complete
	$import_file = wp_handle_upload( $_FILES['edd-import-file'], array( 'test_form' => false ) );

A nonce check will normally do the equivalent of a capabilities check, but the WordPress documentation is clear it shouldn’t be relied on for that:

Nonces should never be relied on for authentication, authorization, or access control.

Part of the reason for that is that nonces sometimes are provided to those that shouldn’t have access to them, which has led to serious vulnerabilities.

That function is one of over 300 that are accessible in the way Patchstack mentioned. We found others that are insecure in a way that is a vulnerability and there are likely to be more issues.

Cross-Site Request Forgery (CSRF)

In the file /includes/admin/upgrades/upgrade-functions.php among the functions made accessible through the way Patchstack mentioned is edd_remove_refunded_sale_logs(), which “remove[s] sale logs from refunded orders”:

1248	add_action( 'edd_remove_refunded_sale_logs', 'edd_remove_refunded_sale_logs' );

That function does include a capabilities check, but lacks a nonce check so an attacker could cause someone who has the needed capability to take the actions without intending it:

function edd_remove_refunded_sale_logs() {
	$edd_logs = EDD()->debug_log;
 
	if ( ! current_user_can( 'manage_shop_settings' ) ) {
		wp_die( __( 'You do not have permission to do shop upgrades', 'easy-digital-downloads' ), __( 'Error', 'easy-digital-downloads' ), array( 'response' => 403 ) );
	}
 
	edd_set_time_limit();
 
	$step    = isset( $_GET['step']  ) ? absint( $_GET['step']  ) : 1;
	$total   = isset( $_GET['total'] ) ? absint( $_GET['total'] ) : edd_count_payments()->refunded;
 
	$refunds = edd_get_payments( array(
		'status' => 'refunded',
		'number' => 20,
		'page'   => $step
	) );
 
	if ( ! empty( $refunds ) ) {
 
		// Refunded Payments found so process them
		foreach ( $refunds as $refund ) {
 
			// Remove related sale log entries
			$edd_logs->delete_logs(
				null,
				'sale',
				array(
					array(
						'key'   => '_edd_log_payment_id',
						'value' => $refund->ID
					)
				)
			);
		}

That is a cross-site request forgery (CSRF) vulnerability.

We contacted the developer yesterday to notify them that they still have worked to do and offered to help them to do that. They responded today that they would be addressing this in the next version of the plugin.