diff --git a/CHANGELOG.rst b/CHANGELOG.rst index 8ea195c1..21ad9c01 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -4,7 +4,8 @@ Changelog Unreleased ========== -* Pin the following packages to a python 3.6 compatible version matching this release stream: django-admin-sortable2, djangocms-text-ckeditor, djangocms-version-locking and djangocms-versioning. +* fix: Refactor flawed add to collection XSS redirect sanitisation added in 1.0.26 +* Pin the following packages to a python 3.6 compatible version matching this release stream: django-admin-sortable2, djangocms-text-ckeditor, djangocms-version-locking and djangocms-versioning. 1.0.28 (2021-10-18) =================== diff --git a/djangocms_moderation/views.py b/djangocms_moderation/views.py index cee30199..43e1ecd3 100644 --- a/djangocms_moderation/views.py +++ b/djangocms_moderation/views.py @@ -1,12 +1,14 @@ from __future__ import unicode_literals +from urllib.parse import quote + from django.contrib import admin, messages from django.db import transaction from django.http import Http404, HttpResponseRedirect from django.shortcuts import get_object_or_404, render from django.urls import reverse from django.utils.decorators import method_decorator -from django.utils.http import is_safe_url, urlquote +from django.utils.http import is_safe_url from django.utils.translation import ugettext_lazy as _, ungettext from django.views.generic import FormView @@ -88,7 +90,9 @@ def _get_success_redirect(self): allowed_hosts=self.request.get_host(), require_https=self.request.is_secure(), ) - return_to_url = urlquote(return_to_url) + # Protect against refracted XSS attacks + # Allow : in http://, ?=& for GET parameters + return_to_url = quote(return_to_url, safe='/:?=&') if not url_is_safe: return_to_url = self.request.path return HttpResponseRedirect(return_to_url) diff --git a/tests/test_views.py b/tests/test_views.py index d496d48d..c59e1ebe 100644 --- a/tests/test_views.py +++ b/tests/test_views.py @@ -6,7 +6,7 @@ from django.urls import reverse from cms.test_utils.testcases import CMSTestCase -from cms.utils.urlutils import add_url_parameters +from cms.utils.urlutils import add_url_parameters, admin_reverse from djangocms_versioning.test_utils.factories import PageVersionFactory @@ -550,15 +550,20 @@ def test_adding_page_not_by_the_author_doesnt_trigger_nested_collection_mechanis ModerationRequestTreeNode.objects.filter(moderation_request=mr1.first()).count(), 0 ) - def test_collection_with_redirect_url_query_string_parameter_sanitisation(self): + def test_collection_with_redirect_url_query_redirect_sanitisation(self): + """ + Reflected XSS Protection by ensuring that harmful characters are encoded + + When a collection is successful a redirect occurs back to the grouper in versioning, + this functionality should continue to function even when sanitised! + """ user = self.get_superuser() collection = ModerationCollectionFactory(author=user) page1_version = PageVersionFactory(created_by=user) page2_version = PageVersionFactory(created_by=user) - - redirect_to_url = f"""{reverse( - "admin:djangocms_moderation_moderationcollection_changelist" - )}?=""" + opts = page1_version.versionable.version_model_proxy._meta + redirect_to_url = admin_reverse(f"{opts.app_label}_{opts.model_name}_changelist") + redirect_to_url += f"?page={page1_version.content.page.id}&" url = add_url_parameters( get_admin_url( @@ -578,8 +583,19 @@ def test_collection_with_redirect_url_query_string_parameter_sanitisation(self): follow=False, ) + messages = [message.message for message in list(get_messages(response.wsgi_request))] + self.assertEqual(response.status_code, 302) - self.assertIn("%3F%3D%3Cscript%3Ealert%28%27attack%21%27%29%3C/script%3E", response.url) + self.assertIn("%3Cscript%3Ealert%28%27attack%21%27%29%3C/script%3E", response.url) + self.assertIn(f"?page={page1_version.content.page.id}", response.url) + self.assertIn( + "2 items successfully added to moderation collection", + messages, + ) + self.assertNotIn( + "Perhaps it was deleted", + messages, + ) class CollectionItemsViewTest(CMSTestCase):