Commit d228c119 authored by Preston Holmes's avatar Preston Holmes
Browse files

Fixed #19866 -- Added security logger and return 400 for SuspiciousOperation. 

SuspiciousOperations have been differentiated into subclasses, and
are now logged to a 'django.security.*' logger. SuspiciousOperations
that reach django.core.handlers.base.BaseHandler will now return a 400
instead of a 500.

Thanks to tiwoc for the report, and Carl Meyer and Donald Stufft
for review.
parent 36d47f72

django/conf/urls/__init__.py

+2 −1
Original line number Diff line number Diff line
@@ -5,8 +5,9 @@ from django.utils.importlib import import_module 
from django.utils import six





__all__ = ['handler403', 'handler404', 'handler500', 'include', 'patterns', 'url']

__all__ = ['handler400', 'handler403', 'handler404', 'handler500', 'include', 'patterns', 'url']



handler400 = 'django.views.defaults.bad_request'

handler403 = 'django.views.defaults.permission_denied'

handler404 = 'django.views.defaults.page_not_found'

handler500 = 'django.views.defaults.server_error'

django/contrib/admin/exceptions.py

0 → 100644
+6 −0
Original line number Diff line number Diff line 
from django.core.exceptions import SuspiciousOperation





class DisallowedModelAdminLookup(SuspiciousOperation):

    """Invalid filter was passed to admin view via URL querystring"""

    pass

django/contrib/admin/views/main.py

+2 −1
Original line number Diff line number Diff line
@@ -14,6 +14,7 @@ from django.utils.translation import ugettext, ugettext_lazy 
from django.utils.http import urlencode



from django.contrib.admin import FieldListFilter

from django.contrib.admin.exceptions import DisallowedModelAdminLookup

from django.contrib.admin.options import IncorrectLookupParameters

from django.contrib.admin.util import (quote, get_fields_from_path,

    lookup_needs_distinct, prepare_lookup_value)
@@ -128,7 +129,7 @@ class ChangeList(six.with_metaclass(RenameChangeListMethods)): 
                lookup_params[force_str(key)] = value



            if not self.model_admin.lookup_allowed(key, value):

                raise SuspiciousOperation("Filtering by %s not allowed" % key)

                raise DisallowedModelAdminLookup("Filtering by %s not allowed" % key)



        filter_specs = []

        if self.list_filter:

django/contrib/auth/tests/test_views.py

+21 −15
Original line number Diff line number Diff line
@@ -10,7 +10,6 @@ from django.conf import global_settings, settings 
from django.contrib.sites.models import Site, RequestSite

from django.contrib.auth.models import User

from django.core import mail

from django.core.exceptions import SuspiciousOperation

from django.core.urlresolvers import reverse, NoReverseMatch

from django.http import QueryDict, HttpRequest

from django.utils.encoding import force_text
@@ -18,7 +17,7 @@ from django.utils.html import escape 
from django.utils.http import urlquote

from django.utils._os import upath

from django.test import TestCase

from django.test.utils import override_settings

from django.test.utils import override_settings, patch_logger

from django.middleware.csrf import CsrfViewMiddleware

from django.contrib.sessions.middleware import SessionMiddleware
@@ -155,23 +154,28 @@ class PasswordResetTest(AuthViewsTestCase): 
        # 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.

        with self.assertRaises(SuspiciousOperation):

            self.client.post('/password_reset/',

        with patch_logger('django.security.DisallowedHost', 'error') as logger_calls:

            response = self.client.post('/password_reset/',

                    {'email': 'staffmember@example.com'},

                    HTTP_HOST='www.example:dr.frankenstein@evil.tld'

                )

            self.assertEqual(response.status_code, 400)

            self.assertEqual(len(mail.outbox), 0)

            self.assertEqual(len(logger_calls), 1)



    # Skip any 500 handler action (like sending more mail...)

    @override_settings(DEBUG_PROPAGATE_EXCEPTIONS=True)

    def test_poisoned_http_host_admin_site(self):

        "Poisoned HTTP_HOST headers can't be used for reset emails on admin views"

        with self.assertRaises(SuspiciousOperation):

            self.client.post('/admin_password_reset/',

        with patch_logger('django.security.DisallowedHost', 'error') as logger_calls:

            response = self.client.post('/admin_password_reset/',

                    {'email': 'staffmember@example.com'},

                    HTTP_HOST='www.example:dr.frankenstein@evil.tld'

                )

            self.assertEqual(response.status_code, 400)

            self.assertEqual(len(mail.outbox), 0)

            self.assertEqual(len(logger_calls), 1)





    def _test_confirm_start(self):

        # Start by creating the email
@@ -678,5 +682,7 @@ class ChangelistTests(AuthViewsTestCase): 
        self.login()



        # A lookup that tries to filter on password isn't OK

        with self.assertRaises(SuspiciousOperation):

        with patch_logger('django.security.DisallowedModelAdminLookup', 'error') as logger_calls:

            response = self.client.get('/admin/auth/user/?password__startswith=sha1$')

            self.assertEqual(response.status_code, 400)

            self.assertEqual(len(logger_calls), 1)

django/contrib/formtools/exceptions.py

0 → 100644
+6 −0
Original line number Diff line number Diff line 
from django.core.exceptions import SuspiciousOperation





class WizardViewCookieModified(SuspiciousOperation):

    """Signature of cookie modified"""

    pass