Fixed CVE-2018-16984 -- Fixed password hash disclosure to admin "view only" users.

carltongibson · carltongibson · commit bf39978a53f1 · 2018-10-01T10:05:01.000+02:00
Thanks Claude Paroz &amp; Tim Graham for collaborating on the patch.
diff --git a/django/contrib/admin/helpers.py b/django/contrib/admin/helpers.py
@@ -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)
diff --git a/django/contrib/auth/forms.py b/django/contrib/auth/forms.py
@@ -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)
diff --git a/docs/releases/2.1.2.txt b/docs/releases/2.1.2.txt
@@ -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
 ========
diff --git a/tests/auth_tests/test_views.py b/tests/auth_tests/test_views.py
@@ -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
@@ -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')
@@ -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',
