[2.0.x] Fixed CVE-2020-13596 -- Fixed potential XSS in admin ForeignKeyRawIdWidget.

jdufresne · carltongibson · commit 6d61860b2287 · 2020-06-03T09:33:38.000+02:00
diff --git a/django/contrib/admin/widgets.py b/django/contrib/admin/widgets.py
@@ -12,7 +12,7 @@
 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 _
 
@@ -150,8 +150,8 @@ def get_context(self, name, value, attrs):
 
             params = self.url_parameters()
             if params:
-                related_url += '?' + '&amp;'.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
@@ -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/tests/admin_widgets/models.py b/tests/admin_widgets/models.py
@@ -27,6 +27,14 @@ def __str__(self):
         return self.name
 
 
+class UnsafeLimitChoicesTo(models.Model):
+    band = models.ForeignKey(
+        Band,
+        models.CASCADE,
+        limit_choices_to={'name': '"&><escapeme'},
+    )
+
+
 class Album(models.Model):
     band = models.ForeignKey(Band, models.CASCADE)
     featuring = models.ManyToManyField(Band, related_name='featured')
diff --git a/tests/admin_widgets/tests.py b/tests/admin_widgets/tests.py
@@ -22,6 +22,7 @@
 from .models import (
     Advisor, Album, Band, Bee, Car, Company, Event, Honeycomb, Individual,
     Inventory, Member, MyFileField, Profile, School, Student,
+    UnsafeLimitChoicesTo,
 )
 from .widgetadmin import site as widget_admin_site
 
@@ -586,6 +587,16 @@ def test_proper_manager_for_label_lookup(self):
             'Hidden</a></strong>' % {'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),
+            '<input type="text" name="test" class="vForeignKeyRawIdAdminField">\n'
+            '<a href="/admin_widgets/band/?name=%22%26%3E%3Cescapeme&amp;_to_field=id" '
+            'class="related-lookup" id="lookup_id_test" title="Lookup"></a>'
+        )
+
 
 @override_settings(ROOT_URLCONF='admin_widgets.urls')
 class ManyToManyRawIdWidgetTest(TestCase):
