Insecure WordPress Security Plugin Closed on Plugin Directory and Then Reopened After Wrong Security Changes Applied
On Tuesday, the 60,000+ install WordPress security plugin WP Encryption was removed from the WordPress Plugin Directory without any explanation given for the closure:
After being alerted to that, we checked the plugin for any easy to find vulnerabilities that hackers might be interested in exploiting. As we have noted recently, hackers look to have been targeting closed plugins with unfixed vulnerabilities (including one where WordPress is stopping people from getting a fix). What we found is that the plugin, despite being a security plugin, is lacking basic security. We will detail examples of that later in the post.
On Wednesday, the developer submitted an update that applied various security changes to the plugin that don’t make sense. We will also detail examples of that later in the post. Those changes didn’t address any of the rather glaring missing security we had noticed.
Yesterday the plugin was reopened without any of the glaring security issues being fixed. We don’t know why the plugin was closed as there is no discussion on the support forum for the plugin about the closure and there is no changelog for the new version of the plugin that was submitted after it was closed.
What we do know is that in the past plugins that have been closed for security vulnerabilities have been reopened without the vulnerabilities even being fixed. Our efforts to try help the team to avoid that type of situation happening again have been met with hostility and lies.
We would recommend against using the plugin unless the developer can show that they have gotten a handle on securing the plugin. That isn’t something we should have to say about a security plugin, but unfortunately plenty of security providers don’t seem to care enough to secure their own plugins. And unfortunately, the team running the plugin directory doesn’t care about that either. (Not helping that is that the team’s Security Reviewer is the chief security officer of a plugin developer with bad security.)
Plugin Security Scorecard Grade Seems Accurate
For those looking for plugins where the developer is handling security well, our Plugin Security Scorecard is an attempt to provide an incomplete view of that. This plugin got an F grade from the tool when someone last checked it. Considering the insecurity of the plugin, that seems like a good grade for the plugin, even though the score wasn’t based on that insecurity as it couldn’t be easily checked for in an automated fashion.
Fake Five for the Future Pledges from Developer?
Before we get in to discussing the plugin’s code, there was something else we noticed while looking into this. The developer seems to be making Five for the Future Pledges that are not real. The “Support Rep” account for the plugin claims to contribute “25 hours per week to the following teams: Core and Support.” But the accounts recent activity consists only posting on the plugin’s support forum:
Activity on the Core and Support teams should have activity that would be shown.
The other account associated with the plugin claims to contribute “5 hours per week to the following teams: Community, Core, and Meta.” Their recent activity only consists of activity taken related to the plugin and not to any of the teams mentioned:
Activity on the Core and Meta teams should have activity that would be shown.
Considering that trust is critical to security, a dishonest developer seems like a big red flag. It does match up with a security plugin being so insecure.
(Fake five for the future pledges seem to be very common.)
Basic Security Checks Missing
If you are going to vet the security of a WordPress plugin at all, one obvious thing to check would be that AJAX accessible functions have a couple of basic security checks. Usually, those should contain a capability check to make sure that only the intended users have access and a nonce check to prevent cross-site request forgery (CSRF).
In the file /admin/le_ajax.php, there is an AJAX accessible function lacking a needed capability check:
30 31 32 33 34 35 36 37 38 39 | public function wple_ajx_verify_http() { if (isset($_POST['nc'])) { if (!wp_verify_nonce(sanitize_text_field(wp_unslash($_POST['nc'])), 'verifyhttprecords')) { exit('Unauthorized'); } $domain = str_ireplace(array('https://', 'http://'), '', site_url()); |
In the same file there are multiple AJAX accessible function missing a needed nonce check, including this one:
359 360 361 362 363 364 365 366 | public function wple_ignore_backup_suggest() { if (!current_user_can('manage_options')) { exit(); } update_option('wple_backup_suggested', true); |
And this one:
245 246 247 248 249 250 251 252 253 254 255 256 257 258 259 260 261 262 263 264 265 266 267 268 269 | public function wple_validate_nocp_ssl() { if (!current_user_can('manage_options')) { exit('Unauthorized'); } $basedomain = str_ireplace(array('http://', 'https://'), array('', ''), addslashes(site_url())); //4.7 if (false !== stripos($basedomain, '/')) { $basedomain = substr($basedomain, 0, stripos($basedomain, '/')); } $client = WPLE_Trait::wple_verify_ssl($basedomain); if ($client || is_ssl()) { $reverter = uniqid('wple'); $savedopts = get_option('wple_opts'); $savedopts['force_ssl'] = 1; $savedopts['revertnonce'] = $reverter; ///WPLE_Trait::wple_send_reverter_secret($reverter); update_option('wple_opts', $savedopts); |
That is how the code looks after the update was made.
Another place that is obvious to check is things registered to run admin_init, which makes them accessible to even those not logged in to WordPress. So you can end up with code that is only intended to accessed by Administrators that can be accessed by anyone.
In the file /admin/le_admin.php, the function wple_basic_get_requests() is registered that way and lacks either a capability or a nonce check despite being needed:
1275 1276 1277 1278 1279 1280 1281 1282 1283 1284 1285 1286 1287 1288 1289 1290 | public function wple_basic_get_requests() { //since 5.1.0 if ( isset( $_GET['restart'] ) ) { //click to restart from beginning delete_option( 'wple_ssl_screen' ); wp_redirect( admin_url( '/admin.php?page=wp_encryption' ), 302 ); exit; } if ( isset( $_GET['force_complete'] ) ) { //Forced SSL completion flag update_option( 'wple_ssl_screen', 'success' ); update_option( 'wple_backend', 1 ); WPLE_Trait::clear_all_renewal_crons( true ); wp_redirect( admin_url( '/admin.php?page=wp_encryption' ), 302 ); exit; } |
Unnecessary and Possibly Harmful Changes
Looking at various security changes applied to the plugin by the developer, plenty don’t make sense from a security perspective. For example, in the file /classes/le-security.php a regular expression check was changed from this:
76 | if (preg_match('/(wp-comments-post)/', $_SERVER['REQUEST_URI']) === 0 && !empty($_REQUEST['author'])) { |
to this:
76 | if (preg_match('/(wp-comments-post)/', sanitize_text_field($_SERVER['REQUEST_URI'])) === 0 && !empty($_REQUEST['author'])) { |
The change sanitizes user input using a sanitization function for a text field, sanitize_text_field(), for something that isn’t a text field. That isn’t as much of a problem as the fact that the sanitization done doesn’t make sense. At best, it doesn’t do anything as the value check to contains another value, so something malicious in the user input doesn’t have any impact. At worst, that kind of code could actually allow something to happen that isn’t intended, as the sanitization could remove something that was supposed to be detected or not detected.
Unnecessary sanitization was also added to the file /classes/le-subdir-challenge.php, where a nonce verification check was changed from this:
200 | if ( !wp_verify_nonce( $_GET['nc'], 'subdir_ch' ) || !current_user_can( 'manage_options' ) ) { |
to this
204 | if ( !wp_verify_nonce( sanitize_text_field( wp_unslash( $_GET['nc'] ) ), 'subdir_ch' ) || !current_user_can( 'manage_options' ) ) { |
Like the other code, sanitize_text_field() is used on a value that isn’t a text field. There also isn’t any need to unslash (using wp_unslash()) a nonce value.
If you look at the function, the value is passed to, wp_verify_nonce(), that only compares the value passed to another value using the PHP function hash_equals().
This looks to be the result of the developer making changes based on the long known to be problematic WordPress Coding Standards (WPCS) tool. The developer of which has been very successful at misleading people as to the security value provided by it and extracting money from the WordPress community while not fixing known problems with it.