1

I have set up several WordPress Admin AJAX scripts (that run only in the Admin panel) that currently all use separate actions, functions and nonce's.

The only "real" difference between these scripts is their PHP functions. So, in an effort to streamline the system I have written 1 JS function and 1 main PHP function to handle them all.

A $_POST value is sent with a path to the include. Therefore when wp_ajax_custom_func() runs the PHP file is included and wrapped in locate_template()

It works great, but I am concerned if this creates a security hole?

I am aware attackers can try fake a nonce, send bogus POST data to admin-ajax.php, XSS/cross scripting attacks, ect...

I am assuming that because I have wrapped the include in locate_template() that this will help protect the system. (long side an SSL, wp_verify_nonce and current_user_can of course)

Here is the core concept, simplified.

Setup

 wp_enqueue_script( 'ajax_js_class', 
         get_template_directory_uri() . '/theme/ajax_js_class.js' 
     );

 wp_localize_script( 'ajax_js_class', 'custom_settings', array(
        'my_ajaxurl'    => admin_url( 'admin-ajax.php' )
         // nonce and action are attached at the form level
    ) );

Function

  add_action( 'wp_ajax_run_ajax_class_func', 'run_ajax_class_func' ); 
  // wp_ajax_nopriv_run_ajax_class_func is not used

function run_ajax_class_func() {
    if(isset($_POST['nonce'])) {
        $nonce=  $_POST['nonce']; // the nonce from the form
        $action = $_POST['action_hash']."-special-string"; // changes on each form
        $verify_nonce = wp_verify_nonce($nonce, $action); 

        if( $verify_nonce !== false ) {
            if(isset($_POST['path']) && current_user_can('administrator') ) {
                $path = $_POST['path']; // <= this is my concern...
                include locate_template($_POST['path']);
            } else {
            $output = json_encode( array( "success" => "false", "message" => "fail") );
            }
        } else {
        $output = json_encode( array( "success" => "false", "message" => "fail") );
        }
    } else {
      $output = json_encode( array( "success" => "false", "message" => "fail") );
    }

    echo $output; // this is handled in the include above
    // always die
    die();
}

HTML Essentials

 <?php $nonce = wp_create_nonce( $random_hash."-special-string" ); ?>
 <form>
    <input type="hidden" name="path" value="in/theme/run_this.php" >
    <input type="hidden" name="nonce" value="<?php echo $nonce; ?>" >
    <input type="hidden" name="action_hash" value="<?php echo $random_hash; ?>" >

Localized Js Essentials

 $.post(custom_settings.my_ajaxurl, data, function(response) {

I have tried to attack the form (using Fiddler2 to modify the headers and post data) but suspect I suffer from confirmation bias.

I would like to consult the community to see if this idea is poor or good. (bad ideas of course deserve criticism)

Questions:

  1. If I have secured the wp_ajax function (according to WordPress best practices) is it OK to use POST data to include a file within the WP Admin AJAX action?
  2. Am I making it easier for attackers to abuse the system?
  3. Is this creating an unnecessary back door in the system just to avoid writing several additional functions, ie: is it lazy?

In closing, if creating each nonce, enqueue, form and action as completely separate scripts is safer please elaborate as to why.

Reference and research:

  • You shouldn't be using a POST request to get something. – Jacob Peattie Nov 28 '18 at 10:40
1

If you use nonces properly, it won't be possible to make the site to process fake request... So this part should be secure, but... There is still one security flaw in your approach...

What if I can make you to run my JS script in your browser, while you're logged in as admin? It is possible.

This way nonce won't cause validation error and your code will execute whatever I want - because you don't verify input from user...

So all your security is based on possibility of injecting some JS to your site - and that's pretty common - just take a look at malware history in WP...

It would be much more secure, if the template was loaded based on verified data. So don't load it directly, but make a list of correct, secure templates and load them based on variable sent in POST.

So back to your 3 questions:

1. If I have secured the wp_ajax function (according to WordPress best practices) is it OK to use POST data to include a file within the WP Admin AJAX action?

No. It's never OK to load any file based on path sent by user. It's much more secure if you pass a variable and then translate this variable to path of a file in PHP.

2. Am I making it easier for attackers to abuse the system?

You're not making it much easier, but yes - it will be possible to attack such code.

3. Is this creating an unnecessary back door in the system just to avoid writing several additional functions, ie: is it lazy?

Yes, it is. But you don't have to write several functions. You just have to secure this one properly and remember - you should never trust anything that comes from user.

  • Thank you. I had suspected this may be the case. I will make a Key/Pair array of "allowed" includes and refer to them by ID rather than raw paths. I appreciate your attention to detail. – Christian Žagarskas Nov 29 '18 at 0:35

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.