Code

Opened 5 years ago

Closed 4 years ago

#10996 closed (fixed)

CSRF documentation doesn't note login CSRF vulnerability

Reported by: smehmood Owned by: lukeplant
Component: Documentation Version: 1.0
Severity: Keywords: CSRF
Cc: Triage Stage: Accepted
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: UI/UX:

Description

It is my understanding the the CsrfMiddleware module does not protect against the login CSRF attacks described in http://www.adambarth.com/papers/2008/barth-jackson-mitchell-b.pdf.
This post to the django-dev seems to confirm this:http://groups.google.com/group/django-developers/browse_thread/thread/ae525f270ed46933/5a339c6d64d40868?lnk=gst&q=csrf#5a339c6d64d40868

However, the documentation for the CsrfMiddleware class does not note this, despite having a specific 'Limitations' section.
It also makes this false statement:
"POST requests that are not accompanied by a session cookie are not protected, but they do not need to be protected, since the 'attacking' Web site could make these kind of requests anyway."

Two things:

1) The fact that an attacking website could make the requests anyway is not a reason to say they don't need to be protected. It might be more accurate to say that such requests are not authenticated, and thus, are unlikely to perform sensitive actions.
2) This statement ignores the possibility of login CSRFs. These are requests that do not have a session cookie, but /do/ need to be protected.

Attachments (0)

Change History (4)

comment:1 Changed 5 years ago by lukeplant

  • Needs documentation unset
  • Needs tests unset
  • Owner changed from nobody to lukeplant
  • Patch needs improvement unset

comment:2 Changed 5 years ago by lukeplant

See CsrfProtection - out-of-the-box login views are not actually vulnerable to login CSRF with the CSRF middleware. But probably the documentation does need fixing in the 1.0.X branch (and 1.1.X branch when it arrives).

comment:3 Changed 5 years ago by Alex

  • Triage Stage changed from Unreviewed to Accepted

comment:4 Changed 4 years ago by lukeplant

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

(In [11662]) [1.1.X] Fixed #10996 - documented login CSRF vulnerabilities in the CsrfMiddleware

1.1.X branch only fix - trunk is completely different now.

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.