Django

Code

Ticket #8127 (closed: fixed)

Opened 2 years ago

Last modified 2 years ago

CSRF + AJAX

Reported by: xlax Assigned to: nobody
Milestone: Component: Contrib apps
Version: SVN Keywords: CSRF AJAX
Cc: jeroen.pulles@redslider.net, carl@dirtcircle.com, stefan.cornelius@gmail.com, lidaobing@gmail.com, johannes.beigel@pediapress.com, neithere@gmail.com Triage Stage: Design decision needed
Has patch: 1 Needs documentation: 0
Needs tests: 1 Patch needs improvement: 0

Description

I've got a patch ready for anyone who needs it (if it's accepted) to use CSRF with AJAX. It's simply adding a line to the main CSRF function, stating that if AJAX is being used, there's no need for a check. I know of no issues regarding this. I can submit the file immediatelly. (It's been tested here)

Attachments

csrf_middleware_ajax.diff (0.5 kB) - added by xlax on 08/06/08 13:11:01.
csrf_check_ajax_requests_too.diff (0.9 kB) - added by Jeroen Pulles <jeroen.pulles@redslider.net> on 08/14/08 04:23:02.
Get the CSRF token from HTTP headers when using Ajax requests
csrf_middleware_ajax_flash_patch.diff (0.8 kB) - added by herrstagl on 09/30/08 08:24:40.
patch which allows to provide a session id within a post request: userfull for flash based uploaders where authentication is required (done for www.xipax.com)

Change History

08/05/08 16:31:04 changed by xlax

  • keywords set to CSRF AJAX.
  • needs_better_patch changed.
  • component changed from Uncategorized to Contrib apps.
  • needs_tests changed.
  • needs_docs changed.

08/05/08 17:03:29 changed by simon

Assuming this is a check for the X-Requested-By: XMLHttpRequest header, I'm all for it - it's a nice, secure way of allowing Ajax requests to avoid having to include a form token, which can be tricky to make available to the Ajax code otherwise.

08/06/08 12:53:58 changed by xlax

It's using the request.is_ajax() function. This is documented here: http://www.djangoproject.com/documentation/request_response/#methods

Quote: """ Returns True if the request was made via an XMLHttpRequest, by checking the HTTP_X_REQUESTED_WITH header for the string 'XMLHttpRequest'. """

I'm currently trying to do the svn diff. First time I did it, so please don't shoot me if I did something wrong :)

08/06/08 13:11:01 changed by xlax

  • attachment csrf_middleware_ajax.diff added.

08/06/08 13:11:40 changed by xlax

** Edit ** I have attached the diff file, I hope it is as wanted...

08/07/08 00:31:09 changed by xlax

  • has_patch set to 1.
  • needs_tests set to 1.

08/08/08 11:30:24 changed by ericholscher

  • stage changed from Unreviewed to Design decision needed.

08/14/08 04:21:48 changed by Jeroen Pulles <jeroen.pulles@redslider.net>

Hi,

Actually, this is a problem that might bite the unwary: If you use the CSRF middleware, and do JSON posts, you will run into trouble right now.

