Fixed django-cms#5938 -- Ensure page redirect url is a valid url

Author: czpython

CHANGELOG.txt

@@ -19,7 +19,8 @@
 
 === 3.4.3 (2017-04-24) ===
 
-
+* Fixed a security vulnerability in the page redirect field which allowed users
+  to insert javascript code.
 
 
 === 3.4.2 (2017-01-23) ===

cms/forms/fields.py

@@ -5,6 +5,7 @@
 from django.utils.translation import ugettext_lazy as _
 
 from cms.forms.utils import get_site_choices, get_page_choices
+from cms.forms.validators import validate_url
 from cms.forms.widgets import PageSelectWidget, PageSmartLinkWidget
 from cms.models.pagemodel import Page
 
@@ -75,8 +76,10 @@ def has_changed(self, initial, data):
     def _has_changed(self, initial, data):
         return self.has_changed(initial, data)
 
+
 class PageSmartLinkField(forms.CharField):
     widget = PageSmartLinkWidget
+    default_validators = [validate_url]
 
     def __init__(self, max_length=None, min_length=None, placeholder_text=None,
                  ajax_view=None, *args, **kwargs):
@@ -89,3 +92,7 @@ def widget_attrs(self, widget):
         attrs = super(PageSmartLinkField, self).widget_attrs(widget)
         attrs.update({'placeholder_text': self.placeholder_text})
         return attrs
+
+    def clean(self, value):
+        value = self.to_python(value).strip()
+        return super(PageSmartLinkField, self).clean(value)

cms/forms/validators.py

@@ -0,0 +1,13 @@
+from django.core.exceptions import ValidationError
+from django.core.validators import RegexValidator, URLValidator
+
+from cms.utils.urlutils import relative_url_regex
+
+
+def validate_url(value):
+    try:
+        # Validate relative urls first
+        RegexValidator(regex=relative_url_regex)(value)
+    except ValidationError:
+        # Fallback to absolute urls
+        URLValidator()(value)

cms/tests/test_page_admin.py

@@ -344,6 +344,44 @@ def test_edit_page_sets_publisher_dirty(self):
                 self.client.post(endpoint, page_data)
                 self.assertTrue(page.reload().is_dirty('en'), change_message.format(field))
 
+    def test_page_redirect_field_validation(self):
+        superuser = self.get_superuser()
+        data = self.get_new_page_data()
+
+        with self.login_user_context(superuser):
+            self.client.post(URL_CMS_PAGE_ADD, data)
+
+        page = Page.objects.get(title_set__slug=data['slug'], publisher_is_draft=True)
+
+        endpoint = URL_CMS_PAGE_ADVANCED_CHANGE % page.pk
+        redirect_to = URL_CMS_PAGE
+
+        with self.login_user_context(superuser):
+            data['redirect'] = '/hello/'
+            # Absolute paths should continue to work
+            response = self.client.post(endpoint, data)
+            self.assertRedirects(response, redirect_to)
+
+        with self.login_user_context(superuser):
+            data['redirect'] = '../hello/'
+            # Relative paths should continue to work
+            response = self.client.post(endpoint, data)
+            self.assertRedirects(response, redirect_to)
+
+        with self.login_user_context(superuser):
+            data['redirect'] = 'javascript:alert(1)'
+            # Asserts users can't insert javascript call
+            response = self.client.post(endpoint, data)
+            validation_error = '<ul class="errorlist"><li>Enter a valid URL.</li></ul>'
+            self.assertContains(response, validation_error, html=True)
+
+        with self.login_user_context(superuser):
+            data['redirect'] = '<script>alert("test")</script>'
+            # Asserts users can't insert javascript call
+            response = self.client.post(endpoint, data)
+            validation_error = '<ul class="errorlist"><li>Enter a valid URL.</li></ul>'
+            self.assertContains(response, validation_error, html=True)
+
     def test_moderator_edit_page_redirect(self):
         """
         Test that a page can be edited multiple times with moderator

cms/utils/urlutils.py

@@ -14,6 +14,14 @@
 # checks validity of absolute / relative url
 any_path_re = re.compile('^/?[a-zA-Z0-9_.-]+(/[a-zA-Z0-9_.-]+)*/?$')
 
+# checks validity of relative url
+# matches the following:
+# /test
+# /test/
+# ./test/
+# ../test/
+relative_url_regex = re.compile('^[^/<>]+/[^/<>].*$|^/[^/<>].*$', re.IGNORECASE)
+
 
 def levelize_path(path):
     """Splits given path to list of paths removing latest level in each step.