Skip to content

Commit

Permalink
[3.0.x] Fixed CVE-2020-13596 -- Fixed potential XSS in admin ForeignK…
Browse files Browse the repository at this point in the history
…eyRawIdWidget.
  • Loading branch information
jdufresne authored and carltongibson committed Jun 3, 2020
1 parent 256d297 commit 1f2dd37
Show file tree
Hide file tree
Showing 5 changed files with 36 additions and 3 deletions.
6 changes: 3 additions & 3 deletions django/contrib/admin/widgets.py
Original file line number Diff line number Diff line change
Expand Up @@ -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 _

Expand Down Expand Up @@ -150,8 +150,8 @@ def get_context(self, name, value, attrs):

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')
Expand Down
7 changes: 7 additions & 0 deletions docs/releases/2.2.13.txt
Original file line number Diff line number Diff line change
Expand Up @@ -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
========

Expand Down
7 changes: 7 additions & 0 deletions docs/releases/3.0.7.txt
Original file line number Diff line number Diff line change
Expand Up @@ -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
========

Expand Down
8 changes: 8 additions & 0 deletions tests/admin_widgets/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -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')
Expand Down
11 changes: 11 additions & 0 deletions tests/admin_widgets/tests.py
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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):
Expand Down

0 comments on commit 1f2dd37

Please sign in to comment.