From 470997ae3b85b2ab6fa5d01304564e9a23677f74 Mon Sep 17 00:00:00 2001 From: marcosptf Date: Sun, 19 May 2019 10:53:19 -0300 Subject: [PATCH 1/6] WIP: fixing XSS vulnerability --- quokka/admin/actions.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/quokka/admin/actions.py b/quokka/admin/actions.py index 600d603d5..d29ab17e6 100644 --- a/quokka/admin/actions.py +++ b/quokka/admin/actions.py @@ -82,6 +82,10 @@ def action_create_userprofile(self, ids): existing_block = current_app.db.get( 'index', {'content_type': 'block', 'slug': fullslug} ) + + #fix vulnerabillity here + #test sanity variables values + if existing_block: blocklink = url_for( 'quokka.core.content.admin.blockview.edit_view', From 538b074e792bef1abd293af7021cbada8b1207c8 Mon Sep 17 00:00:00 2001 From: marcosptf Date: Thu, 6 Jun 2019 08:45:50 -0300 Subject: [PATCH 2/6] fixing pep8 --- quokka/admin/actions.py | 4 ++-- quokka/admin/wtforms_html5.py | 2 +- quokka/core/content/models.py | 14 +++++++------- quokka/core/content/utils.py | 5 +++-- quokka/core/content/views.py | 5 +++-- quokka/core/db.py | 19 ++++++++----------- quokka/core/views/sitemap.py | 10 +++++----- quokka/utils/upload.py | 2 +- 8 files changed, 30 insertions(+), 31 deletions(-) diff --git a/quokka/admin/actions.py b/quokka/admin/actions.py index d29ab17e6..42c4b17de 100644 --- a/quokka/admin/actions.py +++ b/quokka/admin/actions.py @@ -83,8 +83,8 @@ def action_create_userprofile(self, ids): 'index', {'content_type': 'block', 'slug': fullslug} ) - #fix vulnerabillity here - #test sanity variables values + # fix vulnerabillity here + # test sanity variables values if existing_block: blocklink = url_for( diff --git a/quokka/admin/wtforms_html5.py b/quokka/admin/wtforms_html5.py index ccc3a1c09..85e01933f 100644 --- a/quokka/admin/wtforms_html5.py +++ b/quokka/admin/wtforms_html5.py @@ -195,7 +195,7 @@ def set_title(field, render_kw=None): """ if render_kw is None: render_kw = {} - if 'title' not in render_kw and getattr(field, 'description'): + if 'title' not in render_kw and getattr(field, 'description', None): render_kw['title'] = '{}'.format(field.description) return render_kw diff --git a/quokka/core/content/models.py b/quokka/core/content/models.py index 6d2fcc4d9..5358ed079 100644 --- a/quokka/core/content/models.py +++ b/quokka/core/content/models.py @@ -230,13 +230,13 @@ def metadata(self): # TODO: get metadata from database # TODO: implement libratar/gravatar # return { - # 'cover': 'foo', - # 'author_gravatar': 'http://i.pravatar.cc/300', - # 'about_author': 'About Author', - # 'translations': ['en'], - # 'og_image': 'foo', - # 'series': 'aa', - # 'asides': 'aaa' + # 'cover': 'foo', + # 'author_gravatar': 'http://i.pravatar.cc/300', + # 'about_author': 'About Author', + # 'translations': ['en'], + # 'og_image': 'foo', + # 'series': 'aa', + # 'asides': 'aaa' # } data = {} data.update(custom_var_dict(self.data.get('custom_vars'))) diff --git a/quokka/core/content/utils.py b/quokka/core/content/utils.py index 57aaef312..9d5deb8ce 100644 --- a/quokka/core/content/utils.py +++ b/quokka/core/content/utils.py @@ -10,9 +10,10 @@ def url_for_content(content, include_ext=True): else: data = content + category_slug_data = data.get('category_slug') + category_data = slugify_category(data.get('category') or '') category_slug = ( - data.get('category_slug') or - slugify_category(data.get('category') or '') + category_slug_data or category_data ) slug = data.get('slug') or slugify(data.get('title')) diff --git a/quokka/core/content/views.py b/quokka/core/content/views.py index 74c72d288..08d6d9bc1 100644 --- a/quokka/core/content/views.py +++ b/quokka/core/content/views.py @@ -255,6 +255,8 @@ def render_rss(self, content_type, templates, **context): for content in contents: content = make_model(content) + content_data = content.title.encode('utf-8') + content_data += content.url.encode('utf-8') if content.date > rss_pubdate: rss_pubdate = content.date @@ -267,8 +269,7 @@ def render_rss(self, content_type, templates, **context): author=str(content.author), categories=[str(content.tags)], guid=hashlib.sha1( - content.title.encode('utf-8') + - content.url.encode('utf-8') + content_data ).hexdigest(), pubDate=content.date, ) diff --git a/quokka/core/db.py b/quokka/core/db.py index 431c55aea..afc09ad77 100644 --- a/quokka/core/db.py +++ b/quokka/core/db.py @@ -183,17 +183,14 @@ def page_set(self, *args, **kwargs): return self.content_set(*args, **kwargs) def block_set(self, *args, **kwargs): - kwargs.setdefault( - 'sort', - self.app.theme_context.get( - 'BLOCK_ORDER_BY', [('title', -1)] - ) - ) - if not args: - args = [{'content_type': 'block'}] - elif isinstance(args[0], dict): - args[0]['content_type'] = 'block' - return self.content_set(*args, **kwargs) + kwargs.setdefault('sort', self.app.theme_context.get( + 'BLOCK_ORDER_BY', [('title', -1)] + )) + if not args: + args = [{'content_type': 'block'}] + elif isinstance(args[0], dict): + args[0]['content_type'] = 'block' + return self.content_set(*args, **kwargs) def select(self, colname, *args, **kwargs): return self.get_collection(colname).find(*args, **kwargs) diff --git a/quokka/core/views/sitemap.py b/quokka/core/views/sitemap.py index 871372817..2cdca943b 100644 --- a/quokka/core/views/sitemap.py +++ b/quokka/core/views/sitemap.py @@ -11,12 +11,12 @@ def get_contents(self): TODO: Should include extra paths, fixed paths config based paths, static paths """ + content = self.get_index() + self.get_categories() + content += self.get_tags() + self.get_authors() + content += self.get_articles_and_pages() + return ( - self.get_index() + - self.get_categories() + - self.get_tags() + - self.get_authors() + - self.get_articles_and_pages() + content ) def get_index(self): diff --git a/quokka/utils/upload.py b/quokka/utils/upload.py index 251e1c50f..bc8042679 100644 --- a/quokka/utils/upload.py +++ b/quokka/utils/upload.py @@ -9,7 +9,7 @@ def dated_path(obj, file_data): try: - prefix = getattr(obj, 'model_name') + prefix = getattr(obj, 'model_name', None) except BaseException: prefix = "undefined" From 7156c76b3ab3973059d9dad835dff21f071d2086 Mon Sep 17 00:00:00 2001 From: marcosptf Date: Fri, 7 Jun 2019 07:05:44 -0300 Subject: [PATCH 3/6] fixing XSS issue --- quokka/admin/actions.py | 7 ++++--- quokka/utils/text.py | 12 ++++++++++++ 2 files changed, 16 insertions(+), 3 deletions(-) diff --git a/quokka/admin/actions.py b/quokka/admin/actions.py index 42c4b17de..90c3cc2c7 100644 --- a/quokka/admin/actions.py +++ b/quokka/admin/actions.py @@ -8,7 +8,7 @@ from flask import Response, current_app, flash, redirect, url_for from flask_admin.actions import action -from quokka.utils.text import slugify +from quokka.utils.text import slugify, remove_tags_from_string class PublishAction(object): @@ -83,8 +83,9 @@ def action_create_userprofile(self, ids): 'index', {'content_type': 'block', 'slug': fullslug} ) - # fix vulnerabillity here - # test sanity variables values + # fix vulnerabillity here XSS + user['fullname'] = remove_tags_from_string(user['fullname']) + user['username'] = remove_tags_from_string(user['username']) if existing_block: blocklink = url_for( diff --git a/quokka/utils/text.py b/quokka/utils/text.py index 1612f6666..b0ed0bd14 100644 --- a/quokka/utils/text.py +++ b/quokka/utils/text.py @@ -91,3 +91,15 @@ def split_all_category_roots(cat): return cats else: return [cat] + + +def remove_tags_from_string(data): + """remove tags html, commas, semicolon + and double quote from string prevent XSS + """ + resp = re.sub('<[^>]*>', '', data) + resp = re.sub('"', '', resp) + resp = re.sub('\(', '', resp) + resp = re.sub('\)', '', resp) + return re.sub(';', '', resp) + From 313605921b272aed250ce92c6eb352853fb60f46 Mon Sep 17 00:00:00 2001 From: marcosptf Date: Fri, 7 Jun 2019 07:13:55 -0300 Subject: [PATCH 4/6] fixing patterns and add import regex --- quokka/utils/text.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/quokka/utils/text.py b/quokka/utils/text.py index b0ed0bd14..1dbdd8ebb 100644 --- a/quokka/utils/text.py +++ b/quokka/utils/text.py @@ -1,3 +1,4 @@ +import re from flask import request from urllib.parse import urljoin from slugify.main import Slugify @@ -99,7 +100,6 @@ def remove_tags_from_string(data): """ resp = re.sub('<[^>]*>', '', data) resp = re.sub('"', '', resp) - resp = re.sub('\(', '', resp) - resp = re.sub('\)', '', resp) + resp = re.sub('(', '', resp) + resp = re.sub(')', '', resp) return re.sub(';', '', resp) - From 10b1d22d57af9197eacb45c80d0806e67be3a62c Mon Sep 17 00:00:00 2001 From: marcosptf Date: Fri, 7 Jun 2019 07:53:07 -0300 Subject: [PATCH 5/6] add pytest to hotfix/issue675 --- quokka/utils/text.py | 13 ++-- requirements-dev.txt | 1 + tests/test_basic.py | 156 ++++++++++++++++++++++++++++++++++++++++++- 3 files changed, 164 insertions(+), 6 deletions(-) diff --git a/quokka/utils/text.py b/quokka/utils/text.py index 1dbdd8ebb..f0a6195cb 100644 --- a/quokka/utils/text.py +++ b/quokka/utils/text.py @@ -99,7 +99,12 @@ def remove_tags_from_string(data): and double quote from string prevent XSS """ resp = re.sub('<[^>]*>', '', data) - resp = re.sub('"', '', resp) - resp = re.sub('(', '', resp) - resp = re.sub(')', '', resp) - return re.sub(';', '', resp) + return resp.replace( + '"', '' + ).replace( + ';', '' + ).replace( + '(', '' + ).replace( + ')', '' + ) diff --git a/requirements-dev.txt b/requirements-dev.txt index ba32aa2f0..7eda9b364 100644 --- a/requirements-dev.txt +++ b/requirements-dev.txt @@ -12,3 +12,4 @@ twine>=1.9.1 wheel>=0.30.0 flask>=1.0 logzero +pytest_mock diff --git a/tests/test_basic.py b/tests/test_basic.py index 1f9418114..2157ec4e1 100644 --- a/tests/test_basic.py +++ b/tests/test_basic.py @@ -1,3 +1,155 @@ +import mock +import quokka +from quokka import create_app, create_app_base +from pytest_mock import mocker +from quokka.core.app import QuokkaApp +from quokka.core.flask_dynaconf import configure_dynaconf + + +################################ +# pytest - fixtures - setUp(); # +################################ +class MockTestApp(object): + + def __init__(self, config): + self.config = config + return self.config + + +##################################### +# pytest - Quokka - test__init__.py # +##################################### +def test_create_app_called_params_default(mocker): + mocker.patch("quokka.create_app_base") + mocker.patch("quokka.core.configure_extensions") + quokka.create_app() + quokka.create_app_base.assert_called_once_with(test=False) + + +def test_create_app_called_test_false(mocker): + mocker.patch("quokka.create_app_base") + mocker.patch("quokka.core.configure_extensions") + quokka.create_app(test=False) + quokka.create_app_base.assert_called_once_with(test=False) + + +def test_create_app_called_test_true(mocker): + mocker.patch("quokka.create_app_base") + mocker.patch("quokka.core.configure_extensions") + quokka.create_app(test=True) + quokka.create_app_base.assert_called_once_with(test=True) + + +def test_create_app_called_test_true_and_settings_dict(mocker): + mocker.patch("quokka.create_app_base") + mocker.patch("quokka.core.configure_extensions") + quokka.create_app(test=True, settings={'a':'1', 'b':'2', 'c':'3', 'd':'4', 'e':'5'}) + quokka.create_app_base.assert_called_once_with(test=True, settings={'a':'1', 'b':'2', 'c':'3', 'd':'4', 'e':'5'}) + + +@mock.patch("quokka.core.app.QuokkaApp") +@mock.patch("quokka.core.flask_dynaconf.configure_dynaconf") +@mock.patch("quokka.core.configure_extension") +def test_create_app_base_function_quokkaapp_called_is_false(mock_configure_extension, mock_configure_dynaconf, mock_QuokkaApp): + quokka.create_app_base(test=False, ext_list=None) + assert mock_QuokkaApp.called is False + + +@mock.patch("quokka.core.app.QuokkaApp") +@mock.patch("quokka.core.flask_dynaconf.configure_dynaconf") +@mock.patch("quokka.core.configure_extension") +def test_create_app_base_function_dynaconf_called_is_false(mock_configure_extension, mock_configure_dynaconf, mock_QuokkaApp): + quokka.create_app_base(test=False, ext_list=None) + assert mock_configure_dynaconf.called is False + + +@mock.patch("quokka.core.app.QuokkaApp") +@mock.patch("quokka.core.flask_dynaconf.configure_dynaconf") +@mock.patch("quokka.core.configure_extension") +def test_create_app_base_function_configure_extension_called_is_false(mock_configure_extension, mock_configure_dynaconf, mock_QuokkaApp): + quokka.create_app_base(test=False, ext_list=None) + assert mock_configure_extension.called is False + + +@mock.patch("quokka.core.app.QuokkaApp") +@mock.patch("quokka.core.flask_dynaconf.configure_dynaconf") +@mock.patch("quokka.core.configure_extension") +def test_create_app_base_function_quokkaapp_called_is_false_and_test_true(mock_configure_extension, mock_configure_dynaconf, mock_QuokkaApp): + quokka.create_app_base(test=True, ext_list=[]) + assert mock_QuokkaApp.called is False + + +@mock.patch("quokka.core.app.QuokkaApp") +@mock.patch("quokka.core.flask_dynaconf.configure_dynaconf") +@mock.patch("quokka.core.configure_extension") +def test_create_app_base_function_dynaconf_called_is_false_test_true_and_ext_list(mock_configure_extension, mock_configure_dynaconf, mock_QuokkaApp): + quokka.create_app_base(test=True, ext_list=['quokka.core.configure_extension']) + assert mock_configure_dynaconf.called is False + + +@mock.patch("quokka.core.app.QuokkaApp") +@mock.patch("quokka.core.flask_dynaconf.configure_dynaconf") +@mock.patch("quokka.core.configure_extension") +def test_create_app_base_function_configure_dynaconf_called_is_true(mock_configure_extension, mock_configure_dynaconf, mock_QuokkaApp): + list_ext = ['quokka.core.app.QuokkaApp', + 'quokka.core.flask_dynaconf.configure_dynaconf', + 'quokka.core.configure_extension'] + quokka.create_app_base(test=True, ext_list=list_ext) + assert mock_configure_dynaconf.called is True + + +@mock.patch("quokka.core.app.QuokkaApp") +@mock.patch("quokka.core.flask_dynaconf.configure_dynaconf") +@mock.patch("quokka.core.configure_extension") +def test_create_app_base_function_quokkaapp_called_is_true(mock_configure_extension, mock_configure_dynaconf, mock_QuokkaApp): + list_ext = ['quokka.core.app.QuokkaApp', + 'quokka.core.flask_dynaconf.configure_dynaconf', + 'quokka.core.configure_extension'] + quokka.create_app_base(test=True, ext_list=list_ext) + assert mock_QuokkaApp.called is True + + +@mock.patch("quokka.core.app.QuokkaApp") +@mock.patch("quokka.core.flask_dynaconf.configure_dynaconf") +@mock.patch("quokka.core.configure_extension") +def test_create_app_base_function_configure_extension_called_is_true(mock_configure_extension, mock_configure_dynaconf, mock_QuokkaApp): + list_ext = ['quokka.core.app.QuokkaApp', + 'quokka.core.flask_dynaconf.configure_dynaconf', + 'quokka.core.configure_extension'] + quokka.create_app_base(test=True, ext_list=list_ext) + assert mock_configure_extension.called is True + + +@mock.patch("quokka.core.app.QuokkaApp") +@mock.patch("quokka.core.flask_dynaconf.configure_dynaconf") +@mock.patch("quokka.core.configure_extension") +def test_create_app_base_function_configure_extension_called_is_true_and_settings(mock_configure_extension, mock_configure_dynaconf, mock_QuokkaApp): + list_ext = ['quokka.core.app.QuokkaApp', + 'quokka.core.flask_dynaconf.configure_dynaconf', + 'quokka.core.configure_extension'] + quokka.create_app_base(test=True, ext_list=list_ext, settings={'a':'1', 'b':'2', 'c':'3', 'd':'4', 'e':'5'}) + assert mock_configure_extension.called is True + + +@mock.patch("quokka.core.app.QuokkaApp") +@mock.patch("quokka.core.flask_dynaconf.configure_dynaconf") +@mock.patch("quokka.core.configure_extension") +def test_create_app_base_function_configure_dynaconf_called_is_true_and_settings(mock_configure_extension, mock_configure_dynaconf, mock_QuokkaApp): + list_ext = ['quokka.core.app.QuokkaApp', + 'quokka.core.flask_dynaconf.configure_dynaconf', + 'quokka.core.configure_extension'] + quokka.create_app_base(test=True, ext_list=list_ext, settings={'a':'1', 'b':'2', 'c':'3', 'd':'4', 'e':'5'}) + assert mock_configure_dynaconf.called is True + + +@mock.patch("quokka.core.app.QuokkaApp") +@mock.patch("quokka.core.flask_dynaconf.configure_dynaconf") +@mock.patch("quokka.core.configure_extension") +def test_create_app_base_function_quokkaapp_called_is_true_and_settings(mock_configure_extension, mock_configure_dynaconf, mock_QuokkaApp): + list_ext = ['quokka.core.app.QuokkaApp', + 'quokka.core.flask_dynaconf.configure_dynaconf', + 'quokka.core.configure_extension'] + quokka.create_app_base(test=True, ext_list=list_ext, settings={'a':'1', 'b':'2', 'c':'3', 'd':'4', 'e':'5'}) + assert mock_QuokkaApp.called is True + -def test_basic(app): - assert app.name == 'quokka' From 1746e24c609f66cc39c9c8b40f904bdd1668dbb6 Mon Sep 17 00:00:00 2001 From: marcosptf Date: Fri, 7 Jun 2019 07:53:57 -0300 Subject: [PATCH 6/6] add pytest to hotfix/issue675 - 2 --- tests/__init__.py | 0 tests/utils/test_text.py | 64 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 64 insertions(+) create mode 100644 tests/__init__.py create mode 100644 tests/utils/test_text.py diff --git a/tests/__init__.py b/tests/__init__.py new file mode 100644 index 000000000..e69de29bb diff --git a/tests/utils/test_text.py b/tests/utils/test_text.py new file mode 100644 index 000000000..42676a5bc --- /dev/null +++ b/tests/utils/test_text.py @@ -0,0 +1,64 @@ +import pytest +from flask import request +from urllib.parse import urljoin +from slugify.main import Slugify +from quokka.utils.text import ( + abbreviate, normalize_var, + make_social_link, make_social_link, + make_social_name, cdata, + make_external_url, split_all_category_roots, + remove_tags_from_string +) + +################################ +#pytest - fixtures - setUp(); # +################################ +slugify = Slugify() +slugify.to_lower = True +slugify_category = Slugify() +slugify_category.to_lower = True +slugify_category.safe_chars = '/' +abbrev = abbreviate("pytest-mock") +norma = normalize_var("http://yahoo.com") +make_link = make_social_link(network="twitter", txt="http://twitter.com/python") +make_name = make_social_name('http://twitter.com/python') +data = cdata("py-cdata") +split = split_all_category_roots(cat="categoria1/categoria2/categoria3") + + +################################## +#pytest - Quokka - test_text.py # +################################## +def test_abbreviate(): + debugger = abbreviate("pytest-mock") + assert abbrev == 'pytest-mock' + +def test_normalize_var(): + assert norma == "http:__yahoo.com" + + +def test_make_social_link(): + assert make_link == 'http://twitter.com/python' + + +def test_make_social_name(): + assert make_name == 'python' + +def test_cdata(): + assert data == '' + +def test_make_external_url(): + with pytest.raises(RuntimeError) as err: + make_external_url("http://it.yahoo.com") + assert "Working outside of application context." in str(err.value) + +def test_split_all_category_roots(): + assert split[0] == 'categoria1/categoria2/categoria3' + assert split[1] == 'categoria1/categoria2' + assert split[2] == 'categoria1' + +def test_remove_tags_from_string(): + assert remove_tags_from_string('') == 'alertpython-quokka' + assert remove_tags_from_string('') == 'console.logpython-quokka' + assert remove_tags_from_string('python-quokka') == 'python-quokka' + assert remove_tags_from_string('') == 'position:relativetop:10pxfloat:left'