From c37bb28677295f6edda61d8ac461014ef0d3aeb2 Mon Sep 17 00:00:00 2001 From: Tim Graham Date: Tue, 23 Jan 2018 13:20:18 -0500 Subject: [PATCH] [2.0.x] Fixed CVE-2018-6188 -- Fixed information leakage in AuthenticationForm. Reverted 359370a8b8ca0efe99b1d4630b291ec060b69225 (refs #28645). This is a security fix. --- django/contrib/auth/forms.py | 9 --------- docs/releases/1.11.10.txt | 23 +++++++++++++++++++++-- docs/releases/2.0.2.txt | 23 +++++++++++++++++++++-- tests/auth_tests/test_forms.py | 21 +++++++++++++++++++++ 4 files changed, 63 insertions(+), 13 deletions(-) diff --git a/django/contrib/auth/forms.py b/django/contrib/auth/forms.py index 5a83110a4b7..a5de5bf6505 100644 --- a/django/contrib/auth/forms.py +++ b/django/contrib/auth/forms.py @@ -194,15 +194,6 @@ class AuthenticationForm(forms.Form): if username is not None and password: self.user_cache = authenticate(self.request, username=username, password=password) if self.user_cache is None: - # An authentication backend may reject inactive users. Check - # if the user exists and is inactive, and raise the 'inactive' - # error if so. - try: - self.user_cache = UserModel._default_manager.get_by_natural_key(username) - except UserModel.DoesNotExist: - pass - else: - self.confirm_login_allowed(self.user_cache) raise forms.ValidationError( self.error_messages['invalid_login'], code='invalid_login', diff --git a/docs/releases/1.11.10.txt b/docs/releases/1.11.10.txt index cfa8fc2070a..96f07920a36 100644 --- a/docs/releases/1.11.10.txt +++ b/docs/releases/1.11.10.txt @@ -2,9 +2,28 @@ Django 1.11.10 release notes ============================ -*Expected February 1, 2018* +*February 1, 2018* -Django 1.11.10 fixes several bugs in 1.11.9. +Django 1.11.10 fixes a security issue and several bugs in 1.11.9. + +CVE-2018-6188: Information leakage in ``AuthenticationForm`` +============================================================ + +A regression in Django 1.11.8 made +:class:`~django.contrib.auth.forms.AuthenticationForm` run its +``confirm_login_allowed()`` method even if an incorrect password is entered. +This can leak information about a user, depending on what messages +``confirm_login_allowed()`` raises. If ``confirm_login_allowed()`` isn't +overridden, an attacker enter an arbitrary username and see if that user has +been set to ``is_active=False``. If ``confirm_login_allowed()`` is overridden, +more sensitive details could be leaked. + +This issue is fixed with the caveat that ``AuthenticationForm`` can no longer +raise the "This account is inactive." error if the authentication backend +rejects inactive users (the default authentication backend, ``ModelBackend``, +has done that since Django 1.10). This issue will be revisited for Django 2.1 +as a fix to address the caveat will likely be too invasive for inclusion in +older versions. Bugfixes ======== diff --git a/docs/releases/2.0.2.txt b/docs/releases/2.0.2.txt index 562f30995f6..475ddfb23aa 100644 --- a/docs/releases/2.0.2.txt +++ b/docs/releases/2.0.2.txt @@ -2,9 +2,28 @@ Django 2.0.2 release notes ========================== -*Expected February 1, 2018* +*February 1, 2018* -Django 2.0.2 fixes several bugs in 2.0.1. +Django 2.0.2 fixes a security issue and several bugs in 2.0.1. + +CVE-2018-6188: Information leakage in ``AuthenticationForm`` +============================================================ + +A regression in Django 1.11.8 made +:class:`~django.contrib.auth.forms.AuthenticationForm` run its +``confirm_login_allowed()`` method even if an incorrect password is entered. +This can leak information about a user, depending on what messages +``confirm_login_allowed()`` raises. If ``confirm_login_allowed()`` isn't +overridden, an attacker enter an arbitrary username and see if that user has +been set to ``is_active=False``. If ``confirm_login_allowed()`` is overridden, +more sensitive details could be leaked. + +This issue is fixed with the caveat that ``AuthenticationForm`` can no longer +raise the "This account is inactive." error if the authentication backend +rejects inactive users (the default authentication backend, ``ModelBackend``, +has done that since Django 1.10). This issue will be revisited for Django 2.1 +as a fix to address the caveat will likely be too invasive for inclusion in +older versions. Bugfixes ======== diff --git a/tests/auth_tests/test_forms.py b/tests/auth_tests/test_forms.py index 7d8b5f74860..7690b625148 100644 --- a/tests/auth_tests/test_forms.py +++ b/tests/auth_tests/test_forms.py @@ -262,6 +262,9 @@ class UserCreationFormTest(TestDataMixin, TestCase): ) +# To verify that the login form rejects inactive users, use an authentication +# backend that allows them. +@override_settings(AUTHENTICATION_BACKENDS=['django.contrib.auth.backends.AllowAllUsersModelBackend']) class AuthenticationFormTest(TestDataMixin, TestCase): def test_invalid_username(self): @@ -291,6 +294,24 @@ class AuthenticationFormTest(TestDataMixin, TestCase): self.assertFalse(form.is_valid()) self.assertEqual(form.non_field_errors(), [str(form.error_messages['inactive'])]) + # Use an authentication backend that rejects inactive users. + @override_settings(AUTHENTICATION_BACKENDS=['django.contrib.auth.backends.ModelBackend']) + def test_inactive_user_incorrect_password(self): + """An invalid login doesn't leak the inactive status of a user.""" + data = { + 'username': 'inactive', + 'password': 'incorrect', + } + form = AuthenticationForm(None, data) + self.assertFalse(form.is_valid()) + self.assertEqual( + form.non_field_errors(), [ + form.error_messages['invalid_login'] % { + 'username': User._meta.get_field('username').verbose_name + } + ] + ) + def test_login_failed(self): signal_calls = []