Code

Opened 9 years ago

Closed 8 years ago

Last modified 7 years ago

#510 closed enhancement (fixed)

[patch] Defend admin against CSRF attacks

Reported by: Simon Willison Owned by: adrian
Component: contrib.admin Version:
Severity: major Keywords:
Cc: gdub@… Triage Stage: Unreviewed
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: UI/UX:

Description

Django's admin pages are curently vulnerable to CSRF attacks, as described here:

http://www.squarefree.com/securitytips/web-developers.html#CSRF

All POST forms in the admin should contain a hidden field with a shared secret that can be used to confirm the origin of the form.

Attachments (2)

csrf-protection.patch (7.1 KB) - added by Simon Willison 9 years ago.
Patch implementing CSRF protection for Django admin screens.
csrf-protection2.patch (7.5 KB) - added by Simon Willison 9 years ago.
Improved patch; now uses module methods for token checking and creation.

Download all attachments as: .zip

Change History (15)

comment:1 Changed 9 years ago by Simon Willison

My post about this to the developer mailing list: http://groups.google.com/group/django-developers/msg/74d0a8d633990a1e

comment:2 Changed 9 years ago by anonymous

Here's a way of defending the admin with an extra validator and a custom template tag.

Have a model built in to Django called csrf_tokens, with three fields:

tokenA long, randomly generated string.
issuedThe date and time that the token was issued.
user_idThe user to which the token was issued.

Define a custom template tag called {% csrf_token %}, which, when rendered, does the following:

  1. Creates a new random token and inserts it in to the csrf_tokens table. If user is available in the context, populate the user_id field.
  2. Displays a hidden form field with name="csrf_token" and the value set to the token.

The form view (probably in the validator logic) looks at the csrf_token of any submitted forms and checks that it is a record in the csrf_tokens table that was added for the same user (if the user field is set) within the last 10 minutes. If it finds one, it deletes that token - hence tokens can only ever be used once. If the token isn't there or is too old, the form is redisplayed with a suitable error message and the user has to resubmit it.

This should be secure. My only concern is this: would it be possible for an attacker to trick your browser in to making the initial submission with an incorrect token, then submit the form that was returned? I'm pretty sure the answer to that one is no.

Changed 9 years ago by Simon Willison

Patch implementing CSRF protection for Django admin screens.

comment:3 Changed 9 years ago by Simon Willison

  • Summary changed from Defend admin against CSRF attacks to [patch] Defend admin against CSRF attacks
  • Type changed from defect to enhancement

This patch implements CSRF protection for all Add, Change and Delete forms in Django's admin. It adds a new table to Django (defined in the auth model file) called auth_csrf_tokens, which stores csrf tokens. When ever you view an add, change or delete form a taken is created and saved in that table. When you submit the form, the token is deleted.

At the moment, there is no mechanism for clearing stale tokens out of the table. This needs to be done every day or so, by deleting tokens that are over a day old. Token accumulation is inevitable - to get to the delete screen for an item you must first visit its change screen, which will result in the creation of a stale token.

I've tested the patch and it appears to block all CSRF attacks.

Changed 9 years ago by Simon Willison

Improved patch; now uses module methods for token checking and creation.

comment:4 Changed 9 years ago by jacob

  • milestone set to Version 1.0

comment:5 Changed 8 years ago by anonymous

  • Cc gdub@… added

comment:6 Changed 8 years ago by SmileyChris

Just looking at csrf-protection2.patch, CsrfTokenNode should return XHTML:

class CsrfTokenNode(template.Node): 
    def render(self, context): 
        token = csrf.create_token(context['user'].id) 
        return '<input type="hidden" name="csrf_token" value="%s" />' % token.token

comment:7 Changed 8 years ago by SmileyChris

Do we really need to store these tokens in their own model?
I propose the CSRF value should be stored in the session, set when the session is created and changed whenever tested against.

comment:8 Changed 8 years ago by lukeplant

I was prompted by SmileyChris's addition to add a link here to the CSRF middleware I wrote a while back. It uses a more lightweight approach that doesn't require storing anything in the database, and works for any POST request, not just the admin.

http://lukeplant.me.uk/resources/csrfmiddleware/

comment:9 Changed 8 years ago by mir@…

Lukeplant's middleware works fine. The difference between this and the patch is:

  • the patch provides a way to enable csrf fields on a per-template base.
  • Lukeplant's middleware works out of the box for all forms automatically

I chose the middleware solution for myself ... either the middleware (into contrib?) or the patch should really be integrated into django.

comment:10 Changed 8 years ago by SmileyChris

Sweet, I see Luke's solution has just been added to contrib :)

comment:11 Changed 8 years ago by adrian

  • Resolution set to fixed
  • Status changed from new to closed

Fixed now that we have the CSRFMiddleware.

comment:12 Changed 8 years ago by anonymous

which happened in [2868].

comment:13 Changed 7 years ago by anonymous

  • milestone Version 1.0 deleted

Milestone Version 1.0 deleted

Add Comment

Modify Ticket

Change Properties
<Author field>
Action
as closed
as The resolution will be set. Next status will be 'closed'
The resolution will be deleted. Next status will be 'new'
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.