From 2654e1b93923bac55f12b4e66c5e39b16695ace5 Mon Sep 17 00:00:00 2001 From: Baptiste Mispelon Date: Sun, 8 Mar 2015 11:50:32 +0100 Subject: [PATCH] [1.7.x] Fixed #24461 -- Fixed XSS issue in ModelAdmin.readonly_fields --- django/contrib/admin/helpers.py | 2 +- docs/releases/1.7.6.txt | 18 ++++++++++++++++-- tests/admin_views/admin.py | 2 +- tests/admin_views/models.py | 7 +++++++ tests/admin_views/tests.py | 9 +++++++++ 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 +` 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, '', 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='evil') + response = self.client.get(reverse('admin:admin_views_section_change', args=(section.pk,))) + self.assertNotContains(response, "evil", status_code=200) + self.assertContains(response, "<a>evil</a>", status_code=200) + @override_settings(PASSWORD_HASHERS=('django.contrib.auth.hashers.SHA1PasswordHasher',)) class LimitChoicesToInAdminTest(TestCase):