diff --git a/django/contrib/admin/exceptions.py b/django/contrib/admin/exceptions.py
index 2e094c6da1..f619bc2252 100644
--- a/django/contrib/admin/exceptions.py
+++ b/django/contrib/admin/exceptions.py
@@ -4,3 +4,8 @@ from django.core.exceptions import SuspiciousOperation
class DisallowedModelAdminLookup(SuspiciousOperation):
"""Invalid filter was passed to admin view via URL querystring"""
pass
+
+
+class DisallowedModelAdminToField(SuspiciousOperation):
+ """Invalid to_field was passed to admin view via URL query string"""
+ pass
diff --git a/django/contrib/admin/options.py b/django/contrib/admin/options.py
index 3b429fbaed..ce89cf4dbd 100644
--- a/django/contrib/admin/options.py
+++ b/django/contrib/admin/options.py
@@ -11,6 +11,7 @@ from django.contrib.admin import widgets, helpers
from django.contrib.admin import validation
from django.contrib.admin.checks import (BaseModelAdminChecks, ModelAdminChecks,
InlineModelAdminChecks)
+from django.contrib.admin.exceptions import DisallowedModelAdminToField
from django.contrib.admin.utils import (quote, unquote, flatten_fieldsets,
get_deleted_objects, model_format_dict, NestedObjects,
lookup_needs_distinct)
@@ -434,6 +435,24 @@ class BaseModelAdmin(six.with_metaclass(forms.MediaDefiningClass)):
valid_lookups.append(filter_item)
return clean_lookup in valid_lookups
+ 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 in related_object.field.foreign_related_fields):
+ return True
+
+ return False
+
def has_add_permission(self, request):
"""
Returns True if the given request has permission to add an object.
@@ -1337,6 +1356,10 @@ class ModelAdmin(BaseModelAdmin):
@transaction.atomic
def changeform_view(self, request, object_id=None, form_url='', extra_context=None):
+ to_field = request.POST.get(TO_FIELD_VAR, request.GET.get(TO_FIELD_VAR))
+ if to_field and not self.to_field_allowed(request, to_field):
+ raise DisallowedModelAdminToField("The field %s cannot be referenced." % to_field)
+
model = self.model
opts = model._meta
add = object_id is None
@@ -1409,8 +1432,7 @@ class ModelAdmin(BaseModelAdmin):
original=obj,
is_popup=(IS_POPUP_VAR in request.POST or
IS_POPUP_VAR in request.GET),
- to_field=request.POST.get(TO_FIELD_VAR,
- request.GET.get(TO_FIELD_VAR)),
+ to_field=to_field,
media=media,
inline_admin_formsets=inline_formsets,
errors=helpers.AdminErrorList(form, formsets),
diff --git a/django/contrib/admin/views/main.py b/django/contrib/admin/views/main.py
index d1b6d82a3e..1f42310f8e 100644
--- a/django/contrib/admin/views/main.py
+++ b/django/contrib/admin/views/main.py
@@ -12,7 +12,9 @@ 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 DisallowedModelAdminLookup
+from django.contrib.admin.exceptions import (
+ DisallowedModelAdminLookup, DisallowedModelAdminToField,
+)
from django.contrib.admin.options import IncorrectLookupParameters, IS_POPUP_VAR, TO_FIELD_VAR
from django.contrib.admin.utils import (quote, get_fields_from_path,
lookup_needs_distinct, prepare_lookup_value)
@@ -58,7 +60,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]
diff --git a/docs/ref/exceptions.txt b/docs/ref/exceptions.txt
index 3b8b644c76..e74e0991e1 100644
--- a/docs/ref/exceptions.txt
+++ b/docs/ref/exceptions.txt
@@ -56,6 +56,7 @@ SuspiciousOperation
* DisallowedHost
* DisallowedModelAdminLookup
+ * DisallowedModelAdminToField
* DisallowedRedirect
* InvalidSessionKey
* SuspiciousFileOperation
diff --git a/docs/releases/1.4.14.txt b/docs/releases/1.4.14.txt
index 811c3f67ea..98de8b018e 100644
--- a/docs/releases/1.4.14.txt
+++ b/docs/releases/1.4.14.txt
@@ -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.
diff --git a/docs/releases/1.5.9.txt b/docs/releases/1.5.9.txt
index 5070f0eca1..5b91de0eb2 100644
--- a/docs/releases/1.5.9.txt
+++ b/docs/releases/1.5.9.txt
@@ -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.
diff --git a/docs/releases/1.6.6.txt b/docs/releases/1.6.6.txt
index e52ea6b23f..63a1537c97 100644
--- a/docs/releases/1.6.6.txt
+++ b/docs/releases/1.6.6.txt
@@ -48,6 +48,21 @@ 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/?_popup=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.
+
Bugfixes
========
diff --git a/tests/admin_views/tests.py b/tests/admin_views/tests.py
index adc49a5db0..60ad3d2b7b 100644
--- a/tests/admin_views/tests.py
+++ b/tests/admin_views/tests.py
@@ -18,6 +18,7 @@ from django.contrib.auth import get_permission_codename
from django.contrib.admin import ModelAdmin
from django.contrib.admin.helpers import ACTION_CHECKBOX_NAME
from django.contrib.admin.models import LogEntry, DELETION
+from django.contrib.admin.options import TO_FIELD_VAR
from django.contrib.admin.templatetags.admin_static import static
from django.contrib.admin.templatetags.admin_urls import add_preserved_filters
from django.contrib.admin.tests import AdminSeleniumWebDriverTestCase
@@ -598,6 +599,36 @@ class AdminViewBasicTest(AdminViewBasicTestCase):
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 patch_logger('django.security.DisallowedModelAdminToField', 'error') as calls:
+ response = self.client.get("/test_admin/admin/admin_views/section/", {TO_FIELD_VAR: 'missing_field'})
+ self.assertEqual(response.status_code, 400)
+ self.assertEqual(len(calls), 1)
+
+ # Specifying a field that is not refered by any other model registered
+ # to this admin site should raise an exception.
+ with patch_logger('django.security.DisallowedModelAdminToField', 'error') as calls:
+ response = self.client.get("/test_admin/admin/admin_views/section/", {TO_FIELD_VAR: 'name'})
+ self.assertEqual(response.status_code, 400)
+ self.assertEqual(len(calls), 1)
+
+ # 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)
+
+ # We also want to prevent the add and change view from leaking a
+ # disallowed field value.
+ with patch_logger('django.security.DisallowedModelAdminToField', 'error') as calls:
+ response = self.client.post("/test_admin/admin/admin_views/section/add/", {TO_FIELD_VAR: 'name'})
+ self.assertEqual(response.status_code, 400)
+ self.assertEqual(len(calls), 1)
+
+ section = Section.objects.create()
+ with patch_logger('django.security.DisallowedModelAdminToField', 'error') as calls:
+ response = self.client.post("/test_admin/admin/admin_views/section/%d/" % section.pk, {TO_FIELD_VAR: 'name'})
+ self.assertEqual(response.status_code, 400)
+ self.assertEqual(len(calls), 1)
+
def test_allowed_filtering_15103(self):
"""
Regressions test for ticket 15103 - filtering on fields defined in a
@@ -2378,10 +2409,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, '', html=True)
+ self.assertContains(response, '' % TO_FIELD_VAR, html=True)
def test_exact_matches(self):
response = self.client.get('/test_admin/admin/admin_views/recommendation/?q=bar')