diff --git a/django/contrib/auth/tests/urls.py b/django/contrib/auth/tests/urls.py index 3d76a4e443..c01964f833 100644 --- a/django/contrib/auth/tests/urls.py +++ b/django/contrib/auth/tests/urls.py @@ -19,6 +19,7 @@ urlpatterns = urlpatterns + patterns('', (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/')), ) diff --git a/django/contrib/auth/tests/views.py b/django/contrib/auth/tests/views.py index b03489c7d2..046d00da0b 100644 --- a/django/contrib/auth/tests/views.py +++ b/django/contrib/auth/tests/views.py @@ -9,6 +9,7 @@ from django.contrib.sites.models import Site, RequestSite 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 @@ -69,6 +70,44 @@ class PasswordResetTest(AuthViewsTestCase): 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'}) diff --git a/django/contrib/auth/views.py b/django/contrib/auth/views.py index eba83a269e..727e9162bd 100644 --- a/django/contrib/auth/views.py +++ b/django/contrib/auth/views.py @@ -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: diff --git a/django/http/__init__.py b/django/http/__init__.py index 2dfe12ee4b..dddd9a89c4 100644 --- a/django/http/__init__.py +++ b/django/http/__init__.py @@ -165,6 +165,11 @@ class HttpRequest(object): 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):