From deeba6d92006999fee9adfbd8be79bf0a59e8008 Mon Sep 17 00:00:00 2001 From: Carlton Gibson Date: Thu, 23 May 2019 12:06:34 +0200 Subject: [PATCH] Fixed CVE-2019-12308 -- Made AdminURLFieldWidget validate URL before rendering clickable link. --- .../admin/templates/admin/widgets/url.html | 2 +- django/contrib/admin/widgets.py | 10 +++++++- docs/releases/1.11.21.txt | 16 ++++++++++++- docs/releases/2.1.9.txt | 14 +++++++++++ docs/releases/2.2.2.txt | 14 +++++++++++ tests/admin_widgets/tests.py | 23 ++++++++++++------- 6 files changed, 68 insertions(+), 11 deletions(-) diff --git a/django/contrib/admin/templates/admin/widgets/url.html b/django/contrib/admin/templates/admin/widgets/url.html index ee1a66a35f..69dc401ba1 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 81dbcaf236..aa4b613894 100644 --- a/django/contrib/admin/widgets.py +++ b/django/contrib/admin/widgets.py @@ -7,6 +7,7 @@ import json from django import forms from django.conf import settings 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 @@ -330,14 +331,21 @@ 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): super().__init__(attrs={'class': 'vURLField', **(attrs or {})}) + 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().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/docs/releases/2.1.9.txt b/docs/releases/2.1.9.txt index 765643759d..0022de965c 100644 --- a/docs/releases/2.1.9.txt +++ b/docs/releases/2.1.9.txt @@ -5,3 +5,17 @@ Django 2.1.9 release notes *June 3, 2019* Django 2.1.9 fixes security issues in 2.1.8. + +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/docs/releases/2.2.2.txt b/docs/releases/2.2.2.txt index f0b97c9f26..8c70d104d7 100644 --- a/docs/releases/2.2.2.txt +++ b/docs/releases/2.2.2.txt @@ -6,6 +6,20 @@ Django 2.2.2 release notes Django 2.2.2 fixes security issues and several bugs in 2.2.1. +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`. + Bugfixes ======== diff --git a/tests/admin_widgets/tests.py b/tests/admin_widgets/tests.py index a7335f8f69..18475658c9 100644 --- a/tests/admin_widgets/tests.py +++ b/tests/admin_widgets/tests.py @@ -333,6 +333,13 @@ 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!")']: + with self.subTest(url=invalid): + 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( @@ -366,31 +373,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(