From 5b1fbcef7a8bec991ebe7b2a18b5d5a95d72cb70 Mon Sep 17 00:00:00 2001 From: Simon Charette Date: Mon, 16 Dec 2019 21:51:57 -0500 Subject: [PATCH] Fixed CVE-2019-19844 -- Used verified user email for password reset requests. Co-Authored-By: Florian Apolloner --- django/contrib/auth/forms.py | 24 +++++++++++++++++++---- docs/releases/1.11.27.txt | 20 +++++++++++++++++-- docs/releases/2.2.9.txt | 20 +++++++++++++++++-- docs/releases/3.0.1.txt | 20 +++++++++++++++++-- tests/auth_tests/test_forms.py | 36 ++++++++++++++++++++++++++++++++++ 5 files changed, 110 insertions(+), 10 deletions(-) diff --git a/django/contrib/auth/forms.py b/django/contrib/auth/forms.py index 54e85d5a33..e5e6a86c1e 100644 --- a/django/contrib/auth/forms.py +++ b/django/contrib/auth/forms.py @@ -20,6 +20,15 @@ from django.utils.translation import gettext, gettext_lazy as _ UserModel = get_user_model() +def _unicode_ci_compare(s1, s2): + """ + Perform case-insensitive comparison of two identifiers, using the + recommended algorithm from Unicode Technical Report 36, section + 2.11.2(B)(2). + """ + return unicodedata.normalize('NFKC', s1).casefold() == unicodedata.normalize('NFKC', s2).casefold() + + class ReadOnlyPasswordHashWidget(forms.Widget): template_name = 'auth/widgets/read_only_password_hash.html' read_only = True @@ -269,11 +278,16 @@ class PasswordResetForm(forms.Form): that prevent inactive users and users with unusable passwords from resetting their password. """ + email_field_name = UserModel.get_email_field_name() active_users = UserModel._default_manager.filter(**{ - '%s__iexact' % UserModel.get_email_field_name(): email, + '%s__iexact' % email_field_name: email, 'is_active': True, }) - return (u for u in active_users if u.has_usable_password()) + return ( + u for u in active_users + if u.has_usable_password() and + _unicode_ci_compare(email, getattr(u, email_field_name)) + ) def save(self, domain_override=None, subject_template_name='registration/password_reset_subject.txt', @@ -292,9 +306,11 @@ class PasswordResetForm(forms.Form): domain = current_site.domain else: site_name = domain = domain_override + email_field_name = UserModel.get_email_field_name() for user in self.get_users(email): + user_email = getattr(user, email_field_name) context = { - 'email': email, + 'email': user_email, 'domain': domain, 'site_name': site_name, 'uid': urlsafe_base64_encode(force_bytes(user.pk)), @@ -305,7 +321,7 @@ class PasswordResetForm(forms.Form): } self.send_mail( subject_template_name, email_template_name, context, from_email, - email, html_email_template_name=html_email_template_name, + user_email, html_email_template_name=html_email_template_name, ) diff --git a/docs/releases/1.11.27.txt b/docs/releases/1.11.27.txt index cb4329afdb..6197dee1f6 100644 --- a/docs/releases/1.11.27.txt +++ b/docs/releases/1.11.27.txt @@ -2,9 +2,25 @@ Django 1.11.27 release notes ============================ -*Expected January 2, 2020* +*December 18, 2019* -Django 1.11.27 fixes a data loss bug in 1.11.26. +Django 1.11.27 fixes a security issue and a data loss bug in 1.11.26. + +CVE-2019-19844: Potential account hijack via password reset form +================================================================ + +By submitting a suitably crafted email address making use of Unicode +characters, that compared equal to an existing user email when lower-cased for +comparison, an attacker could be sent a password reset token for the matched +account. + +In order to avoid this vulnerability, password reset requests now compare the +submitted email using the stricter, recommended algorithm for case-insensitive +comparison of two identifiers from `Unicode Technical Report 36, section +2.11.2(B)(2)`__. Upon a match, the email containing the reset token will be +sent to the email address on record rather than the submitted address. + +.. __: https://www.unicode.org/reports/tr36/#Recommendations_General Bugfixes ======== diff --git a/docs/releases/2.2.9.txt b/docs/releases/2.2.9.txt index efd0cb4e1c..25a9374194 100644 --- a/docs/releases/2.2.9.txt +++ b/docs/releases/2.2.9.txt @@ -2,9 +2,25 @@ Django 2.2.9 release notes ========================== -*Expected January 2, 2020* +*December 18, 2019* -Django 2.2.9 fixes a data loss bug in 2.2.8. +Django 2.2.9 fixes a security issue and a data loss bug in 2.2.8. + +CVE-2019-19844: Potential account hijack via password reset form +================================================================ + +By submitting a suitably crafted email address making use of Unicode +characters, that compared equal to an existing user email when lower-cased for +comparison, an attacker could be sent a password reset token for the matched +account. + +In order to avoid this vulnerability, password reset requests now compare the +submitted email using the stricter, recommended algorithm for case-insensitive +comparison of two identifiers from `Unicode Technical Report 36, section +2.11.2(B)(2)`__. Upon a match, the email containing the reset token will be +sent to the email address on record rather than the submitted address. + +.. __: https://www.unicode.org/reports/tr36/#Recommendations_General Bugfixes ======== diff --git a/docs/releases/3.0.1.txt b/docs/releases/3.0.1.txt index b923661bd2..1c32528304 100644 --- a/docs/releases/3.0.1.txt +++ b/docs/releases/3.0.1.txt @@ -2,9 +2,25 @@ Django 3.0.1 release notes ========================== -*Expected January 2, 2020* +*December 18, 2019* -Django 3.0.1 fixes several bugs in 3.0. +Django 3.0.1 fixes a security issue and several bugs in 3.0. + +CVE-2019-19844: Potential account hijack via password reset form +================================================================ + +By submitting a suitably crafted email address making use of Unicode +characters, that compared equal to an existing user email when lower-cased for +comparison, an attacker could be sent a password reset token for the matched +account. + +In order to avoid this vulnerability, password reset requests now compare the +submitted email using the stricter, recommended algorithm for case-insensitive +comparison of two identifiers from `Unicode Technical Report 36, section +2.11.2(B)(2)`__. Upon a match, the email containing the reset token will be +sent to the email address on record rather than the submitted address. + +.. __: https://www.unicode.org/reports/tr36/#Recommendations_General Bugfixes ======== diff --git a/tests/auth_tests/test_forms.py b/tests/auth_tests/test_forms.py index 40e3050144..4e5f8e3094 100644 --- a/tests/auth_tests/test_forms.py +++ b/tests/auth_tests/test_forms.py @@ -804,6 +804,42 @@ class PasswordResetFormTest(TestDataMixin, TestCase): self.assertFalse(form.is_valid()) self.assertEqual(form['email'].errors, [_('Enter a valid email address.')]) + def test_user_email_unicode_collision(self): + User.objects.create_user('mike123', 'mike@example.org', 'test123') + User.objects.create_user('mike456', 'mıke@example.org', 'test123') + data = {'email': 'mıke@example.org'} + form = PasswordResetForm(data) + self.assertTrue(form.is_valid()) + form.save() + self.assertEqual(len(mail.outbox), 1) + self.assertEqual(mail.outbox[0].to, ['mıke@example.org']) + + def test_user_email_domain_unicode_collision(self): + User.objects.create_user('mike123', 'mike@ixample.org', 'test123') + User.objects.create_user('mike456', 'mike@ıxample.org', 'test123') + data = {'email': 'mike@ıxample.org'} + form = PasswordResetForm(data) + self.assertTrue(form.is_valid()) + form.save() + self.assertEqual(len(mail.outbox), 1) + self.assertEqual(mail.outbox[0].to, ['mike@ıxample.org']) + + def test_user_email_unicode_collision_nonexistent(self): + User.objects.create_user('mike123', 'mike@example.org', 'test123') + data = {'email': 'mıke@example.org'} + form = PasswordResetForm(data) + self.assertTrue(form.is_valid()) + form.save() + self.assertEqual(len(mail.outbox), 0) + + def test_user_email_domain_unicode_collision_nonexistent(self): + User.objects.create_user('mike123', 'mike@ixample.org', 'test123') + data = {'email': 'mike@ıxample.org'} + form = PasswordResetForm(data) + self.assertTrue(form.is_valid()) + form.save() + self.assertEqual(len(mail.outbox), 0) + def test_nonexistent_email(self): """ Test nonexistent email address. This should not fail because it would