1
0
mirror of https://github.com/django/django.git synced 2025-07-05 18:29:11 +00:00

Fixed a security issue related to password resets

Full disclosure and new release are forthcoming

backport from master
This commit is contained in:
Preston Holmes 2012-10-17 14:43:08 -07:00
parent c718b4a036
commit b45c377f8f
4 changed files with 46 additions and 1 deletions

View File

@ -19,6 +19,7 @@ urlpatterns = urlpatterns + patterns('',
(r'^logout/next_page/$', 'django.contrib.auth.views.logout', dict(next_page='/somewhere/')), (r'^logout/next_page/$', 'django.contrib.auth.views.logout', dict(next_page='/somewhere/')),
(r'^remote_user/$', remote_user_auth_view), (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'^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_required(password_reset)),
(r'^login_required_login_url/$', login_required(password_reset, login_url='/somewhere/')), (r'^login_required_login_url/$', login_required(password_reset, login_url='/somewhere/')),
) )

View File

@ -9,6 +9,7 @@ from django.contrib.sites.models import Site, RequestSite
from django.contrib.auth.models import User from django.contrib.auth.models import User
from django.test import TestCase from django.test import TestCase
from django.core import mail from django.core import mail
from django.core.exceptions import SuspiciousOperation
from django.core.urlresolvers import reverse from django.core.urlresolvers import reverse
from django.http import QueryDict from django.http import QueryDict
@ -69,6 +70,44 @@ class PasswordResetTest(AuthViewsTestCase):
self.assertEqual(len(mail.outbox), 1) self.assertEqual(len(mail.outbox), 1)
self.assertEqual("staffmember@example.com", mail.outbox[0].from_email) 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): def _test_confirm_start(self):
# Start by creating the email # Start by creating the email
response = self.client.post('/password_reset/', {'email': 'staffmember@example.com'}) response = self.client.post('/password_reset/', {'email': 'staffmember@example.com'})

View File

@ -151,7 +151,7 @@ def password_reset(request, is_admin_site=False,
'request': request, 'request': request,
} }
if is_admin_site: if is_admin_site:
opts = dict(opts, domain_override=request.META['HTTP_HOST']) opts = dict(opts, domain_override=request.get_host())
form.save(**opts) form.save(**opts)
return HttpResponseRedirect(post_reset_redirect) return HttpResponseRedirect(post_reset_redirect)
else: else:

View File

@ -165,6 +165,11 @@ class HttpRequest(object):
server_port = str(self.META['SERVER_PORT']) server_port = str(self.META['SERVER_PORT'])
if server_port != (self.is_secure() and '443' or '80'): if server_port != (self.is_secure() and '443' or '80'):
host = '%s:%s' % (host, server_port) host = '%s:%s' % (host, server_port)
# Disallow potentially poisoned hostnames.
if set(';/?@&=+$,').intersection(host):
raise SuspiciousOperation('Invalid HTTP_HOST header: %s' % host)
return host return host
def get_full_path(self): def get_full_path(self):