Patching Remote Code Execution in the ‘member-hero’ Plugin

Welcome to Patchstack’s “Last Patch”. This is a short series of blog posts where we will be discussing and patching unpatched security bugs in open-source projects. With an initial focus on plugins found in the WordPress.org plugin repository

Today I will be discussing how to address an unauthenticated remote code execution vulnerability in the member-hero plugin.

We do not fault the developers for the lack of a patch. The member-hero plugin was actively developed for only 5 months until 2020-08. The developer was kind enough to also include a warning that the free plugin was no longer supported as of 2020-07. Users of this plugin had significant time to find a replacement.

Over a year after the notice the plugin was no longer supported, on 2022-03 the WordPress.org plugin repository removed the plugin due to this unpatched security issue.

Even a certainly abandoned plugin still has value. We can learn by reviewing what went wrong and what steps they could have taken to patch this security bug. In the process, we will learn and may be able to help other developers and hosting providers by sharing some steps they can take to protect their customers.

TL;DR Just share the patch

For site owners

The fix is a little hard to apply manually but you can patch this code by opening includes/class-memberhero-ajax.php

Add the following to the send_form() function found near line 105:

            /** 
           * This really does take care of security checks.
           * Last Patch provided by Patchstack (RR)
           */
          $allowed_callables = array('foo', 'bar');  

Then modify line 111 (now line 116) from

if ( function_exists( $_REQUEST['_memberhero_hook'] ) ) {

to

if ( in_array($_REQUEST['_memberhero_hook'], $allowed_callables) && function_exists( $_REQUEST['_memberhero_hook'] ) ) {

Finally, add an else clause to capture failures of the above if condition and return wp_die(). This should be on the next line with just a } character, probably near line 118.

 } else { wp_die('Uncallable REQUEST'); }

For hosting providers

The mod_security rule for this utilizes three chained together rules which will block all requests to wp-admin/admin-ajax.php when the action variable is set to memberhero_send_form and the _memberhero_hook variable is set to any value.

SecRule ARGS_NAMES "_memberhero_hook" "deny,id:123,chain"
  SecRule ARGS:action "memberhero_send_form" "chain"
  SecRule REQUEST_FILENAME "/wp-admin/admin-ajax\.php$"

Last Patch: member-hero

Verifying the exploit

The exploit proof of concept is publicly available and was easy to reproduce.

curl https://example.com/wp-admin/admin-ajax.php?action=memberhero_send_form&_memberhero_hook=phpinfo

Sending the above request returns the output of whatever PHP function we provided to _memberhero_hook. In the above case it is the output of phpinfo();

Finding what to patch

The proof of concept makes tracking down the insecure code simple, I just need to look for the memberhero_send_form action.

