WP Admin AJAX Security - using POST to include a relative URL

admin管理员组

文章数量:1130349

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:

• Security - Ajax and Nonce use
• How to include admin-ajax when loading external javascript
• wp_remote_get() to get AJAX url /wp-admin/admin-ajax.php
• WordPress Ajax Data Security
• /

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:

• Security - Ajax and Nonce use
• How to include admin-ajax when loading external javascript
• wp_remote_get() to get AJAX url /wp-admin/admin-ajax.php
• WordPress Ajax Data Security
• https://codex.wordpress/Function_Reference/current_user_can
• https://codex.wordpress/Function_Reference/wp_verify_nonce
• https://codex.wordpress/WordPress_Nonces
• https://codex.wordpress/Function_Reference/check_ajax_referer
• https://tommcfarlin/secure-ajax-requests-in-wordpress/

• ajax
• wp-admin
• security
• nonce

Share Improve this question edited Nov 28, 2018 at 10:02 Krzysiek Dróżdż 25.6k99 gold badges5353 silver badges7474 bronze badges asked Nov 28, 2018 at 7:49 Christian ŽagarskasChristian Žagarskas 15299 bronze badges 1

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

Add a comment  | 

1 Answer 1

Sorted by: Reset to default 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.

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:

• Security - Ajax and Nonce use
• How to include admin-ajax when loading external javascript
• wp_remote_get() to get AJAX url /wp-admin/admin-ajax.php
• WordPress Ajax Data Security
• /

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:

• Security - Ajax and Nonce use
• How to include admin-ajax when loading external javascript
• wp_remote_get() to get AJAX url /wp-admin/admin-ajax.php
• WordPress Ajax Data Security
• https://codex.wordpress/Function_Reference/current_user_can
• https://codex.wordpress/Function_Reference/wp_verify_nonce
• https://codex.wordpress/WordPress_Nonces
• https://codex.wordpress/Function_Reference/check_ajax_referer
• https://tommcfarlin/secure-ajax-requests-in-wordpress/

• ajax
• wp-admin
• security
• nonce

Share Improve this question edited Nov 28, 2018 at 10:02 Krzysiek Dróżdż 25.6k99 gold badges5353 silver badges7474 bronze badges asked Nov 28, 2018 at 7:49 Christian ŽagarskasChristian Žagarskas 15299 bronze badges 1

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

Add a comment  | 

1 Answer 1

Sorted by: Reset to default 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.

本文标签： WP Admin AJAX Securityusing POST to include a relative URL