[1.7.x] Fixed #24461 -- Fixed XSS issue in ModelAdmin.readonly_fields

django · Mar 9, 2015 · 2654e1b · 2654e1b
1 parent 5a3b531
commit 2654e1b
Show file tree

Hide file tree

Showing 5 changed files with 34 additions and 4 deletions.
diff --git a/django/contrib/admin/helpers.py b/django/contrib/admin/helpers.py
@@ -193,7 +193,7 @@ def contents(self):
                     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
@@ -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
@@ -860,7 +860,7 @@ def get_formsets_with_inlines(self, request, obj=None):
 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
@@ -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
@@ -3862,6 +3862,15 @@ def test_readonly_field_overrides(self):
         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):