diff --git a/django/contrib/admin/templates/admin/widgets/url.html b/django/contrib/admin/templates/admin/widgets/url.html index 554a9343fe..629a740664 100644 --- a/django/contrib/admin/templates/admin/widgets/url.html +++ b/django/contrib/admin/templates/admin/widgets/url.html @@ -1 +1 @@ -{% if widget.value %}

{{ current_label }} {{ widget.value }}
{{ change_label }} {% endif %}{% include "django/forms/widgets/input.html" %}{% if widget.value %}

{% endif %} +{% if url_valid %}

{{ current_label }} {{ widget.value }}
{{ change_label }} {% endif %}{% include "django/forms/widgets/input.html" %}{% if url_valid %}

{% endif %} diff --git a/django/contrib/admin/widgets.py b/django/contrib/admin/widgets.py index 62da6265b6..209e0289e3 100644 --- a/django/contrib/admin/widgets.py +++ b/django/contrib/admin/widgets.py @@ -7,6 +7,7 @@ import copy from django import forms from django.core.exceptions import ValidationError +from django.core.validators import URLValidator from django.db.models.deletion import CASCADE from django.urls import reverse from django.urls.exceptions import NoReverseMatch @@ -339,17 +340,24 @@ class AdminEmailInputWidget(forms.EmailInput): class AdminURLFieldWidget(forms.URLInput): template_name = 'admin/widgets/url.html' - def __init__(self, attrs=None): + def __init__(self, attrs=None, validator_class=URLValidator): final_attrs = {'class': 'vURLField'} if attrs is not None: final_attrs.update(attrs) super(AdminURLFieldWidget, self).__init__(attrs=final_attrs) + self.validator = validator_class() def get_context(self, name, value, attrs): + try: + self.validator(value if value else '') + url_valid = True + except ValidationError: + url_valid = False context = super(AdminURLFieldWidget, self).get_context(name, value, attrs) context['current_label'] = _('Currently:') context['change_label'] = _('Change:') context['widget']['href'] = smart_urlquote(context['widget']['value']) if value else '' + context['url_valid'] = url_valid return context diff --git a/docs/releases/1.11.21.txt b/docs/releases/1.11.21.txt index 75d3599c2a..3da7a78612 100644 --- a/docs/releases/1.11.21.txt +++ b/docs/releases/1.11.21.txt @@ -4,4 +4,18 @@ Django 1.11.21 release notes *June 3, 2019* -Django 1.11.21 fixes security issues in 1.11.20. +Django 1.11.21 fixes a security issue in 1.11.20. + +CVE-2019-12308: AdminURLFieldWidget XSS +--------------------------------------- + +The clickable "Current URL" link generated by ``AdminURLFieldWidget`` displayed +the provided value without validating it as a safe URL. Thus, an unvalidated +value stored in the database, or a value provided as a URL query parameter +payload, could result in an clickable JavaScript link. + +``AdminURLFieldWidget`` now validates the provided value using +:class:`~django.core.validators.URLValidator` before displaying the clickable +link. You may customise the validator by passing a ``validator_class`` kwarg to +``AdminURLFieldWidget.__init__()``, e.g. when using +:attr:`~django.contrib.admin.ModelAdmin.formfield_overrides`. diff --git a/tests/admin_widgets/tests.py b/tests/admin_widgets/tests.py index 4d78d5fd5b..96fbfc1252 100644 --- a/tests/admin_widgets/tests.py +++ b/tests/admin_widgets/tests.py @@ -336,6 +336,12 @@ class AdminSplitDateTimeWidgetTest(SimpleTestCase): class AdminURLWidgetTest(SimpleTestCase): + def test_get_context_validates_url(self): + w = widgets.AdminURLFieldWidget() + for invalid in ['', '/not/a/full/url/', 'javascript:alert("Danger XSS!")']: + self.assertFalse(w.get_context('name', invalid, {})['url_valid']) + self.assertTrue(w.get_context('name', 'http://example.com', {})['url_valid']) + def test_render(self): w = widgets.AdminURLFieldWidget() self.assertHTMLEqual( @@ -369,31 +375,31 @@ class AdminURLWidgetTest(SimpleTestCase): VALUE_RE = re.compile('value="([^"]+)"') TEXT_RE = re.compile(']+>([^>]+)') w = widgets.AdminURLFieldWidget() - output = w.render('test', 'http://example.com/some text') + output = w.render('test', 'http://example.com/some-text') self.assertEqual( HREF_RE.search(output).groups()[0], - 'http://example.com/%3Csometag%3Esome%20text%3C/sometag%3E', + 'http://example.com/%3Csometag%3Esome-text%3C/sometag%3E', ) self.assertEqual( TEXT_RE.search(output).groups()[0], - 'http://example.com/<sometag>some text</sometag>', + 'http://example.com/<sometag>some-text</sometag>', ) self.assertEqual( VALUE_RE.search(output).groups()[0], - 'http://example.com/<sometag>some text</sometag>', + 'http://example.com/<sometag>some-text</sometag>', ) - output = w.render('test', 'http://example-äüö.com/some text') + output = w.render('test', 'http://example-äüö.com/some-text') self.assertEqual( HREF_RE.search(output).groups()[0], - 'http://xn--example--7za4pnc.com/%3Csometag%3Esome%20text%3C/sometag%3E', + 'http://xn--example--7za4pnc.com/%3Csometag%3Esome-text%3C/sometag%3E', ) self.assertEqual( TEXT_RE.search(output).groups()[0], - 'http://example-äüö.com/<sometag>some text</sometag>', + 'http://example-äüö.com/<sometag>some-text</sometag>', ) self.assertEqual( VALUE_RE.search(output).groups()[0], - 'http://example-äüö.com/<sometag>some text</sometag>', + 'http://example-äüö.com/<sometag>some-text</sometag>', ) output = w.render('test', 'http://www.example.com/%C3%A4">"') self.assertEqual(