Information disclosure / Authorization bypass (UI-level)

PoC (pre-fix): Prerequisites: - A Django project with admin UI enabled and a User model populated with a known password. - Admin user with insufficient permissions to change users, or a non-admin staff user who can view users in the admin UI. Steps: 1) Log in to the Django admin as a staff user who does not have auth.change_user permission. 2) Open the admin page for a user, e.g. /admin/auth/user/2/change/ 3) Observe the password field: prior to this fix, the actual plaintext password (e.g., testclient) could be displayed in a div with class="readonly". With the fix, the password is displayed as a hash, or the field value is not shown in plaintext. 4) Look for the Reset password button/prompt on that page. In older versions, the anchor with text "Reset password" (e.g. <a role=\"button\" class=\"button\" href=\"../password/\">Reset password</a>) could be visible for users without change_user permission. The fix hides this button unless the user has permission. Note: The exact HTML may vary by Django version, but the presence of a plaintext password and an unauthorized Reset password control are the core indicators. A practical PoC would automate a login and fetch of /admin/auth/user/ID/change/ and assert absence/presence of the plaintext password or reset button.

diff --git a/django/contrib/admin/helpers.py b/django/contrib/admin/helpers.py index 51450d1d9e09..969167f0e272 100644 --- a/django/contrib/admin/helpers.py +++ b/django/contrib/admin/helpers.py @@ -276,12 +276,6 @@ 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/admin/utils.py b/django/contrib/admin/utils.py index cc8ca5604dc8..eec93fa4be25 100644 --- a/django/contrib/admin/utils.py +++ b/django/contrib/admin/utils.py @@ -5,6 +5,8 @@ from functools import reduce from operator import or_ +from django.contrib.auth import get_user_model +from django.contrib.auth.templatetags.auth import render_password_as_hash from django.core.exceptions import FieldDoesNotExist from django.core.validators import EMPTY_VALUES from django.db import models, router @@ -429,7 +431,9 @@ def help_text_for_field(name, model): def display_for_field(value, field, empty_value_display, avoid_link=False): from django.contrib.admin.templatetags.admin_list import _boolean_icon - if getattr(field, "flatchoices", None): + if field.name == "password" and field.model == get_user_model(): + return render_password_as_hash(value) + elif getattr(field, "flatchoices", None): try: return dict(field.flatchoices).get(value, empty_value_display) except TypeError: diff --git a/django/contrib/auth/forms.py b/django/contrib/auth/forms.py index 7b0b3833b8e1..2214e134d092 100644 --- a/django/contrib/auth/forms.py +++ b/django/contrib/auth/forms.py @@ -34,7 +34,6 @@ def _unicode_ci_compare(s1, s2): 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/tests/admin_utils/tests.py b/tests/admin_utils/tests.py index 77d6655290c3..6d165637e782 100644 --- a/tests/admin_utils/tests.py +++ b/tests/admin_utils/tests.py @@ -17,9 +17,12 @@ lookup_field, quote, ) +from django.contrib.auth.models import User +from django.contrib.auth.templatetags.auth import render_password_as_hash from django.core.validators import EMPTY_VALUES from django.db import DEFAULT_DB_ALIAS, models from django.test import SimpleTestCase, TestCase, override_settings +from django.test.utils import isolate_apps from django.utils.formats import localize from django.utils.safestring import mark_safe @@ -238,6 +241,28 @@ def test_number_formats_with_thousand_separator_display_for_field(self): ) self.assertEqual(display_value, "12,345") + @isolate_apps("admin_utils") + def test_display_for_field_password_name_not_user_model(self): + class PasswordModel(models.Model): + password = models.CharField(max_length=200) + + password_field = PasswordModel._meta.get_field("password") + display_value = display_for_field("test", password_field, self.empty_value) + self.assertEqual(display_value, "test") + + def test_password_display_for_field_user_model(self): + password_field = User._meta.get_field("password") + for password in [ + "invalid", + "md5$zjIiKM8EiyfXEGiexlQRw4$a59a82cf344546e7bc09cb5f2246370a", + "!b7pk7RNudAXGTNLK6fW5YnBCLVE6UUmeoJJYQHaO", + ]: + with self.subTest(password=password): + display_value = display_for_field( + password, password_field, self.empty_value + ) + self.assertEqual(display_value, render_password_as_hash(password)) + def test_list_display_for_value(self): display_value = display_for_value([1, 2, 3], self.empty_value) self.assertEqual(display_value, "1, 2, 3") diff --git a/tests/auth_tests/test_views.py b/tests/auth_tests/test_views.py index 156520ebf77a..c8f0be1be74c 100644 --- a/tests/auth_tests/test_views.py +++ b/tests/auth_tests/test_views.py @@ -1703,7 +1703,7 @@ def test_view_user_password_is_readonly(self): ) algo, salt, hash_string = u.password.split("$") self.assertContains(response, '<div class="readonly">testclient</div>') - # ReadOnlyPasswordHashWidget is used to render the field. + # The password value is hashed. self.assertContains( response, "<strong>algorithm</strong>: <bdi>%s</bdi>\n\n" @@ -1716,6 +1716,10 @@ def test_view_user_password_is_readonly(self): ), html=True, ) + self.assertNotContains( + response, + '<a role="button" class="button" href="../password/">Reset password</a>', + ) # Value in POST data is ignored. data = self.get_user_data(u) data["password"] = "shouldnotchange"