Opened 13 years ago

Closed 13 years ago

Last modified 13 years ago

#15869 closed Bug (fixed)

CSRF fix for Ajax POST mentioned in docs intermittently fails to append token for IE7

Reported by: nick@… Owned by: nobody
Component: Documentation Version: 1.3
Severity: Normal Keywords: ajax csrf post
Cc: Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

I've found that in some cases(not sure why), IE7 will prepend protocol://servername to a form's action, causing the

 if (!(/^http:.*/.test(settings.url) || /^https:.*/.test(settings.url))) {...}

test to fail...

I propose we use the following instead, as it will work in more cases:

    var page_host = window.location.host;
    var regex=new RegExp('^https?://' + page_host + '/', 'i');
    if (regex.test(settings.url) || !(/^http:.*/.test(settings.url) || /^https:.*/.test(settings.url))) {
        // Only send the token to relative URLs i.e. locally.
    }

Change History (6)

comment:1 by Luke Plant, 13 years ago

Easy pickings: unset
Triage Stage: UnreviewedAccepted

Thanks. Can you check if this works when there is an explicit port number in the URL? (i.e. presumably IE will have to include this in the URL, but is it also included in window.location.host?)

in reply to:  1 comment:2 by anonymous, 13 years ago

Just checked, and yes, it does appear to work properly, even when a port is in the URL. Good idea, I hadn't thought of that!

Replying to lukeplant:

Thanks. Can you check if this works when there is an explicit port number in the URL? (i.e. presumably IE will have to include this in the URL, but is it also included in window.location.host?)

comment:3 by Luke Plant, 13 years ago

Two problems:

1) the regex approach actually is incorrect if page_host contains any special characters like ., which it will do, opening up vulnerabilities. Escaping the special characters is one fix, but that is trickier than it ought to be, because javascript doesn't have built in methods for doing it.

2) I think we should treat http and https as completely different. There are many bugs that come from treating them as the same. We should just the 'same origin' protocol here.

In addition, the original code doesn't handle things like scheme relative URLs correctly. I'll come up with a patch shortly.

comment:4 by Luke Plant, 13 years ago

Resolution: fixed
Status: newclosed

In [16183]:

Fixed #15869 - example AJAX code in CSRF docs fails sometimes for IE7 or absolute same origin URLs

Thanks to nick for the report.

comment:5 by Luke Plant, 13 years ago

In [16184]:

[1.3.X] Fixed #15869 - example AJAX code in CSRF docs fails sometimes for IE7 or absolute same origin URLs

Thanks to nick for the report.

Backport of [16183] from trunk.

comment:6 by Luke Plant, 13 years ago

In [16185]:

[1.2.X] Fixed #15869 - example AJAX code in CSRF docs fails sometimes for IE7 or absolute same origin URLs

Thanks to nick for the report.

Backport of [16183] from trunk.

Note: See TracTickets for help on using tickets.
Back to Top