Unspecified vulnerability in the MailPoet Newsletters(wysija-newsletters) plugin before 2.6.8 for WordPress has unspecified impact and attack vectors

TL;DR: CVE-2014-4726 is the same vulnerability than CVE-2014-4725, due to an incorrect fix.


The vulnerability is fairly easy to spot in the diff:

diff --git a/helpers/back.php b/helpers/back.php
index a6a090f..57beedb 100644
--- a/helpers/back.php
+++ b/helpers/back.php
@@ -178,9 +178,9 @@ class WYSIJA_help_back extends WYSIJA_help{
      * @return boolean
     function verify_capability(){
-        if( isset( $_REQUEST['page'] ) && substr( $_REQUEST['page'] ,0 ,7 ) == 'wysija_' ){
+        if( isset( $_GET['page'] ) && substr( $_GET['page'] ,0 ,7 ) == 'wysija_' ){

-            switch( $_REQUEST['page'] ){
+            switch( $_GET['page'] ){
                 case 'wysija_campaigns':
                     $role_needed = 'wysija_newsletters';

So we see that the only thing that changed it the usage of $_GET global rather than $_REQUEST. This behaviour is really of interest since $_REQUEST is still widely in use in old codebases (like 90% of the PHP applications?). In the default configuration of PHP, the POST values are overwriting GET ones with the same name (see variables_order). It can lead to some logic errors.

After some researches, it seems that the version 2.6.7 fixed an unauthenticated arbitrary file upload. From here, we can suppose that the fix was incomplete. Let’s diff the 2.6.6 and the 2.6.7:

diff --git a/helpers/back.php b/helpers/back.php
index 55d5dc0..a6a090f 100644
--- a/helpers/back.php
+++ b/helpers/back.php
@@ -46,6 +46,7 @@ class WYSIJA_help_back extends WYSIJA_help{

             if(WYSIJA_ITF)  {
+                add_action('admin_init', array( $this , 'verify_capability'),1);
                 add_action('admin_init', array($this->controller, 'main'));
@@ -172,6 +173,43 @@ class WYSIJA_help_back extends WYSIJA_help{

+     * this function will check the role of the user executing the action, if it's called from another
+     * WordPress admin page than page.php for instance admin-post.php
+     * @return boolean
+     */
+    function verify_capability(){
+        if( isset( $_REQUEST['page'] ) && substr( $_REQUEST['page'] ,0 ,7 ) == 'wysija_' ){
+            switch( $_REQUEST['page'] ){
+                case 'wysija_campaigns':
+                    $role_needed = 'wysija_newsletters';
+                    break;
+                case 'wysija_subscribers':
+                    $role_needed = 'wysija_subscribers';
+                    break;
+                case 'wysija_config':
+                    $role_needed = 'wysija_config';
+                    break;
+                case 'wysija_statistics':
+                    $role_needed = 'wysija_stats_dashboard';
+                    break;
+                default:
+                    $role_needed = 'switch_themes';
+            }
+            if( current_user_can( $role_needed ) ){
+                return true;
+            } else{
+                die( 'You are not allowed here.' );
+            }
+        }else{
+            // this is not a wysija interface/action we can let it pass
+            return true;
+        }
+    }
+    /**
      * translatable strings need to be not loaded to early, this is why we put them ina separate function
      * @global type $wysija_installing

The lack of access restriction was allowing a malicious user to use any functionality of the plugin (themeupload action), and thus uploading arbitrary files on the target by creating a fake theme zip (basically, a style.css file in a zip was enough to make it believe it was a valid theme). The theme will be created inside wp-content/uploads/wysija/themes/{name}/ by extracting the zip file. The target was redirecting you via a 302 in case of success.

They added a call to verify_capability call for the admin_init hook. I think they thought that it would prevent any non-admin user to trigger the hooks but since any call to wp-admin/admin-post.php will trigger it even if the user is not authenticated, it would have required to check if he has at least one administrator right (using current_user_can(), that’s what they do in the patch).

Wrong fix

So why issue a fix in the 2.6.8 if the 2.6.7 patch seems to be correct? Wordpress uses only $_GET to identify the page we’re on but the plugin will only try to check our permission by using && substr( $_REQUEST['page'] ,0 ,7 ) == 'wysija_'. POST values overriding GET values, we can avoid the check just by adding a page POST field with a value which is not starting by wysija_ , permitting us to upload an arbitrary file, as in CVE-2014-4725.

Metasploit module

The Metasploit module already integrates this (cheers firefart), by adding, as we could expect, the page field in the POST payload. However, if you want to check if it’s vulnerable or not without trying to exploit it and the readme.txt (that’s what the module is doing) but the access is denied, there’s still a way.

Version detection

Just subscribe to a newsletter and wait to receive a message from the owner of the website. Depending of the values of the X-PHP-Script header (“clean” or not), it will be possible to tell if the version in use is more or less than 2.6.9, without probing too much the target. The related diff is below:

diff --git a/helpers/mailer.php b/helpers/mailer.php
index ac2686d..f85d553 100644
--- a/helpers/mailer.php
+++ b/helpers/mailer.php
@@ -190,6 +190,13 @@ class WYSIJA_help_mailer extends acymailingPHPMailer {

 	function send(){
+			// prevent user/script details being exposed in X-PHP-Script header
+			$oldphpself = $_SERVER['PHP_SELF'];
+			$oldremoteaddr = $_SERVER['REMOTE_ADDR'];
+			$_SERVER['PHP_SELF'] = "/";
 				$this->AltBody = $this->textVersion($this->Body,true);
@@ -271,6 +278,11 @@ class WYSIJA_help_mailer extends acymailingPHPMailer {

+			// restore obfuscated server variables
+			$_SERVER['PHP_SELF'] = $oldphpself;
+			$_SERVER['REMOTE_ADDR'] = $oldremoteaddr;
 			return $result;
 	function load($email_id){