From 2dd4d110c159d0c81dff42eaead2c378a0998735 Mon Sep 17 00:00:00 2001 From: Jon Dufresne Date: Tue, 26 May 2020 09:51:02 +0200 Subject: [PATCH] Fixed CVE-2020-13596 -- Fixed potential XSS in admin ForeignKeyRawIdWidget. --- django/contrib/admin/widgets.py | 6 +++--- docs/releases/2.2.13.txt | 7 +++++++ docs/releases/3.0.7.txt | 7 +++++++ tests/admin_widgets/models.py | 8 ++++++++ tests/admin_widgets/tests.py | 11 +++++++++++ 5 files changed, 36 insertions(+), 3 deletions(-) diff --git a/django/contrib/admin/widgets.py b/django/contrib/admin/widgets.py index 1ec7c70abd..59d1004d2d 100644 --- a/django/contrib/admin/widgets.py +++ b/django/contrib/admin/widgets.py @@ -12,7 +12,7 @@ from django.db.models import CASCADE from django.urls import reverse from django.urls.exceptions import NoReverseMatch from django.utils.html import smart_urlquote -from django.utils.safestring import mark_safe +from django.utils.http import urlencode from django.utils.text import Truncator from django.utils.translation import get_language, gettext as _ @@ -145,8 +145,8 @@ class ForeignKeyRawIdWidget(forms.TextInput): params = self.url_parameters() if params: - related_url += '?' + '&'.join('%s=%s' % (k, v) for k, v in params.items()) - context['related_url'] = mark_safe(related_url) + related_url += '?' + urlencode(params) + context['related_url'] = related_url context['link_title'] = _('Lookup') # The JavaScript code looks for this class. context['widget']['attrs'].setdefault('class', 'vForeignKeyRawIdAdminField') diff --git a/docs/releases/2.2.13.txt b/docs/releases/2.2.13.txt index 2e149a1f18..ee381fdcce 100644 --- a/docs/releases/2.2.13.txt +++ b/docs/releases/2.2.13.txt @@ -6,6 +6,13 @@ Django 2.2.13 release notes Django 2.2.13 fixes two security issues and a regression in 2.2.12. +CVE-2020-13596: Possible XSS via admin ``ForeignKeyRawIdWidget`` +================================================================ + +Query parameters for the admin ``ForeignKeyRawIdWidget`` were not properly URL +encoded, posing an XSS attack vector. ``ForeignKeyRawIdWidget`` now +ensures query parameters are correctly URL encoded. + Bugfixes ======== diff --git a/docs/releases/3.0.7.txt b/docs/releases/3.0.7.txt index f1775b3471..51ac0d7edd 100644 --- a/docs/releases/3.0.7.txt +++ b/docs/releases/3.0.7.txt @@ -6,6 +6,13 @@ Django 3.0.7 release notes Django 3.0.7 fixes two security issues and several bugs in 3.0.6. +CVE-2020-13596: Possible XSS via admin ``ForeignKeyRawIdWidget`` +================================================================ + +Query parameters for the admin ``ForeignKeyRawIdWidget`` were not properly URL +encoded, posing an XSS attack vector. ``ForeignKeyRawIdWidget`` now +ensures query parameters are correctly URL encoded. + Bugfixes ======== diff --git a/tests/admin_widgets/models.py b/tests/admin_widgets/models.py index b5025fdfd7..88bf2b8fca 100644 --- a/tests/admin_widgets/models.py +++ b/tests/admin_widgets/models.py @@ -27,6 +27,14 @@ class Band(models.Model): return self.name +class UnsafeLimitChoicesTo(models.Model): + band = models.ForeignKey( + Band, + models.CASCADE, + limit_choices_to={'name': '"&>' % {'pk': hidden.pk} ) + def test_render_unsafe_limit_choices_to(self): + rel = UnsafeLimitChoicesTo._meta.get_field('band').remote_field + w = widgets.ForeignKeyRawIdWidget(rel, widget_admin_site) + self.assertHTMLEqual( + w.render('test', None), + '\n' + '' + ) + @override_settings(ROOT_URLCONF='admin_widgets.urls') class ManyToManyRawIdWidgetTest(TestCase):