Loading django/contrib/admin/exceptions.py 0 → 100644 +6 −0 Original line number Diff line number Diff line from django.core.exceptions import SuspiciousOperation class DisallowedModelAdminToField(SuspiciousOperation): """Invalid to_field was passed to admin view via URL query string""" pass django/contrib/admin/options.py +18 −0 Original line number Diff line number Diff line Loading @@ -269,6 +269,24 @@ class BaseModelAdmin(object): clean_lookup = LOOKUP_SEP.join(parts) return clean_lookup in self.list_filter or clean_lookup == self.date_hierarchy def to_field_allowed(self, request, to_field): opts = self.model._meta try: field = opts.get_field(to_field) except FieldDoesNotExist: return False # Make sure at least one of the models registered for this site # references this field. registered_models = self.admin_site._registry for related_object in opts.get_all_related_objects(): if (related_object.model in registered_models and field == related_object.field.rel.get_related_field()): return True return False def has_add_permission(self, request): """ Returns True if the given request has permission to add an object. Loading django/contrib/admin/views/main.py +5 −1 Original line number Diff line number Diff line Loading @@ -10,6 +10,7 @@ from django.utils.translation import ugettext, ugettext_lazy from django.utils.http import urlencode from django.contrib.admin import FieldListFilter from django.contrib.admin.exceptions import DisallowedModelAdminToField from django.contrib.admin.options import IncorrectLookupParameters from django.contrib.admin.util import (quote, get_fields_from_path, lookup_needs_distinct, prepare_lookup_value) Loading Loading @@ -56,7 +57,10 @@ class ChangeList(object): self.page_num = 0 self.show_all = ALL_VAR in request.GET self.is_popup = IS_POPUP_VAR in request.GET self.to_field = request.GET.get(TO_FIELD_VAR) to_field = request.GET.get(TO_FIELD_VAR) if to_field and not model_admin.to_field_allowed(request, to_field): raise DisallowedModelAdminToField("The field %s cannot be referenced." % to_field) self.to_field = to_field self.params = dict(request.GET.items()) if PAGE_VAR in self.params: del self.params[PAGE_VAR] Loading docs/releases/1.4.14.txt +15 −0 Original line number Diff line number Diff line Loading @@ -47,3 +47,18 @@ and the ``RemoteUserBackend``, a change to the ``REMOTE_USER`` header between requests without an intervening logout could result in the prior user's session being co-opted by the subsequent user. The middleware now logs the user out on a failed login attempt. Data leakage via query string manipulation in ``contrib.admin`` =============================================================== In older versions of Django it was possible to reveal any field's data by modifying the "popup" and "to_field" parameters of the query string on an admin change form page. For example, requesting a URL like ``/admin/auth/user/?pop=1&t=password`` and viewing the page's HTML allowed viewing the password hash of each user. While the admin requires users to have permissions to view the change form pages in the first place, this could leak data if you rely on users having access to view only certain fields on a model. To address the issue, an exception will now be raised if a ``to_field`` value that isn't a related field to a model that has been registered with the admin is specified. tests/regressiontests/admin_views/tests.py +17 −4 Original line number Diff line number Diff line Loading @@ -13,11 +13,12 @@ from django.core.files import temp as tempfile from django.core.urlresolvers import reverse # Register auth models with the admin. from django.contrib import admin from django.contrib.admin.exceptions import DisallowedModelAdminToField from django.contrib.admin.helpers import ACTION_CHECKBOX_NAME from django.contrib.admin.models import LogEntry, DELETION from django.contrib.admin.sites import LOGIN_FORM_KEY from django.contrib.admin.util import quote from django.contrib.admin.views.main import IS_POPUP_VAR from django.contrib.admin.views.main import IS_POPUP_VAR, TO_FIELD_VAR from django.contrib.admin.tests import AdminSeleniumWebDriverTestCase from django.contrib.auth import REDIRECT_FIELD_NAME from django.contrib.auth.models import Group, User, Permission, UNUSABLE_PASSWORD Loading Loading @@ -572,6 +573,19 @@ class AdminViewBasicTest(TestCase): response = self.client.get("/test_admin/admin/admin_views/workhour/?employee__person_ptr__exact=%d" % e1.pk) self.assertEqual(response.status_code, 200) def test_disallowed_to_field(self): with self.assertRaises(DisallowedModelAdminToField): response = self.client.get("/test_admin/admin/admin_views/section/", {TO_FIELD_VAR: 'missing_field'}) # Specifying a field that is not refered by any other model registered # to this admin site should raise an exception. with self.assertRaises(DisallowedModelAdminToField): response = self.client.get("/test_admin/admin/admin_views/section/", {TO_FIELD_VAR: 'name'}) # Specifying a field referenced by another model should be allowed. response = self.client.get("/test_admin/admin/admin_views/section/", {TO_FIELD_VAR: 'id'}) self.assertEqual(response.status_code, 200) def test_allowed_filtering_15103(self): """ Regressions test for ticket 15103 - filtering on fields defined in a Loading Loading @@ -2061,10 +2075,9 @@ class AdminSearchTest(TestCase): """Ensure that the to_field GET parameter is preserved when a search is performed. Refs #10918. """ from django.contrib.admin.views.main import TO_FIELD_VAR response = self.client.get('/test_admin/admin/auth/user/?q=joe&%s=username' % TO_FIELD_VAR) response = self.client.get('/test_admin/admin/auth/user/?q=joe&%s=id' % TO_FIELD_VAR) self.assertContains(response, "\n1 user\n") self.assertContains(response, '<input type="hidden" name="t" value="username"/>', html=True) self.assertContains(response, '<input type="hidden" name="%s" value="id"/>' % TO_FIELD_VAR, html=True) def test_exact_matches(self): response = self.client.get('/test_admin/admin/admin_views/recommendation/?q=bar') Loading Loading
django/contrib/admin/exceptions.py 0 → 100644 +6 −0 Original line number Diff line number Diff line from django.core.exceptions import SuspiciousOperation class DisallowedModelAdminToField(SuspiciousOperation): """Invalid to_field was passed to admin view via URL query string""" pass
django/contrib/admin/options.py +18 −0 Original line number Diff line number Diff line Loading @@ -269,6 +269,24 @@ class BaseModelAdmin(object): clean_lookup = LOOKUP_SEP.join(parts) return clean_lookup in self.list_filter or clean_lookup == self.date_hierarchy def to_field_allowed(self, request, to_field): opts = self.model._meta try: field = opts.get_field(to_field) except FieldDoesNotExist: return False # Make sure at least one of the models registered for this site # references this field. registered_models = self.admin_site._registry for related_object in opts.get_all_related_objects(): if (related_object.model in registered_models and field == related_object.field.rel.get_related_field()): return True return False def has_add_permission(self, request): """ Returns True if the given request has permission to add an object. Loading
django/contrib/admin/views/main.py +5 −1 Original line number Diff line number Diff line Loading @@ -10,6 +10,7 @@ from django.utils.translation import ugettext, ugettext_lazy from django.utils.http import urlencode from django.contrib.admin import FieldListFilter from django.contrib.admin.exceptions import DisallowedModelAdminToField from django.contrib.admin.options import IncorrectLookupParameters from django.contrib.admin.util import (quote, get_fields_from_path, lookup_needs_distinct, prepare_lookup_value) Loading Loading @@ -56,7 +57,10 @@ class ChangeList(object): self.page_num = 0 self.show_all = ALL_VAR in request.GET self.is_popup = IS_POPUP_VAR in request.GET self.to_field = request.GET.get(TO_FIELD_VAR) to_field = request.GET.get(TO_FIELD_VAR) if to_field and not model_admin.to_field_allowed(request, to_field): raise DisallowedModelAdminToField("The field %s cannot be referenced." % to_field) self.to_field = to_field self.params = dict(request.GET.items()) if PAGE_VAR in self.params: del self.params[PAGE_VAR] Loading
docs/releases/1.4.14.txt +15 −0 Original line number Diff line number Diff line Loading @@ -47,3 +47,18 @@ and the ``RemoteUserBackend``, a change to the ``REMOTE_USER`` header between requests without an intervening logout could result in the prior user's session being co-opted by the subsequent user. The middleware now logs the user out on a failed login attempt. Data leakage via query string manipulation in ``contrib.admin`` =============================================================== In older versions of Django it was possible to reveal any field's data by modifying the "popup" and "to_field" parameters of the query string on an admin change form page. For example, requesting a URL like ``/admin/auth/user/?pop=1&t=password`` and viewing the page's HTML allowed viewing the password hash of each user. While the admin requires users to have permissions to view the change form pages in the first place, this could leak data if you rely on users having access to view only certain fields on a model. To address the issue, an exception will now be raised if a ``to_field`` value that isn't a related field to a model that has been registered with the admin is specified.
tests/regressiontests/admin_views/tests.py +17 −4 Original line number Diff line number Diff line Loading @@ -13,11 +13,12 @@ from django.core.files import temp as tempfile from django.core.urlresolvers import reverse # Register auth models with the admin. from django.contrib import admin from django.contrib.admin.exceptions import DisallowedModelAdminToField from django.contrib.admin.helpers import ACTION_CHECKBOX_NAME from django.contrib.admin.models import LogEntry, DELETION from django.contrib.admin.sites import LOGIN_FORM_KEY from django.contrib.admin.util import quote from django.contrib.admin.views.main import IS_POPUP_VAR from django.contrib.admin.views.main import IS_POPUP_VAR, TO_FIELD_VAR from django.contrib.admin.tests import AdminSeleniumWebDriverTestCase from django.contrib.auth import REDIRECT_FIELD_NAME from django.contrib.auth.models import Group, User, Permission, UNUSABLE_PASSWORD Loading Loading @@ -572,6 +573,19 @@ class AdminViewBasicTest(TestCase): response = self.client.get("/test_admin/admin/admin_views/workhour/?employee__person_ptr__exact=%d" % e1.pk) self.assertEqual(response.status_code, 200) def test_disallowed_to_field(self): with self.assertRaises(DisallowedModelAdminToField): response = self.client.get("/test_admin/admin/admin_views/section/", {TO_FIELD_VAR: 'missing_field'}) # Specifying a field that is not refered by any other model registered # to this admin site should raise an exception. with self.assertRaises(DisallowedModelAdminToField): response = self.client.get("/test_admin/admin/admin_views/section/", {TO_FIELD_VAR: 'name'}) # Specifying a field referenced by another model should be allowed. response = self.client.get("/test_admin/admin/admin_views/section/", {TO_FIELD_VAR: 'id'}) self.assertEqual(response.status_code, 200) def test_allowed_filtering_15103(self): """ Regressions test for ticket 15103 - filtering on fields defined in a Loading Loading @@ -2061,10 +2075,9 @@ class AdminSearchTest(TestCase): """Ensure that the to_field GET parameter is preserved when a search is performed. Refs #10918. """ from django.contrib.admin.views.main import TO_FIELD_VAR response = self.client.get('/test_admin/admin/auth/user/?q=joe&%s=username' % TO_FIELD_VAR) response = self.client.get('/test_admin/admin/auth/user/?q=joe&%s=id' % TO_FIELD_VAR) self.assertContains(response, "\n1 user\n") self.assertContains(response, '<input type="hidden" name="t" value="username"/>', html=True) self.assertContains(response, '<input type="hidden" name="%s" value="id"/>' % TO_FIELD_VAR, html=True) def test_exact_matches(self): response = self.client.get('/test_admin/admin/admin_views/recommendation/?q=bar') Loading