Skip to content
Permalink
Browse files

[1.4.x] Prevented data leakage in contrib.admin via query string mani…

…pulation.

This is a security fix. Disclosure following shortly.
  • Loading branch information...
charettes authored and timgraham committed Aug 11, 2014
1 parent c9e3b99 commit 027bd348642007617518379f8b02546abacaa6e0
@@ -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
@@ -269,6 +269,24 @@ def lookup_allowed(self, lookup, value):
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.
@@ -10,6 +10,7 @@
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)
@@ -56,7 +57,10 @@ def __init__(self, request, model, list_display, list_display_links,
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]
@@ -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.
@@ -13,11 +13,12 @@
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
@@ -572,6 +573,19 @@ def test_disallowed_filtering(self):
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
@@ -2061,10 +2075,9 @@ def test_with_fk_to_field(self):
"""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')

0 comments on commit 027bd34

Please sign in to comment.
You can’t perform that action at this time.