Personally, I prefer to check the key in JSON request bodies as well (being a bit paranoid, hey I'm using the CSRF middleware :-/).

So the attached diff (csrf_check_ajax_requests_too.diff) checks for a HTTP_X_CSRFMIDDLEWARETOKEN in the HTTP header. You can easily add that header in most Ajax libraries, e.g. request.setHeader('X-CsrfMiddlewareToken', document.getElementById('csrfmiddlewaretoken').value); .

The only downside you have is that there might not be a key on a webpage because it has no <form method="post"> in the response. That seems to be the exception in my experience, and that can be worked around just as easy.

08/14/08 04:23:02 changed by Jeroen Pulles <jeroen.pulles@redslider.net>

  • attachment csrf_check_ajax_requests_too.diff added.

Get the CSRF token from HTTP headers when using Ajax requests

08/14/08 04:35:22 changed by anonymous

  • cc set to jeroen.pulles@redslider.net.

08/14/08 05:23:02 changed by Jeroen Pulles <jeroen.pulles@redslider.net>

Uh, oops:

My patch should have checked the Content-Type of the request body, and assume that requests of the type application/x-www-form-urlencoded are still handled with the existing request.POST['csrfmiddlewaretoken'] .

08/14/08 05:31:35 changed by Jeroen Pulles <jeroen.pulles@redslider.net>

More rationale:

Actually, this is a problem that might bite the unwary: If you use the CSRF middleware, and do JSON posts, you will run into trouble right now.

That goes for any POST request that isn't application/x-www-form-urlencoded. Which is why it would be welcome to make the request processing more robust, regardless of the design choice to check for the token or not.

09/05/08 16:10:46 changed by xlax

Perhaps the best way to go is to make CSRF generate a key per-standard, and make it available. That way, at all points in the application, the string is usable and the ajax user could implement that into the "form" sent through.

09/27/08 20:46:56 changed by jmillikin

I found a SecurityFocus? article[1] that mentions that Adobe Flash is capable of both cross-site requests and setting arbitrary HTTP headers. If this is true, then relying on the X-Requested-With header will allow hostile cross-site requests if a user visits a malicious site.

Is this concern accurate, or does Flash enforce the same cross-site rules as the XMLHttpRequest object?

[1] http://www.securityfocus.com/archive/1/441014

09/30/08 08:22:36 changed by herrstagl

We had some issues with csrf and ajax when we used an flash uploader in combination with windows and firefox. in this special case, flash is sending the IE cookies instead of the Firefox cookies and therefore CSRF raises an error because no valid session is available. What we did, to get csrf working even with the flash YUI upload:

  • we added the csrfmiddlewaretoken to the ajax post request
  • we wrote a patch during the implementation of www.xipax.com for the middleware.py which checks if there is POST variable called session_id, if not, it falls back to the standard behaviour of the CSRF middleware

This file is attached to the ticket

09/30/08 08:24:40 changed by herrstagl

  • attachment csrf_middleware_ajax_flash_patch.diff added.

patch which allows to provide a session id within a post request: userfull for flash based uploaders where authentication is required (done for www.xipax.com)

10/01/08 09:43:28 changed by carljm

  • cc changed from jeroen.pulles@redslider.net to jeroen.pulles@redslider.net, carl@dirtcircle.com.

10/14/08 12:26:56 changed by DerCorny

  • cc changed from jeroen.pulles@redslider.net, carl@dirtcircle.com to jeroen.pulles@redslider.net, carl@dirtcircle.com, stefan.cornelius@gmail.com.

Being post-1.0 now, how is it going with the design descision? There are also related bugs, e.g. #6709 and #7167. The patches here actually look kind of good to me and I don't think that the SafeForms? discussed in #7167 will help for highly dynamic AJAX, so why not extend the CSRF middleware with these patches and add a nice and simple template tag to generate the token?

While this might not be the best solution ever, I'm still convinced that a lot of people will benefit from a "standard" solution to at least mitigate the problem in an easy way - the patches don't mess up too much and I think it's way better than having users develop their own hackish and potentially insecure method of extending the CSRF protection. Think of all the snippets and apps out there that are currently lacking a standard way of tackling this problem (e.g. threaded-comments, iirc).

10/26/08 22:52:45 changed by lidaobing

  • cc changed from jeroen.pulles@redslider.net, carl@dirtcircle.com, stefan.cornelius@gmail.com to jeroen.pulles@redslider.net, carl@dirtcircle.com, stefan.cornelius@gmail.com, lidaobing@gmail.com.

10/28/08 05:00:05 changed by jojo

  • cc changed from jeroen.pulles@redslider.net, carl@dirtcircle.com, stefan.cornelius@gmail.com, lidaobing@gmail.com to jeroen.pulles@redslider.net, carl@dirtcircle.com, stefan.cornelius@gmail.com, lidaobing@gmail.com, johannes.beigel@pediapress.com.

10/28/08 05:49:17 changed by neithere

  • cc changed from jeroen.pulles@redslider.net, carl@dirtcircle.com, stefan.cornelius@gmail.com, lidaobing@gmail.com, johannes.beigel@pediapress.com to jeroen.pulles@redslider.net, carl@dirtcircle.com, stefan.cornelius@gmail.com, lidaobing@gmail.com, johannes.beigel@pediapress.com, neithere@gmail.com.

01/06/09 13:03:34 changed by bthomas

  • status changed from new to closed.
  • resolution set to fixed.

This was fixed in [9554]


Add/Change #8127 (CSRF + AJAX)




Change Properties
Action