summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorBaptiste Mispelon <bmispelon@gmail.com>2015-03-08 11:50:32 +0100
committerTim Graham <timograham@gmail.com>2015-03-09 10:17:54 -0400
commit2654e1b93923bac55f12b4e66c5e39b16695ace5 (patch)
tree339c5e65ac69a07dd58bee6e4831a23bc87e7500
parent5a3b53112193cc74d2043e09894dd29a763b1105 (diff)
[1.7.x] Fixed #24461 -- Fixed XSS issue in ModelAdmin.readonly_fields
-rw-r--r--django/contrib/admin/helpers.py2
-rw-r--r--docs/releases/1.7.6.txt18
-rw-r--r--tests/admin_views/admin.py2
-rw-r--r--tests/admin_views/models.py7
-rw-r--r--tests/admin_views/tests.py9
5 files changed, 34 insertions, 4 deletions
diff --git a/django/contrib/admin/helpers.py b/django/contrib/admin/helpers.py
index 4f6bbe51e4..6ea34e242a 100644
--- a/django/contrib/admin/helpers.py
+++ b/django/contrib/admin/helpers.py
@@ -193,7 +193,7 @@ class AdminReadonlyField(object):
if getattr(attr, "allow_tags", False):
result_repr = mark_safe(result_repr)
else:
- result_repr = linebreaksbr(result_repr)
+ result_repr = linebreaksbr(result_repr, autoescape=True)
else:
if isinstance(f.rel, ManyToManyRel) and value is not None:
result_repr = ", ".join(map(six.text_type, value.all()))
diff --git a/docs/releases/1.7.6.txt b/docs/releases/1.7.6.txt
index af0907816d..7f54da1474 100644
--- a/docs/releases/1.7.6.txt
+++ b/docs/releases/1.7.6.txt
@@ -2,9 +2,23 @@
Django 1.7.6 release notes
==========================
-*Under Development*
+*March 9, 2015*
-Django 1.7.6 fixes several bugs in 1.7.5.
+Django 1.7.6 fixes a security issue and several bugs in 1.7.5.
+
+Mitigated an XSS attack via properties in ``ModelAdmin.readonly_fields``
+========================================================================
+
+The :attr:`ModelAdmin.readonly_fields
+<django.contrib.admin.ModelAdmin.readonly_fields>` attribute in the Django
+admin allows displaying model fields and model attributes. While the former
+were correctly escaped, the latter were not. Thus untrusted content could be
+injected into the admin, presenting an exploitation vector for XSS attacks.
+
+In this vulnerability, every model attribute used in ``readonly_fields`` that
+is not an actual model field (e.g. a :class:`property`) will **fail to be
+escaped** even if that attribute is not marked as safe. In this release,
+autoescaping is now correctly applied.
Bugfixes
========
diff --git a/tests/admin_views/admin.py b/tests/admin_views/admin.py
index 9a05df614a..15c1384635 100644
--- a/tests/admin_views/admin.py
+++ b/tests/admin_views/admin.py
@@ -860,7 +860,7 @@ class GetFormsetsArgumentCheckingAdmin(admin.ModelAdmin):
site = admin.AdminSite(name="admin")
site.register(Article, ArticleAdmin)
site.register(CustomArticle, CustomArticleAdmin)
-site.register(Section, save_as=True, inlines=[ArticleInline])
+site.register(Section, save_as=True, inlines=[ArticleInline], readonly_fields=['name_property'])
site.register(ModelWithStringPrimaryKey)
site.register(Color)
site.register(Thing, ThingAdmin)
diff --git a/tests/admin_views/models.py b/tests/admin_views/models.py
index 341b7aec65..f41b4de088 100644
--- a/tests/admin_views/models.py
+++ b/tests/admin_views/models.py
@@ -22,6 +22,13 @@ class Section(models.Model):
"""
name = models.CharField(max_length=100)
+ @property
+ def name_property(self):
+ """
+ A property that simply returns the name. Used to test #24461
+ """
+ return self.name
+
@python_2_unicode_compatible
class Article(models.Model):
diff --git a/tests/admin_views/tests.py b/tests/admin_views/tests.py
index 96da7c438e..e61064211c 100644
--- a/tests/admin_views/tests.py
+++ b/tests/admin_views/tests.py
@@ -3862,6 +3862,15 @@ class ReadonlyTest(TestCase):
self.assertContains(response, '<label for="id_public">Overridden public label:</label>', html=True)
self.assertNotContains(response, "Some help text for the date (with unicode ŠĐĆŽćžšđ)")
+ def test_correct_autoescaping(self):
+ """
+ Make sure that non-field readonly elements are properly autoescaped (#24461)
+ """
+ section = Section.objects.create(name='<a>evil</a>')
+ response = self.client.get(reverse('admin:admin_views_section_change', args=(section.pk,)))
+ self.assertNotContains(response, "<a>evil</a>", status_code=200)
+ self.assertContains(response, "&lt;a&gt;evil&lt;/a&gt;", status_code=200)
+
@override_settings(PASSWORD_HASHERS=('django.contrib.auth.hashers.SHA1PasswordHasher',))
class LimitChoicesToInAdminTest(TestCase):