Skip to content

Commit

Permalink
Fixed a security issue related to password resets
Browse files Browse the repository at this point in the history
Full disclosure and new release are forthcoming

backport from master
  • Loading branch information
ptone committed Oct 17, 2012
1 parent c718b4a commit b45c377
Show file tree
Hide file tree
Showing 4 changed files with 46 additions and 1 deletion.
1 change: 1 addition & 0 deletions django/contrib/auth/tests/urls.py
Expand Up @@ -19,6 +19,7 @@ def remote_user_auth_view(request):
(r'^logout/next_page/$', 'django.contrib.auth.views.logout', dict(next_page='/somewhere/')),
(r'^remote_user/$', remote_user_auth_view),
(r'^password_reset_from_email/$', 'django.contrib.auth.views.password_reset', dict(from_email='staffmember@example.com')),
(r'^admin_password_reset/$', 'django.contrib.auth.views.password_reset', dict(is_admin_site=True)),
(r'^login_required/$', login_required(password_reset)),
(r'^login_required_login_url/$', login_required(password_reset, login_url='/somewhere/')),
)
Expand Down
39 changes: 39 additions & 0 deletions django/contrib/auth/tests/views.py
Expand Up @@ -9,6 +9,7 @@
from django.contrib.auth.models import User
from django.test import TestCase
from django.core import mail
from django.core.exceptions import SuspiciousOperation
from django.core.urlresolvers import reverse
from django.http import QueryDict

Expand Down Expand Up @@ -69,6 +70,44 @@ def test_email_found_custom_from(self):
self.assertEqual(len(mail.outbox), 1)
self.assertEqual("staffmember@example.com", mail.outbox[0].from_email)

def test_admin_reset(self):
"If the reset view is marked as being for admin, the HTTP_HOST header is used for a domain override."
response = self.client.post('/admin_password_reset/',
{'email': 'staffmember@example.com'},
HTTP_HOST='adminsite.com'
)
self.assertEqual(response.status_code, 302)
self.assertEqual(len(mail.outbox), 1)
self.assertTrue("http://adminsite.com" in mail.outbox[0].body)
self.assertEqual(settings.DEFAULT_FROM_EMAIL, mail.outbox[0].from_email)

def test_poisoned_http_host(self):
"Poisoned HTTP_HOST headers can't be used for reset emails"
# This attack is based on the way browsers handle URLs. The colon
# should be used to separate the port, but if the URL contains an @,
# the colon is interpreted as part of a username for login purposes,
# making 'evil.com' the request domain. Since HTTP_HOST is used to
# produce a meaningful reset URL, we need to be certain that the
# HTTP_HOST header isn't poisoned. This is done as a check when get_host()
# is invoked, but we check here as a practical consequence.
def test_host_poisoning():
self.client.post('/password_reset/',
{'email': 'staffmember@example.com'},
HTTP_HOST='www.example:dr.frankenstein@evil.tld'
)
self.assertRaises(SuspiciousOperation, test_host_poisoning)
self.assertEqual(len(mail.outbox), 0)

def test_poisoned_http_host_admin_site(self):
"Poisoned HTTP_HOST headers can't be used for reset emails on admin views"
def test_host_poisoning():
self.client.post('/admin_password_reset/',
{'email': 'staffmember@example.com'},
HTTP_HOST='www.example:dr.frankenstein@evil.tld'
)
self.assertRaises(SuspiciousOperation, test_host_poisoning)
self.assertEqual(len(mail.outbox), 0)

def _test_confirm_start(self):
# Start by creating the email
response = self.client.post('/password_reset/', {'email': 'staffmember@example.com'})
Expand Down
2 changes: 1 addition & 1 deletion django/contrib/auth/views.py
Expand Up @@ -151,7 +151,7 @@ def password_reset(request, is_admin_site=False,
'request': request,
}
if is_admin_site:
opts = dict(opts, domain_override=request.META['HTTP_HOST'])
opts = dict(opts, domain_override=request.get_host())
form.save(**opts)
return HttpResponseRedirect(post_reset_redirect)
else:
Expand Down
5 changes: 5 additions & 0 deletions django/http/__init__.py
Expand Up @@ -165,6 +165,11 @@ def get_host(self):
server_port = str(self.META['SERVER_PORT'])
if server_port != (self.is_secure() and '443' or '80'):
host = '%s:%s' % (host, server_port)

# Disallow potentially poisoned hostnames.
if set(';/?@&=+$,').intersection(host):
raise SuspiciousOperation('Invalid HTTP_HOST header: %s' % host)

return host

def get_full_path(self):
Expand Down

0 comments on commit b45c377

Please sign in to comment.