Skip to content

Commit

Permalink
Fixed CVE-2018-16984 -- Fixed password hash disclosure to admin "view…
Browse files Browse the repository at this point in the history
… only" users.

Thanks Claude Paroz & Tim Graham for collaborating on the patch.
  • Loading branch information
carltongibson committed Oct 1, 2018
1 parent a4932be commit bf39978
Show file tree
Hide file tree
Showing 4 changed files with 44 additions and 3 deletions.
6 changes: 6 additions & 0 deletions django/contrib/admin/helpers.py
Expand Up @@ -197,6 +197,12 @@ def contents(self):
except (AttributeError, ValueError, ObjectDoesNotExist):
result_repr = self.empty_value_display
else:
if field in self.form.fields:
widget = self.form[field].field.widget
# This isn't elegant but suffices for contrib.auth's
# ReadOnlyPasswordHashWidget.
if getattr(widget, 'read_only', False):
return widget.render(field, value)
if f is None:
if getattr(attr, 'boolean', False):
result_repr = _boolean_icon(value)
Expand Down
1 change: 1 addition & 0 deletions django/contrib/auth/forms.py
Expand Up @@ -22,6 +22,7 @@

class ReadOnlyPasswordHashWidget(forms.Widget):
template_name = 'auth/widgets/read_only_password_hash.html'
read_only = True

def get_context(self, name, value, attrs):
context = super().get_context(name, value, attrs)
Expand Down
13 changes: 11 additions & 2 deletions docs/releases/2.1.2.txt
Expand Up @@ -4,8 +4,17 @@ Django 2.1.2 release notes

*Expected October 1, 2018*

Django 2.1.2 fixes several bugs in 2.1.1. Also, the latest string translations
from Transifex are incorporated.
Django 2.1.2 fixes a security issue and several bugs in 2.1.1. Also, the latest
string translations from Transifex are incorporated.

CVE-2018-16984: Password hash disclosure to "view only" admin users
===================================================================

If an admin user has the change permission to the user model, only part of the
password hash is displayed in the change form. Admin users with the view (but
not change) permission to the user model were displayed the entire hash. While
it's typically infeasible to reverse a strong password hash, if your site uses
weaker password hashing algorithms such as MD5 or SHA1, it could be a problem.

Bugfixes
========
Expand Down
27 changes: 26 additions & 1 deletion tests/auth_tests/test_views.py
Expand Up @@ -15,11 +15,12 @@
from django.contrib.auth.forms import (
AuthenticationForm, PasswordChangeForm, SetPasswordForm,
)
from django.contrib.auth.models import User
from django.contrib.auth.models import Permission, User
from django.contrib.auth.views import (
INTERNAL_RESET_SESSION_TOKEN, LoginView, logout_then_login,
redirect_to_login,
)
from django.contrib.contenttypes.models import ContentType
from django.contrib.sessions.middleware import SessionMiddleware
from django.contrib.sites.requests import RequestSite
from django.core import mail
Expand Down Expand Up @@ -1098,6 +1099,11 @@ def test_logout_redirect_url_named_setting(self):
self.assertRedirects(response, '/logout/', fetch_redirect_response=False)


def get_perm(Model, perm):
ct = ContentType.objects.get_for_model(Model)
return Permission.objects.get(content_type=ct, codename=perm)


# Redirect in test_user_change_password will fail if session auth hash
# isn't updated after password change (#21649)
@override_settings(ROOT_URLCONF='auth_tests.urls_admin')
Expand Down Expand Up @@ -1211,6 +1217,25 @@ def test_user_change_password_passes_user_to_has_change_permission(self, has_cha
(_request, user), _kwargs = has_change_permission.call_args
self.assertEqual(user.pk, self.admin.pk)

def test_view_user_password_is_readonly(self):
u = User.objects.get(username='testclient')
u.is_superuser = False
u.save()
u.user_permissions.add(get_perm(User, 'view_user'))
response = self.client.get(reverse('auth_test_admin:auth_user_change', args=(u.pk,)),)
algo, salt, hash_string = (u.password.split('$'))
self.assertContains(response, '<div class="readonly">testclient</div>')
# ReadOnlyPasswordHashWidget is used to render the field.
self.assertContains(
response,
'<strong>algorithm</strong>: %s\n\n'
'<strong>salt</strong>: %s**********\n\n'
'<strong>hash</strong>: %s**************************\n\n' % (
algo, salt[:2], hash_string[:6],
),
html=True,
)


@override_settings(
AUTH_USER_MODEL='auth_tests.UUIDUser',
Expand Down

0 comments on commit bf39978

Please sign in to comment.