ezyang / csrf-magic

Automatic CSRF protection for PHP applications
BSD 2-Clause "Simplified" License
41 stars 22 forks source link

No file upload with v1.0.4 #4

Open webbird opened 11 years ago

webbird commented 11 years ago

After upgrading from 1.0.1 to 1.0.4, file uploads are no longer possible. It seems the JS scrambles the form data somehow. On the server side, $ REQUEST and $ POST are just empty.

After replacing the JS with the old version, uploads are working again.

ezyang commented 10 years ago

I need more information to fix this bug. How are you constructing the file upload widget?

webbird commented 10 years ago

It's just a simple upload form. I will try to create a very simple test case for you.

webbird commented 10 years ago

Still working on a test script for you, because it's a complex application where we use csrf-magic. For now, assume this: Given is a file upload form which calls an upload script via AJAX. I also have a custom csrf_callback() method. Here I've added these lines to see the difference:

echo "POST: ";
print_r($_POST);
echo "FILES: ";
print_r($_FILES);
echo "TOKENS: ";
print_r($tokens);

Result for 1.0.4:

POST: Array ( ) FILES: Array ( ) TOKENS:

Result for 1.0.3: Nothing, as the callback is not called. Adding the same code to the upload handler gives:

POST: Array
(
    [__csrf_magic] => sid:6215f76a2879162666a85455380a2bda6e47505d,1406200385
    [form_title] => Datei(en) hochladen
    [folder_path] => /media/gallery2
    [_cat_ajax] => 1
    [test] => Datei wählen...
    [upload_counter] => Array
        (
            [0] => 0
            [1] => 1
        )

    [upload_1] =>
)
FILES: Array
(
    [upload_0] => Array
        (
            [name] => 1239901119_reload_all_tabs.png
            [type] => image/png
            [tmp_name] => D:\xampp\tmp\php6A12.tmp
            [error] => 0
            [size] => 1757
        )

)
webbird commented 10 years ago

My config:

function csrf_startup()
{
    // AJAX requests are allowed via POST only and must identify themselves
    // by adding a '_cat_ajax' param to the request
    if (isset($_POST['_cat_ajax']))
        csrf_conf('rewrite', false);
    // This enables JavaScript rewriting and will ensure your AJAX calls
    // don't stop working.
    csrf_conf('rewrite-js', CAT_URL . '/modules/lib_csrfmagic/csrf-magic.js');
    // This makes csrf-magic call my_csrf_callback() before exiting when
    // there is a bad csrf token. This lets me customize the error page.
    csrf_conf('callback', 'cat_csrf_callback');
    // While this is enabled by default to boost backwards compatibility,
    // for security purposes it should ideally be off. Some users can be
    // NATted or have dialup addresses which rotate frequently. Cookies
    // are much more reliable.
    csrf_conf('allow-ip', false);
    // Token lifetime
    if (defined('TOKEN_LIFETIME') && TOKEN_LIFETIME > 0)
    {
        csrf_conf('expires', TOKEN_LIFETIME);
    }
}
webbird commented 10 years ago

Found reason:

if (window.XMLHttpRequest && window.XMLHttpRequest.prototype && '\v' != '\v') {

Old line was

if (window.XMLHttpRequest && window.XMLHttpRequest.prototype && '\v' != 'v') {

Dunno what this is for...?

webbird commented 10 years ago

To be more precise... After changing that line in the old js (v1.0.3), I got the same error. After removing the change, the error was gone again. But after changing that line in v1.0.4 to the old version, the error is still there. I am confused now.

webbird commented 10 years ago

Sorry, I mixed up old and new line. After changing that line in v1.0.4 to the old version - which is the one with 2x \v - the upload works again. Also forgot to mention that I am working with Firefox 30 on Win7.

ezyang commented 10 years ago

I can guess the high-level problem: AJAX and file uploads are not working. (Can it be reproduced with ordinary AJAX?) Can you summarize your investigation in the various few comments? Some of it doesn't seem consistent with what I remember happening in the code. Note that 1.0.2 and 1.0.3 are incorrectly labeled in source as 1.0.1.

webbird commented 10 years ago

The solution that worked for me was to change line

if (window.XMLHttpRequest && window.XMLHttpRequest.prototype && '\v' != 'v') {

back to

if (window.XMLHttpRequest && window.XMLHttpRequest.prototype && '\v' != '\v') {

Sorry for confusion.

ezyang commented 10 years ago

Well, you're just turning off the version detection. But maybe this is a good clue

webbird commented 10 years ago

I am working with Firefox 31. Let me know if I can do something.

netniV commented 4 years ago

Was this a problem after the json object fixes? I suspect not.