Attachment 625210 Details for Bug 865164 – Upstream patch to correct the flaw in Django 1.3.x.

[patch] Upstream patch to correct the flaw in Django 1.3.x.

host_poisoning-1.3.x.diff (text/plain), 8.28 KB, created by Vincent Danen on 2012-10-10 22:11:25 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Vincent Danen

Created: 2012-10-10 22:11:25 UTC

Size: 8.28 KB

patch

obsolete

>diff --git a/django/contrib/auth/tests/urls.py b/django/contrib/auth/tests/urls.py
>index 3d76a4e..c01964f 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 b03489c..1649f76 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 eba83a2..727e916 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 2dfe12e..dddd9a8 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):
>diff --git a/tests/regressiontests/requests/tests.py b/tests/regressiontests/requests/tests.py
>index cd488e2..758c652 100644
>--- a/tests/regressiontests/requests/tests.py
>+++ b/tests/regressiontests/requests/tests.py
>@@ -4,6 +4,7 @@ from StringIO import StringIO
> 
> from django.conf import settings
> from django.core.handlers.modpython import ModPythonRequest
>+from django.core.exceptions import SuspiciousOperation
> from django.core.handlers.wsgi import WSGIRequest, LimitedStream
> from django.http import HttpRequest, HttpResponse, parse_cookie
> from django.utils import unittest
>@@ -101,6 +102,39 @@ class RequestsTests(unittest.TestCase):
>             }
>             self.assertEqual(request.get_host(), 'internal.com:8042')
> 
>+            # Poisoned host headers are rejected as suspicious
>+            legit_hosts = [
>+                'example.com',
>+                'example.com:80',
>+                '12.34.56.78',
>+                '12.34.56.78:443',
>+                '[2001:19f0:feee::dead:beef:cafe]',
>+                '[2001:19f0:feee::dead:beef:cafe]:8080',
>+            ]
>+
>+            poisoned_hosts = [
>+                'example.com@evil.tld',
>+                'example.com:dr.frankenstein@evil.tld',
>+                'example.com:someone@somestie.com:80',
>+                'example.com:80/badpath'
>+            ]
>+
>+            for host in legit_hosts:
>+                request = HttpRequest()
>+                request.META = {
>+                    'HTTP_HOST': host,
>+                }
>+                request.get_host()
>+
>+            for host in poisoned_hosts:
>+                def test_host_poisoning():
>+                    request = HttpRequest()
>+                    request.META = {
>+                        'HTTP_HOST': host,
>+                    }
>+                    request.get_host()
>+                self.assertRaises(SuspiciousOperation, test_host_poisoning)
>+
>         finally:
>             settings.USE_X_FORWARDED_HOST = old_USE_X_FORWARDED_HOST
> 
>@@ -145,6 +179,39 @@ class RequestsTests(unittest.TestCase):
>             }
>             self.assertEqual(request.get_host(), 'internal.com:8042')
> 
>+            # Poisoned host headers are rejected as suspicious
>+            legit_hosts = [
>+                'example.com',
>+                'example.com:80',
>+                '12.34.56.78',
>+                '12.34.56.78:443',
>+                '[2001:19f0:feee::dead:beef:cafe]',
>+                '[2001:19f0:feee::dead:beef:cafe]:8080',
>+            ]
>+
>+            poisoned_hosts = [
>+                'example.com@evil.tld',
>+                'example.com:dr.frankenstein@evil.tld',
>+                'example.com:dr.frankenstein@evil.tld:80',
>+                'example.com:80/badpath'
>+            ]
>+
>+            for host in legit_hosts:
>+                request = HttpRequest()
>+                request.META = {
>+                    'HTTP_HOST': host,
>+                }
>+                request.get_host()
>+
>+            for host in poisoned_hosts:
>+                def test_host_poisoning():
>+                    request = HttpRequest()
>+                    request.META = {
>+                        'HTTP_HOST': host,
>+                    }
>+                    request.get_host()
>+                self.assertRaises(SuspiciousOperation, test_host_poisoning)
>+
>         finally:
>             settings.USE_X_FORWARDED_HOST = old_USE_X_FORWARDED_HOST
>

Actions: View | Diff

Attachments on bug 865164: 625210 | 625211