Opened 13 years ago

Closed 13 years ago

#16827 closed Bug (fixed)

validate CSRF token (Check length)

Reported by: jedie Owned by: Raúl Cumplido
Component: CSRF Version: 1.3
Severity: Normal Keywords:
Cc: Triage Stage: Ready for checkin
Has patch: yes Needs documentation: no
Needs tests: yes Patch needs improvement: no
Easy pickings: yes UI/UX: no

Description

I wonder that the CSRF token send from the client didn't be validated.

Don't know if a DOS attack is possible by sending many request with very long CSRF tokens?

IMHO it's a good idea to check the length before do anything with it.

See also: ​https://groups.google.com/group/django-developers/browse_thread/thread/9fc008d2a3735bc2

Attachments (2)

patch_16827.diff​ (1.9 KB ) - added by Zbigniew Siciarz 13 years ago.
Provided test for the token length.
patch_16827_2.diff​ (2.7 KB ) - added by Zbigniew Siciarz 13 years ago.
Replaced the magic number with a constant.

Download all attachments as: .zip

Change History (8)

comment:2 by tinodb, 13 years ago

Component: Uncategorized → contrib.csrf
Needs tests: set
Triage Stage: Unreviewed → Accepted

comment:3 by Raúl Cumplido, 13 years ago

Owner: changed from nobody to Raúl Cumplido
Status: new → assigned

I will provide the tests to the pull request.

by Zbigniew Siciarz, 13 years ago

Attachment: patch_16827.diff​ added

Provided test for the token length.

by Zbigniew Siciarz, 13 years ago

Attachment: patch_16827_2.diff​ added

Replaced the magic number with a constant.

comment:4 by Tomek Paczkowski, 13 years ago

Triage Stage: Accepted → Ready for checkin

Looks good.

comment:6 by Ashutosh Dwivedi, 13 years ago

from the source code i noticed that the method

django.utils.crypto.constant_time_compare

is being used for comparing the request token from the one already present in the cookie, since the comparison is already constant time is the length checking really required ?

comment:7 by Paul McMillan, 13 years ago

Resolution: → fixed
Status: assigned → closed

In [17500]:

Fixes #16827. Adds a length check to CSRF tokens before applying the santizing regex. Thanks to jedie for the report and zsiciarz for the initial patch.

Version 0, edited 13 years ago by Paul McMillan (next)
Note: See TracTickets for help on using tickets.
Back to Top