The latest version of the WordPress plugin Download Monitor, which has 100,000+ installs, fixed a vulnerability, but the developer didn’t disclose that in the changelog. What makes the lack of disclosure stand out is that the developer had disclosed in the changelog for a recent version that they were fixing a security issue. The vulnerability had been attempted to be addressed in that previous version, but the fix was incomplete. As at least one of our customers was using the plugin, we checked on the fix and noticed it was incomplete. After we notified the developer of that, they fixed that up. The failed fix involved a failure to properly use the built-in security functionality of WordPress’ REST API, so it seems worth looking at what went wrong and how other developers can avoid that.

Until the latest version of the plugin, three REST API routes providing access to reports from the plugin were registered with the permission_callback set to “__return_true”:

register_rest_route(
	'download-monitor/v1',
	'/download_reports',
	array(
		'methods'             => 'GET',
		'callback'            => array( $this, 'rest_stats' ),
		'permission_callback' => '__return_true',
	)
);

That makes it accessible to even those not logged in to WordPress, despite access only being intended for certain logged-in users.

As we noted for our customers, when warning them the vulnerability hadn’t been properly fixed, the developer had tried to limit access by adding this code to the functions called to generate the reports:

check_ajax_referer( 'wp_rest' );
 
if ( ! isset( $_REQUEST['user_can_view_reports'] ) || ! (bool) $_REQUEST['user_can_view_reports'] || ! is_user_logged_in() ) {
	return array(
		'stats'  => array(),
		'offset' => 0,
		'done'   => true,
	);
}

That checks if someone is logged in to WordPress and has access to a nonce. The nonce being checked for is the standard REST nonce, which anyone logged in to WordPress has access to. So the change only limited access to those logged in to WordPress. Meaning that low-level WordPress users had access to data they were not intended to have access to.

The proper way to address that is by utilizing the permission_callback and checking for the right capability. In the latest version, they did that. The changes made included specifying that the function check_api_rights() is used for the permission callback:

register_rest_route(
	'download-monitor/v1',
	'/download_reports',
	array(
		'methods'             => 'GET',
		'callback'            => array( $this, 'rest_stats' ),
		'permission_callback' => array( $this, 'check_api_rights' ),
	)
);

That function though appears to be written by someone who doesn’t have a great grasp of how things are handled by WordPress, as they check for things they don’t need to check for, but they do check if the user has the “dlm_view_reports” capability:

public function check_api_rights( $request ) {
 
	if ( ! isset( $request['user_can_view_reports'] ) || ! (bool) $request['user_can_view_reports'] ||
		 ! is_user_logged_in() || ! current_user_can( 'dlm_view_reports' ) ) {
		return new WP_Error(
			'rest_forbidden_context',
			esc_html__( 'Sorry, you are not allowed to see data from this endpoint.', 'download-monitor' ),
			array( 'status' => rest_authorization_required_code() )
		);
	}
 
	return true;
}

If the request comes from someone without that capability, the processing of the request will not proceed further. There isn’t a need to check if the request is from someone logged in, is_user_logged_in(), as was done there.

What isn’t there is a nonce check, as was included in the previous code, as that nonce check will occur automatically if the permission_callback isn’t set to “__return_true”.

More information on the permission_callback can be found here.

You can see what REST API routes plugins are publicly available using a tool we recently released.