Opened 11 years ago

Closed 11 years ago

#20411 closed Bug (fixed)

Invalid Referer header blows up on CSRF protection middleware

Reported by: André Cruz Owned by: Steffen Zieger
Component: HTTP handling Version: 1.5
Severity: Normal Keywords: referer valueerror csrf
Cc: bmispelon@… Triage Stage: Ready for checkin
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

If a client sends an invalid Referer header such as 'http://http://xxx.pt/', the CSRF middleware will blow up with an error:

ERROR 2013-05-15 17:38:56,542 django.request:212 22023 140475533584128 Internal Server Error: /
Traceback (most recent call last):
  File "/servers/python-environments/discosite/local/lib/python2.7/site-packages/django/core/handlers/base.py", line 109, in get_response
    response = middleware_method(request, callback, callback_args, callback_kwargs)
  File "/servers/python-environments/discosite/local/lib/python2.7/site-packages/django/middleware/csrf.py", line 148, in process_view
    if not same_origin(referer, good_referer):
  File "/servers/python-environments/discosite/local/lib/python2.7/site-packages/django/utils/http.py", line 229, in same_origin
    return (p1.scheme, p1.hostname, p1.port) == (p2.scheme, p2.hostname, p2.port)
  File "/usr/lib/python2.7/urlparse.py", line 110, in port
    port = int(port, 10)
ValueError: invalid literal for int() with base 10: ''

Either we catch the Exception or we are more careful when comparing.

Attachments (2)

test_20411.diff (1.0 KB ) - added by Baptiste Mispelon 11 years ago.
Minimal test case for #20411
20411_fix_exception_invalid_referer.diff (1.6 KB ) - added by Steffen Zieger 11 years ago.
Fix for ValueError exception if referer is invalid

Download all attachments as: .zip

Change History (9)

comment:1 by Claude Paroz, 11 years ago

Triage Stage: UnreviewedAccepted

by Baptiste Mispelon, 11 years ago

Attachment: test_20411.diff added

Minimal test case for #20411

comment:2 by Baptiste Mispelon, 11 years ago

Cc: bmispelon@… added

Hi,

Thanks for your report.

I added a small test case to reproduce the issue.

The problem happens when trying to access the port part of a malformed urlparse result.

There's two places where this could be fixed in django:

1) In django.utils.http.same_origin
2) In django.middleware.csrf.CsrfViewMiddleware.process_view

I'm not sure which one is the best place to fix this. I'd be inclined to go
with 1) and add a try/except around the return statement, catch a ValueError
and return False in that case.

For the tests, it might be worth it to test a wider ranger of malformed hosts
(I'm not sure if it really applies, but you could check what the tests for ALLOWED_HOSTS test against [1]).

[1] https://github.com/django/django/blob/master/tests/requests/tests.py#L183-L190

comment:3 by Claude Paroz, 11 years ago

+1 for the 1) approach (and returning False in case of errors).

comment:4 by Steffen Zieger, 11 years ago

Owner: changed from nobody to Steffen Zieger
Status: newassigned

by Steffen Zieger, 11 years ago

Fix for ValueError exception if referer is invalid

comment:5 by Steffen Zieger, 11 years ago

Has patch: set

comment:6 by Baptiste Mispelon, 11 years ago

Triage Stage: AcceptedReady for checkin

Patch looks good. Marking this as Ready For Checkin.

comment:7 by Florian Apolloner <florian@…>, 11 years ago

Resolution: fixed
Status: assignedclosed

In 051cb1f4c60ac8e7087d92ef34ed41e6684d8b9b:

Fixed #20411 -- Don't let invalid referers blow up CSRF same origin checks.

Thanks to edevil for the report and saz for the patch.

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