diff --git a/django/contrib/admin/exceptions.py b/django/contrib/admin/exceptions.py
new file mode 100644
index 0000000000..d9de47eefd
--- /dev/null
+++ b/django/contrib/admin/exceptions.py
@@ -0,0 +1,6 @@
+from django.core.exceptions import SuspiciousOperation
+
+
+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 6204e0b2c2..e7cb85f777 100644
--- a/django/contrib/admin/options.py
+++ b/django/contrib/admin/options.py
@@ -275,6 +275,24 @@ class BaseModelAdmin(six.with_metaclass(forms.MediaDefiningClass)):
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.
diff --git a/django/contrib/admin/views/main.py b/django/contrib/admin/views/main.py
index be7067ff61..cf53d6a42d 100644
--- a/django/contrib/admin/views/main.py
+++ b/django/contrib/admin/views/main.py
@@ -12,6 +12,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)
@@ -58,7 +59,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/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/tests/regressiontests/admin_views/tests.py b/tests/regressiontests/admin_views/tests.py
index 72c182908e..e7efca21d8 100644
--- a/tests/regressiontests/admin_views/tests.py
+++ b/tests/regressiontests/admin_views/tests.py
@@ -16,11 +16,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
@@ -557,6 +558,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
@@ -2138,10 +2152,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')