Skip to content

Commit

Permalink
[3.0.x] Fixed CVE-2019-19844 -- Used verified user email for password…
Browse files Browse the repository at this point in the history
… reset requests.

Backport of 5b1fbce from master.

Co-Authored-By: Florian Apolloner <florian@apolloner.eu>
  • Loading branch information
2 people authored and felixxm committed Dec 18, 2019
1 parent 33d2cda commit 302a4ff
Show file tree
Hide file tree
Showing 5 changed files with 110 additions and 10 deletions.
24 changes: 20 additions & 4 deletions django/contrib/auth/forms.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,15 @@
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
Expand Down Expand Up @@ -269,11 +278,16 @@ def get_users(self, email):
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',
Expand All @@ -286,15 +300,17 @@ def save(self, domain_override=None,
user.
"""
email = self.cleaned_data["email"]
email_field_name = UserModel.get_email_field_name()
for user in self.get_users(email):
if not domain_override:
current_site = get_current_site(request)
site_name = current_site.name
domain = current_site.domain
else:
site_name = domain = domain_override
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)),
Expand All @@ -305,7 +321,7 @@ def save(self, domain_override=None,
}
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,
)


Expand Down
20 changes: 18 additions & 2 deletions docs/releases/1.11.27.txt
Original file line number Diff line number Diff line change
Expand Up @@ -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
========
Expand Down
20 changes: 18 additions & 2 deletions docs/releases/2.2.9.txt
Original file line number Diff line number Diff line change
Expand Up @@ -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
========
Expand Down
20 changes: 18 additions & 2 deletions docs/releases/3.0.1.txt
Original file line number Diff line number Diff line change
Expand Up @@ -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
========
Expand Down
36 changes: 36 additions & 0 deletions tests/auth_tests/test_forms.py
Original file line number Diff line number Diff line change
Expand Up @@ -804,6 +804,42 @@ def test_invalid_email(self):
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
Expand Down

0 comments on commit 302a4ff

Please sign in to comment.