Fix a security issue in the admin. Disclosure and new release forthcoming. (732198ed) · Commits · Dom Sekotill / django

django/contrib/admin/options.py

+40 −1

Original line number	Diff line number	Diff line
		@@ -11,7 +11,9 @@ from django.views.decorators.csrf import csrf_protect
		from django.core.exceptions import PermissionDenied, ValidationError
		from django.core.paginator import Paginator
		from django.db import models, transaction, router
		from django.db.models.fields import BLANK_CHOICE_DASH
		from django.db.models.related import RelatedObject
		from django.db.models.fields import BLANK_CHOICE_DASH, FieldDoesNotExist
		from django.db.models.sql.constants import LOOKUP_SEP, QUERY_TERMS
		from django.http import Http404, HttpResponse, HttpResponseRedirect
		from django.shortcuts import get_object_or_404, render_to_response
		from django.utils.decorators import method_decorator
		@@ -203,6 +205,43 @@ class BaseModelAdmin(object):
		qs = qs.order_by(*ordering)
		return qs

		def lookup_allowed(self, lookup):
		parts = lookup.split(LOOKUP_SEP)

		# Last term in lookup is a query term (__exact, __startswith etc)
		# This term can be ignored.
		if len(parts) > 1 and parts[-1] in QUERY_TERMS:
		parts.pop()

		# Special case -- foo__id__exact and foo__id queries are implied
		# if foo has been specificially included in the lookup list; so
		# drop __id if it is the last part. However, first we need to find
		# the pk attribute name.
		model = self.model
		pk_attr_name = None
		for part in parts[:-1]:
		field, _, _, _ = model._meta.get_field_by_name(part)
		if hasattr(field, 'rel'):
		model = field.rel.to
		pk_attr_name = model._meta.pk.name
		elif isinstance(field, RelatedObject):
		model = field.model
		pk_attr_name = model._meta.pk.name
		else:
		pk_attr_name = None
		if pk_attr_name and len(parts) > 1 and parts[-1] == pk_attr_name:
		parts.pop()

		try:
		self.model._meta.get_field_by_name(parts[0])
		except FieldDoesNotExist:
		# Lookups on non-existants fields are ok, since they're ignored
		# later.
		return True
		else:
		clean_lookup = LOOKUP_SEP.join(parts)
		return clean_lookup in self.list_filter or clean_lookup == self.date_hierarchy

		class ModelAdmin(BaseModelAdmin):
		"Encapsulates all admin options and functionality for a given model."

django/contrib/admin/views/main.py

+7 −1

Original line number	Diff line number	Diff line
		from django.contrib.admin.filterspecs import FilterSpec
		from django.contrib.admin.options import IncorrectLookupParameters
		from django.contrib.admin.util import quote, get_fields_from_path
		from django.core.exceptions import SuspiciousOperation
		from django.core.paginator import Paginator, InvalidPage
		from django.db import models
		from django.utils.encoding import force_unicode, smart_str
		@@ -188,6 +189,11 @@ class ChangeList(object):
		else:
		lookup_params[key] = True

		if not self.model_admin.lookup_allowed(key):
		raise SuspiciousOperation(
		"Filtering by %s not allowed" % key
		)

		# Apply lookup parameters from the query string.
		try:
		qs = qs.filter(**lookup_params)

tests/regressiontests/admin_views/models.py

+5 −2

Original line number	Diff line number	Diff line
		@@ -100,7 +100,7 @@ class ChapterXtra1Admin(admin.ModelAdmin):

		class ArticleAdmin(admin.ModelAdmin):
		list_display = ('content', 'date', callable_year, 'model_year', 'modeladmin_year')
		list_filter = ('date',)
		list_filter = ('date', 'section')

		def changelist_view(self, request):
		"Test that extra_context works"
		@@ -611,6 +611,9 @@ class Album(models.Model):
		owner = models.ForeignKey(User)
		title = models.CharField(max_length=30)

		class AlbumAdmin(admin.ModelAdmin):
		list_filter = ['title']

		admin.site.register(Article, ArticleAdmin)
		admin.site.register(CustomArticle, CustomArticleAdmin)
		admin.site.register(Section, save_as=True, inlines=[ArticleInline])
		@@ -657,4 +660,4 @@ admin.site.register(Promo)
		admin.site.register(ChapterXtra1, ChapterXtra1Admin)
		admin.site.register(Pizza, PizzaAdmin)
		admin.site.register(Topping)
		admin.site.register(Album)
		admin.site.register(Album, AlbumAdmin)

tests/regressiontests/admin_views/tests.py

+10 −0

Original line number	Diff line number	Diff line
		@@ -5,6 +5,7 @@ import datetime

		from django.conf import settings
		from django.core import mail
		from django.core.exceptions import SuspiciousOperation
		from django.core.files import temp as tempfile
		from django.core.urlresolvers import reverse
		# Register auth models with the admin.
		@@ -348,6 +349,15 @@ class AdminViewBasicTest(TestCase):
		self.assertContains(response, 'Choisir une heure')
		deactivate()

		def test_disallowed_filtering(self):
		self.assertRaises(SuspiciousOperation,
		self.client.get, "/test_admin/admin/admin_views/album/?owner__email__startswith=fuzzy"
		)

		try:
		self.client.get("/test_admin/admin/admin_views/stuff/?color__value__startswith=red")
		except SuspiciousOperation:
		self.fail("Filters are allowed if explicitly included in list_filter")

		class SaveAsTests(TestCase):
		fixtures = ['admin-views-users.xml','admin-views-person.xml']