CSRF-Protector-PHP icon indicating copy to clipboard operation
CSRF-Protector-PHP copied to clipboard

Ajax wrongfully blocked

Open JimmyPruitt opened this issue 7 years ago • 11 comments

This is the new issue you told me to create from issue #57.

If I have an Ajax request that is supposed to start as soon as the page loads, the request is blocked because xhr's send() isn't overridden until after the request has started:

<?php
    include_once 'csrf-protector/libs/csrf/csrfprotector.php';
    csrfProtector::init();

    function test_csrfp()
    {
        echo
            '<html>
                <head>
                    <script src="jquery-1.12.0.min.js"></script>
                </head>
                <body>
                    <script>
                        $(document).ready(function() {
                            $.ajax({
                                method: "POST",
                                url: "index.php",
                                data: {
                                    name: "csrfp_test"
                                }
                            });
                        });
                    </script>
                </body>
            </html>';
    }

    function ajax()
    {
        echo
            "<html>
                <body>Success</body>
            </html>";
    }

    if ($_POST)
        ajax();

    else
        test_csrfp();

I was able to find a workaround that works for my case. If you holdReady until init() is called:

if (typeof window.jQuery != 'undefined')
    $.holdReady(true);

window.addEventListener("DOMContentLoaded", function() {
    csrfprotector_init();

    if (typeof window.jQuery != 'undefined')
        $.holdReady(false);

}, false);

JimmyPruitt avatar Jan 06 '17 22:01 JimmyPruitt

Wait, I tested the code you sent and I'm getting proper output and correct validation for CSRF. I'm assuming you are using latest code. Could you give more information about environment / infrastructure?

Output of ajax call looks like this

<html>
                <body> <noscript>This site attempts to protect users against 
Cross-Site Request Forgeries attacks. In order to do so, you must have JavaScript enabled in your web browser otherwise th is site will fail to work correctly for you.
See details of your web browser for how to enable JavaScript.</noscript>Success<input type="hidden" id="csrfp_hidden_data_token" value="__vccsrfp">
<input type="hidden" id="csrfp_hidden_data_urls" value='[":\/\/\/"]'><script type="text/javascript" src="http://localhost:8080/csrfp/js/csrfprotector.js"></script>
</body>
            </html>

mebjas avatar Jan 19 '17 17:01 mebjas

What is it you'd like to know? I'm using PHP 7, Apache 2.2, and jQuery 1.12. I tested in Chrome, Firefox, Edge, IE 11, and Opera, and they're all affected. I am using the default config options in config.php with the exception of the jsUrl and the tokenLength that I increased to 40. I am completely up to date; just ran a pull to confirm.

I tried to make a short video of it, but my capture software is behaving rather oddly right now.

JimmyPruitt avatar Jan 27 '17 19:01 JimmyPruitt

I was finally able to make the video. I'm not sure it will help much, but it's there for you. Sorry the quality is so bad.

JimmyPruitt avatar Jan 30 '17 17:01 JimmyPruitt

I also have run into this whereby some inline Javascript (or scripts added before the csrfprotector.js script) have their ajax functions run before csrfprotector_init() has been run.

I got around it by making the following hacks but it would be nice to ensure that the csrfprotector_init() function is run before anything else (once the dom is loaded).

//window.addEventListener("DOMContentLoaded", function() {
// 	csrfprotector_init();
// }, false);

// As some internal code does ajax calls on document.ready with $.ajax we need
// to catch these and ensure the csrfprotector has been initalized. The standard
// approach using the DOMContentLoaded listener happens after these calls so we
// need to use .ajaxStart also.
var csrfInitialized = false;
// Standard approach using DOMContentLoaded listener.
window.addEventListener("DOMContentLoaded", function() {
  if (!csrfInitialized) {
    csrfprotector_init();
    csrfInitialized = true;
  }
}, false);
// Additional approach when ajax is initiated before the above listener is run.
$(document).ajaxStart(function(){
  if (!csrfInitialized) {
    csrfprotector_init();
    csrfInitialized = true;
  }
});

jarrodirwin avatar Feb 12 '17 21:02 jarrodirwin

csrfprotector_init() is highly dependent on DOM. So it can run only after DOM is fully loaded. Inline javascripts could be called before DOM is loaded I'm assuming - so this would be like calling a jquery method before jquery has initialised.

However we can have a callback in init method like $(docuemnt).ready method which will make sure functions are triggered only once the csrfp has been initialized.

mebjas avatar Feb 13 '17 05:02 mebjas

@mebjas Yeah sorry should have been more specific. In my specific case, the inline javascript that was running before csrfprotector_initi() was run was still attached to the $(document).ready event but was just higher up the execution order so would get run first.

jarrodirwin avatar Feb 13 '17 20:02 jarrodirwin

One way to it is, attach the CSRFP JS before any other script like just after <head.*> but this doesn't seem to be standard.

As of now the script is attached after <body/>. I'll add a config parameter to select where should the script be attached. Sounds good?

mebjas avatar Feb 14 '17 06:02 mebjas

Tried the adding script after <head*> but it doesn't give any benefit at this library is gonna initiate itself once DOM has loaded. I'm removing this from milestone for now as it's not a blocker. it's a good practice to not have inline JS or initiate ajax before document is ready.

Also, once can easily modify the js to trigger the scripts after csrp_init.

mebjas avatar Feb 16 '17 15:02 mebjas

The problem isn't caused when the events are fired, it's caused when the events are registered, since jQuery.ready is just a DOMContentLoaded event that is registered whenever jQuery.js is first run, and fires all of the functions that are passed to ready at a later time . If multiple functions for the same event are registered, then those functions fire in the order that they were registered. That's why I still think this is a problem. It doesn't matter if the script is inline or in its own file; if the call to ready comes before your script chronologically, which is a current certainty, then everything staged with ready will fire first.

I would have thought putting your script at the start of head, if one exists, would have worked, though that may have caused other problems.

JimmyPruitt avatar Feb 17 '17 18:02 JimmyPruitt

@JimmyPruitt @jarrodirwin - What is your opinion on this - https://github.com/mebjas/CSRF-Protector-PHP/pull/112

mebjas avatar Apr 18 '19 06:04 mebjas

I wrote this workaround (in csrfprotector.js) to ensure JQuery is always loaded before the csrf protector is initialized:

var waitForJQuery = setInterval(function () { if (typeof $ != 'undefined') {

//

window.addEventListener("DOMContentLoaded", function() { csrfprotector_init();

}, false);

// clearInterval(waitForJQuery); } }, 10);

I hope this helps.

ShazVM avatar Oct 09 '19 12:10 ShazVM