From 8c35a0a903fd979e3262fe300ca084ffbfb300d6 Mon Sep 17 00:00:00 2001 From: Natalia <124304+nessita@users.noreply.github.com> Date: Mon, 19 Aug 2024 14:47:38 -0300 Subject: [PATCH] Fixed CVE-2024-45231 -- Avoided server error on password reset when email sending fails. On successful submission of a password reset request, an email is sent to the accounts known to the system. If sending this email fails (due to email backend misconfiguration, service provider outage, network issues, etc.), an attacker might exploit this by detecting which password reset requests succeed and which ones generate a 500 error response. Thanks to Thibaut Spriet for the report, and to Mariusz Felisiak, Adam Johnson, and Sarah Boyce for the reviews. --- django/contrib/auth/forms.py | 9 ++++++++- docs/ref/logging.txt | 12 ++++++++++++ docs/releases/4.2.16.txt | 11 +++++++++++ docs/releases/5.0.9.txt | 11 +++++++++++ docs/releases/5.1.1.txt | 11 +++++++++++ docs/topics/auth/default.txt | 4 +++- tests/auth_tests/test_forms.py | 21 +++++++++++++++++++++ tests/mail/custombackend.py | 6 ++++++ 8 files changed, 83 insertions(+), 2 deletions(-) diff --git a/django/contrib/auth/forms.py b/django/contrib/auth/forms.py index a11668944a8..edf672a6e51 100644 --- a/django/contrib/auth/forms.py +++ b/django/contrib/auth/forms.py @@ -1,3 +1,4 @@ +import logging import unicodedata from django import forms @@ -16,6 +17,7 @@ from django.utils.translation import gettext from django.utils.translation import gettext_lazy as _ UserModel = get_user_model() +logger = logging.getLogger("django.contrib.auth") def _unicode_ci_compare(s1, s2): @@ -418,7 +420,12 @@ class PasswordResetForm(forms.Form): html_email = loader.render_to_string(html_email_template_name, context) email_message.attach_alternative(html_email, "text/html") - email_message.send() + try: + email_message.send() + except Exception: + logger.exception( + "Failed to send password reset email to %s", context["user"].pk + ) def get_users(self, email): """Given an email, return matching user(s) who should receive a reset. diff --git a/docs/ref/logging.txt b/docs/ref/logging.txt index 8a7e58997eb..24ab5d97f37 100644 --- a/docs/ref/logging.txt +++ b/docs/ref/logging.txt @@ -209,6 +209,18 @@ Django development server. This logger generates an ``INFO`` message upon detecting a modification in a source code file and may produce ``WARNING`` messages during filesystem inspection and event subscription processes. +.. _django-contrib-auth-logger: + +``django.contrib.auth`` +~~~~~~~~~~~~~~~~~~~~~~~ + +.. versionadded:: 4.2.16 + +Log messages related to :doc:`contrib/auth`, particularly ``ERROR`` messages +are generated when a :class:`~django.contrib.auth.forms.PasswordResetForm` is +successfully submitted but the password reset email cannot be delivered due to +a mail sending exception. + .. _django-contrib-gis-logger: ``django.contrib.gis`` diff --git a/docs/releases/4.2.16.txt b/docs/releases/4.2.16.txt index 2a84186867c..963036345c5 100644 --- a/docs/releases/4.2.16.txt +++ b/docs/releases/4.2.16.txt @@ -13,3 +13,14 @@ CVE-2024-45230: Potential denial-of-service vulnerability in ``django.utils.html :tfilter:`urlize` and :tfilter:`urlizetrunc` were subject to a potential denial-of-service attack via very large inputs with a specific sequence of characters. + +CVE-2024-45231: Potential user email enumeration via response status on password reset +====================================================================================== + +Due to unhandled email sending failures, the +:class:`~django.contrib.auth.forms.PasswordResetForm` class allowed remote +attackers to enumerate user emails by issuing password reset requests and +observing the outcomes. + +To mitigate this risk, exceptions occurring during password reset email sending +are now handled and logged using the :ref:`django-contrib-auth-logger` logger. diff --git a/docs/releases/5.0.9.txt b/docs/releases/5.0.9.txt index 50e94ea3f2e..52595ae4ffd 100644 --- a/docs/releases/5.0.9.txt +++ b/docs/releases/5.0.9.txt @@ -13,3 +13,14 @@ CVE-2024-45230: Potential denial-of-service vulnerability in ``django.utils.html :tfilter:`urlize` and :tfilter:`urlizetrunc` were subject to a potential denial-of-service attack via very large inputs with a specific sequence of characters. + +CVE-2024-45231: Potential user email enumeration via response status on password reset +====================================================================================== + +Due to unhandled email sending failures, the +:class:`~django.contrib.auth.forms.PasswordResetForm` class allowed remote +attackers to enumerate user emails by issuing password reset requests and +observing the outcomes. + +To mitigate this risk, exceptions occurring during password reset email sending +are now handled and logged using the :ref:`django-contrib-auth-logger` logger. diff --git a/docs/releases/5.1.1.txt b/docs/releases/5.1.1.txt index dd249291004..8c0468d7271 100644 --- a/docs/releases/5.1.1.txt +++ b/docs/releases/5.1.1.txt @@ -14,6 +14,17 @@ CVE-2024-45230: Potential denial-of-service vulnerability in ``django.utils.html denial-of-service attack via very large inputs with a specific sequence of characters. +CVE-2024-45231: Potential user email enumeration via response status on password reset +====================================================================================== + +Due to unhandled email sending failures, the +:class:`~django.contrib.auth.forms.PasswordResetForm` class allowed remote +attackers to enumerate user emails by issuing password reset requests and +observing the outcomes. + +To mitigate this risk, exceptions occurring during password reset email sending +are now handled and logged using the :ref:`django-contrib-auth-logger` logger. + Bugfixes ======== diff --git a/docs/topics/auth/default.txt b/docs/topics/auth/default.txt index 3ea21fac914..a22cebbf15a 100644 --- a/docs/topics/auth/default.txt +++ b/docs/topics/auth/default.txt @@ -1732,7 +1732,9 @@ provides several built-in forms located in :mod:`django.contrib.auth.forms`: .. method:: send_mail(subject_template_name, email_template_name, context, from_email, to_email, html_email_template_name=None) Uses the arguments to send an ``EmailMultiAlternatives``. - Can be overridden to customize how the email is sent to the user. + Can be overridden to customize how the email is sent to the user. If + you choose to override this method, be mindful of handling potential + exceptions raised due to email sending failures. :param subject_template_name: the template for the subject. :param email_template_name: the template for the email body. diff --git a/tests/auth_tests/test_forms.py b/tests/auth_tests/test_forms.py index 87267cc0a78..5d81d8f7fd2 100644 --- a/tests/auth_tests/test_forms.py +++ b/tests/auth_tests/test_forms.py @@ -1357,6 +1357,27 @@ class PasswordResetFormTest(TestDataMixin, TestCase): ) ) + @override_settings(EMAIL_BACKEND="mail.custombackend.FailingEmailBackend") + def test_save_send_email_exceptions_are_catched_and_logged(self): + (user, username, email) = self.create_dummy_user() + form = PasswordResetForm({"email": email}) + self.assertTrue(form.is_valid()) + + with self.assertLogs("django.contrib.auth", level=0) as cm: + form.save() + + self.assertEqual(len(mail.outbox), 0) + self.assertEqual(len(cm.output), 1) + errors = cm.output[0].split("\n") + pk = user.pk + self.assertEqual( + errors[0], + f"ERROR:django.contrib.auth:Failed to send password reset email to {pk}", + ) + self.assertEqual( + errors[-1], "ValueError: FailingEmailBackend is doomed to fail." + ) + @override_settings(AUTH_USER_MODEL="auth_tests.CustomEmailField") def test_custom_email_field(self): email = "test@mail.com" diff --git a/tests/mail/custombackend.py b/tests/mail/custombackend.py index 14e7f077ba7..c63f1c07b5b 100644 --- a/tests/mail/custombackend.py +++ b/tests/mail/custombackend.py @@ -12,3 +12,9 @@ class EmailBackend(BaseEmailBackend): # Messages are stored in an instance variable for testing. self.test_outbox.extend(email_messages) return len(email_messages) + + +class FailingEmailBackend(BaseEmailBackend): + + def send_messages(self, email_messages): + raise ValueError("FailingEmailBackend is doomed to fail.")