Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP
Browse files

Fix a security issue in the admin. Disclosure and new release forthco…

…ming.

git-svn-id: http://code.djangoproject.com/svn/django/trunk@15031 bcc190cf-cafb-0310-a4f2-bffc1f526a37
  • Loading branch information...
commit 732198ed5c2a127249970e0cd4218d093a38e9a2 1 parent 5ed6e7a
Alex Gaynor alex authored
41 django/contrib/admin/options.py
View
@@ -11,7 +11,9 @@
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 @@ def queryset(self, request):
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."
8 django/contrib/admin/views/main.py
View
@@ -1,6 +1,7 @@
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,13 +189,18 @@ def get_query_set(self):
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)
# Naked except! Because we don't have any other way of validating "params".
# They might be invalid if the keyword arguments are incorrect, or if the
# values are not in the correct type, so we might get FieldError, ValueError,
- # ValicationError, or ? from a custom field that raises yet something else
+ # ValicationError, or ? from a custom field that raises yet something else
# when handed impossible data.
except:
raise IncorrectLookupParameters
7 tests/regressiontests/admin_views/models.py
View
@@ -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 @@ class Album(models.Model):
admin.site.register(ChapterXtra1, ChapterXtra1Admin)
admin.site.register(Pizza, PizzaAdmin)
admin.site.register(Topping)
-admin.site.register(Album)
+admin.site.register(Album, AlbumAdmin)
10 tests/regressiontests/admin_views/tests.py
View
@@ -5,6 +5,7 @@
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 @@ def testI18NLanguageNonEnglishFallback(self):
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']
Please sign in to comment.
Something went wrong with that request. Please try again.