From dc76e18cd7d21007b5724770b3b464453e5124f9 Mon Sep 17 00:00:00 2001 From: Adam Murray Date: Tue, 13 Oct 2020 14:02:24 +0100 Subject: [PATCH 1/4] Added test to ensure response from clipboard admin does not pass on scripts within parameters --- tests/test_views.py | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/tests/test_views.py b/tests/test_views.py index 334cb83..79ae484 100644 --- a/tests/test_views.py +++ b/tests/test_views.py @@ -397,6 +397,20 @@ def test_ajax_upload_clipboardadmin_same_name_as_existing_file_in_moderation(sel error_msg = 'Cannot archive existing test1.jpg file version' self.assertEqual(response.json()['error'], error_msg) + def test_ajax_upload_clipboardadmin_xss_vulnerability(self): + """ + If we add malicious data to an ajax upload request ensure it is stripped in response. + """ + file = self.create_file('test2.pdf') + + with self.login_user_context(self.superuser): + response = self.client.post( + reverse('admin:filer-ajax_upload', kwargs={'folder_id': self.folder.id}), + data={'file': file, 'script': ''}, + ) + + self.assertNotContains(response, '') + def test_folderadmin_directory_listing(self): folder = Folder.objects.create(name='test folder 9') file_grouper_1 = FileGrouper.objects.create() From 1c48b56c96f96587d22068b65e29d093ccdf63d8 Mon Sep 17 00:00:00 2001 From: Adam Murray Date: Wed, 14 Oct 2020 16:46:38 +0100 Subject: [PATCH 2/4] Updated test to properly cover unsanitised path --- tests/test_views.py | 22 +++++++++++++++++----- 1 file changed, 17 insertions(+), 5 deletions(-) diff --git a/tests/test_views.py b/tests/test_views.py index 79ae484..c74e333 100644 --- a/tests/test_views.py +++ b/tests/test_views.py @@ -7,7 +7,7 @@ from django.contrib.admin import helpers from django.contrib.contenttypes.models import ContentType from django.core.files import File as DjangoFile -from django.urls import reverse +from django.urls import reverse, NoReverseMatch from cms.test_utils.testcases import CMSTestCase from cms.utils.urlutils import add_url_parameters @@ -18,6 +18,7 @@ from filer.models import File, Folder from djangocms_versioning_filer.models import FileGrouper +from djangocms_versioning_filer.monkeypatch.admin.clipboardadmin import ajax_upload from .base import BaseFilerVersioningTestCase @@ -401,15 +402,26 @@ def test_ajax_upload_clipboardadmin_xss_vulnerability(self): """ If we add malicious data to an ajax upload request ensure it is stripped in response. """ + with self.login_user_context(self.superuser): + response = self.client.post( + reverse('admin:filer-ajax_upload'), + data={'path': ''} + ) + + self.assertFalse('' in response.request.values()) + self.assertEqual(response.status_code, 500) + file = self.create_file('test2.pdf') with self.login_user_context(self.superuser): response = self.client.post( - reverse('admin:filer-ajax_upload', kwargs={'folder_id': self.folder.id}), - data={'file': file, 'script': ''}, - ) + reverse('admin:filer-ajax_upload'), + data={'path': '', 'file': file} + ) + + self.assertNotContains(response, '') + self.assertEqual(response.status_code, 200) - self.assertNotContains(response, '') def test_folderadmin_directory_listing(self): folder = Folder.objects.create(name='test folder 9') From 6fe64630237a9a2824513802ea0902776d96c1a7 Mon Sep 17 00:00:00 2001 From: Adam Murray Date: Wed, 14 Oct 2020 17:49:06 +0100 Subject: [PATCH 3/4] Split tests for xss into two --- tests/test_views.py | 27 ++++++++++++++++----------- 1 file changed, 16 insertions(+), 11 deletions(-) diff --git a/tests/test_views.py b/tests/test_views.py index c74e333..d3c82e7 100644 --- a/tests/test_views.py +++ b/tests/test_views.py @@ -398,19 +398,11 @@ def test_ajax_upload_clipboardadmin_same_name_as_existing_file_in_moderation(sel error_msg = 'Cannot archive existing test1.jpg file version' self.assertEqual(response.json()['error'], error_msg) - def test_ajax_upload_clipboardadmin_xss_vulnerability(self): + def test_ajax_upload_clipboardadmin_xss_vulnerability_path_param_only(self): """ - If we add malicious data to an ajax upload request ensure it is stripped in response. + If we add malicious data to the path post attribute with an additional file attribute + to an ajax upload request ensure it is stripped in response. """ - with self.login_user_context(self.superuser): - response = self.client.post( - reverse('admin:filer-ajax_upload'), - data={'path': ''} - ) - - self.assertFalse('' in response.request.values()) - self.assertEqual(response.status_code, 500) - file = self.create_file('test2.pdf') with self.login_user_context(self.superuser): @@ -422,6 +414,19 @@ def test_ajax_upload_clipboardadmin_xss_vulnerability(self): self.assertNotContains(response, '') self.assertEqual(response.status_code, 200) + def test_ajax_upload_clipboardadmin_xss_vulnerability_pathand_file_param(self): + """ + If we add malicious data to the path post attribute of an ajax upload request ensure it is stripped in response. + """ + with self.login_user_context(self.superuser): + response = self.client.post( + reverse('admin:filer-ajax_upload'), + data={'path': ''} + ) + + self.assertFalse('' in response.request.values()) + self.assertEqual(response.status_code, 500) + def test_folderadmin_directory_listing(self): folder = Folder.objects.create(name='test folder 9') From e822cd51c1e60721ae5124f4e1a339729d10d120 Mon Sep 17 00:00:00 2001 From: Adam Murray Date: Wed, 14 Oct 2020 18:27:29 +0100 Subject: [PATCH 4/4] iSort --- tests/test_views.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/tests/test_views.py b/tests/test_views.py index d3c82e7..541dd1a 100644 --- a/tests/test_views.py +++ b/tests/test_views.py @@ -7,7 +7,7 @@ from django.contrib.admin import helpers from django.contrib.contenttypes.models import ContentType from django.core.files import File as DjangoFile -from django.urls import reverse, NoReverseMatch +from django.urls import reverse from cms.test_utils.testcases import CMSTestCase from cms.utils.urlutils import add_url_parameters @@ -18,7 +18,6 @@ from filer.models import File, Folder from djangocms_versioning_filer.models import FileGrouper -from djangocms_versioning_filer.monkeypatch.admin.clipboardadmin import ajax_upload from .base import BaseFilerVersioningTestCase