# grep -R memberhero_send_form .
./assets/js/jquery-memberhero/jquery-memberhero.js:        $.post( memberhero_params.ajaxurl, elem.serialize() + '&action=memberhero_send_form', function( response ) {

That shows me some Javascript, which may be useful later … but what I’m looking for is PHP. I will try to look for just “send_form” now.

# grep -R send_form .
./assets/js/jquery-memberhero/jquery-memberhero.js:        $.post( memberhero_params.ajaxurl, elem.serialize() + '&action=memberhero_send_form', function( response ) {
./includes/class-memberhero-ajax.php:            'send_form'                 => true,
./includes/class-memberhero-ajax.php:    public static function send_form() {

This is a low more promising. Let’s look at the contents of ./includes/class-memberhero-ajax.php and I will share the relevant lines of code:

22        /**
23         * Hook in methods - uses WordPress ajax handlers (admin-ajax).
24         */
25        public static function add_ajax_events() {
26            // memberhero_EVENT => nopriv.
27            $ajax_events = array(
28                'check_confirmation_code'   => true,
29                'send_form'                 => true,
...
59    
60            foreach ( $ajax_events as $ajax_event => $nopriv ) {
61                add_action( 'wp_ajax_memberhero_' . $ajax_event, array( __CLASS__, $ajax_event ) );
62    
63                if ( $nopriv ) {
64                    add_action( 'wp_ajax_nopriv_memberhero_' . $ajax_event, array( __CLASS__, $ajax_event ) );
...
101        /**
102         * Send a form.
103         */
104        public static function send_form() {
105            global $the_form;
106    
107            /**
108             * This will take care of security checks.
109             */
110            if ( isset( $_REQUEST['_memberhero_hook'] ) && ! empty( $_REQUEST['_memberhero_hook'] ) ) {
111                if ( function_exists( $_REQUEST['_memberhero_hook'] ) ) {
112                    call_user_func( $_REQUEST['_memberhero_hook'] );
113                }

Looks like that was it. Now let me explain what is going on.

Deciphering the code

On lines 25 – 64 they are using an array to create all of their AJAX endpoints. Each AJAX endpoint created starts with memberhero_ and is then followed by the hook name. I could not find “memberhero_send_form” but was successful when looking for just “send_form”. They concatenate the string to make a bunch of AJAX endpoints starting with “memberhero_” all at once.

Furthermore, they created the memberhero_send_form AJAX endpoints with “wp_ajax_nopriv_”. This means the functionality is intended for not logged-in users. A dangerous decision.

The newly created “wp_ajax_nopriv_memberhero_send_form” AJAX endpoint calls the send_form() function (lines 104 – 138) which is where the remote code execution part comes into play. On lines 107 – 109 an unfortunate comment of “This will take care of security checks” is simply false. The next line of code ( 110 ) is an if statement that has no security implication:

110            if ( isset( $_REQUEST['_memberhero_hook'] ) && ! empty( $_REQUEST['_memberhero_hook'] ) ) {

This line of code only checks that the $_REQUEST['_memberhero_hook'] variable is set and not empty. This is not a security check.

Lines 111 – 113 will take the value of $_REQUEST['_memberhero_hook'] and pass it through to call_user_func() which executes the function provided. In this case, the value of $_REQUEST['_memberhero_hook'] is user-controlled input, and the person making the request can call any PHP function they choose.

111                if ( function_exists( $_REQUEST['_memberhero_hook'] ) ) {
112                    call_user_func( $_REQUEST['_memberhero_hook'] );

Looks like a clear-cut case of arbitrary code execution. Now let’s look into how to patch it.

Writing the patch

My first concern for writing this patch was finding out how this plugin intended to use the “wp_ajax_nopriv_memberhero_send_form” endpoint. We noticed it was called via Javascript in ./assets/js/jquery-memberhero/jquery-memberhero.js, so it is called via a browser interaction.

Using an open-source tool like OWASP ZAP I was able to collect a log of every request my browser made. Using ZAP’s manual explore mode I used a browser to trigger as many of the member-hero features as I could find. After some time building and testing forms, I never once triggered the memberhero_send_form AJAX endpoint.

This is why the patch will use an empty allow list. Someone else can fill it in with valid function names, but I was unable to track down how it was supposed to be used. So, my compromise is an allow list that allows nothing.

Here is the updated code:

101        /**
102         * Send a form.
103         */
104        public static function send_form() {
105            global $the_form;
106    
107            /** 
108              * Lines 111, 116 and 118 take care of security checks.
109              * Last Patch provided by Patchstack (RR)
110              */
111            $allowed_callables = array('foo', 'bar');  
112            /**
113             * This will take care of security checks.
114             */
115            if ( isset( $_REQUEST['_memberhero_hook'] ) && ! empty( $_REQUEST['_memberhero_hook'] ) ) {
116                if ( in_array($_REQUEST['_memberhero_hook'], $allowed_callables) && function_exists( $_REQUEST['_memberhero_hook'] ) ) {
117                    call_user_func( $_REQUEST['_memberhero_hook'] );
118                } else { wp_die('Uncallable REQUEST'); }
119            } else {

Note the changes on line 111, which creates the list of allowable functions or callables and saves them to $allowed_callables. I have left it empty for now so you can add a list of acceptable function calls if you find which are needed.

Line 116 is where I added the check that verifies the value of $_REQUEST['_memberhero_hook'] is in the list of $allowed_callables if it is not, then we stop execution with wp_die() and return the error “Uncallable REQUEST”.

Upon testing, I confirmed that the empty allow list works as expected and prevents calling arbitrary functions.

Writing the WAF rule (mod_security)

The WAF rule for this patch is simple but does not provide a list of allowable function names. Instead, because this plugin is known to be abandoned, we should play it safe and block any requests that look similar to the proof of concept.

Here is a simple mod_security rule that will block any request to WordPress admin AJAX endpoints with the action value of memberhero_send_form if _memberhero_hook is set to any value.

SecRule ARGS_NAMES "_memberhero_hook" "deny,id:123,chain"
  SecRule ARGS:action "memberhero_send_form" "chain"
  SecRule REQUEST_FILENAME "/wp-admin/admin-ajax\.php$"

Notice the usage of “chain” in the Action list. This is what combines these three rules into one rule (with id number 123, which you should change for your own needs.)

Conclusions

The author of the member-hero plugin clearly informed their users that the plugin was no longer supported. It is likely that all of their users have moved on and found a replacement, but the transparency of this open-source project still provided us with the information we needed to learn something new.

Creating allow lists can be an effective way to add defensive code to your project. This should be required any time you perform a dangerous action like calling call_user_func(), especially within an API endpoint that requires no authorization.