From 470997ae3b85b2ab6fa5d01304564e9a23677f74 Mon Sep 17 00:00:00 2001 From: marcosptf Date: Sun, 19 May 2019 10:53:19 -0300 Subject: [PATCH 1/7] 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 0239d7d4abdf3cccd8962c498fb0c56b80489164 Mon Sep 17 00:00:00 2001 From: marcosptf Date: Sun, 19 May 2019 10:56:21 -0300 Subject: [PATCH 2/7] WIP: fixing vulnerability --- quokka/core/content/views.py | 1 + 1 file changed, 1 insertion(+) diff --git a/quokka/core/content/views.py b/quokka/core/content/views.py index 74c72d288..590f875b6 100644 --- a/quokka/core/content/views.py +++ b/quokka/core/content/views.py @@ -91,6 +91,7 @@ def set_elements_visibility(self, context, content_type): class ArticleListView(BaseView): + #apply fixes to vulnerability XXE def get(self, category=None, tag=None, author=None, page_number=1, ext=None): context = {} From 538b074e792bef1abd293af7021cbada8b1207c8 Mon Sep 17 00:00:00 2001 From: marcosptf Date: Thu, 6 Jun 2019 08:45:50 -0300 Subject: [PATCH 3/7] 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 4d07b172037f109b5f1b25760c99d418a0b44438 Mon Sep 17 00:00:00 2001 From: marcosptf Date: Thu, 6 Jun 2019 09:05:39 -0300 Subject: [PATCH 4/7] fixing pep8 --- quokka/core/content/views.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/quokka/core/content/views.py b/quokka/core/content/views.py index b4215e91f..48fe170aa 100644 --- a/quokka/core/content/views.py +++ b/quokka/core/content/views.py @@ -91,7 +91,7 @@ def set_elements_visibility(self, context, content_type): class ArticleListView(BaseView): - #apply fixes to vulnerability XXE + # apply fixes to vulnerability XXE def get(self, category=None, tag=None, author=None, page_number=1, ext=None): context = {} From 1e065a26194d7629b35fee285d911b16a00961b7 Mon Sep 17 00:00:00 2001 From: marcosptf Date: Wed, 17 Jul 2019 07:56:14 -0300 Subject: [PATCH 5/7] add fixing XXE --- quokka/core/content/views.py | 14 ++++++-- quokka/utils/atom.py | 14 ++++++++ quokka/utils/text.py | 18 ++++++++++ tests/utils/test_text.py | 65 ++++++++++++++++++++++++++++++++++++ 4 files changed, 109 insertions(+), 2 deletions(-) create mode 100644 tests/utils/test_text.py diff --git a/quokka/core/content/views.py b/quokka/core/content/views.py index 48fe170aa..013839398 100644 --- a/quokka/core/content/views.py +++ b/quokka/core/content/views.py @@ -8,10 +8,14 @@ # from werkzeug.contrib.atom import AtomFeed # The werkzeug AtomFeed escapes all html tags from quokka.utils.atom import AtomFeed - from .models import make_model, make_paginator, Category, Tag, Author from quokka.utils.text import ( - slugify_category, normalize_var, slugify, cdata, make_external_url + slugify_category, + normalize_var, + slugify, + cdata, + make_external_url, + remove_tags_from_string ) @@ -106,6 +110,7 @@ def get(self, category=None, tag=None, author=None, FEED_ALL_RSS = app.theme_context.get('FEED_ALL_RSS') if category: + category = remove_tags_from_string(category) FEED_ALL_ATOM = f"{category}/index.atom" FEED_ALL_RSS = f"{category}/index.rss" content_type = 'category' @@ -118,6 +123,7 @@ def get(self, category=None, tag=None, author=None, content_type = 'index' else: content_type = 'index' + elif tag: FEED_ALL_ATOM = f"tag/{tag}/index.atom" FEED_ALL_RSS = f"tag/{tag}/index.rss" @@ -126,7 +132,9 @@ def get(self, category=None, tag=None, author=None, template = 'tag.html' # https://github.com/schapman1974/tinymongo/issues/42 query['tags_string'] = {'$regex': f'.*,{tag},.*'} + elif author: + author = remove_tags_from_string(author) FEED_ALL_ATOM = f"author/{author}/index.atom" FEED_ALL_RSS = f"author/{author}/index.rss" content_type = 'author' @@ -141,7 +149,9 @@ def get(self, category=None, tag=None, author=None, ] else: query['authors_string'] = {'$regex': f'.*,{author},.*'} + elif home_template: + home_template = remove_tags_from_string(home_template) # use custom template only when categoty is blank '/' # and INDEX_TEMPLATE is defined template = home_template diff --git a/quokka/utils/atom.py b/quokka/utils/atom.py index d5e817b5c..8ef879a84 100644 --- a/quokka/utils/atom.py +++ b/quokka/utils/atom.py @@ -26,6 +26,7 @@ def atom_feed(request): from werkzeug._compat import implements_to_string, string_types # from werkzeug.utils import escape from werkzeug.wrappers import BaseResponse +from quokka.utils.text import remove_tags_from_string def escape(x): @@ -165,20 +166,27 @@ def generate(self): # noqa dates = sorted([entry.updated for entry in self.entries]) self.updated = dates and dates[-1] or datetime.utcnow() + self.title = remove_tags_from_string(self.title) + yield u'\n' yield u'\n' yield ' ' + _make_text_block('title', self.title, self.title_type) yield u' %s\n' % escape(self.id) yield u' %s\n' % format_iso8601(self.updated) + if self.url: yield u' \n' % escape(self.url) + if self.feed_url: yield u' \n' % \ escape(self.feed_url) + for link in self.links: yield u' \n' % ''.join( '%s="%s" ' % (k, escape(link[k])) for k in link) + for author in self.author: + author['name'] = remove_tags_from_string(author['name']) yield u' \n' yield u' %s\n' % escape(author['name']) if 'uri' in author: @@ -186,16 +194,20 @@ def generate(self): # noqa if 'email' in author: yield ' %s\n' % escape(author['email']) yield ' \n' + if self.subtitle: yield ' ' + _make_text_block('subtitle', self.subtitle, self.subtitle_type) if self.icon: yield u' %s\n' % escape(self.icon) + if self.logo: yield u' %s\n' % escape(self.logo) + if self.rights: yield ' ' + _make_text_block('rights', self.rights, self.rights_type) + generator_name, generator_url, generator_version = self.generator if generator_name or generator_url or generator_version: tmp = [u' %s\n' % escape(generator_name)) yield u''.join(tmp) + for entry in self.entries: for line in entry.generate(): yield u' ' + line yield u'\n' + def to_string(self): """Convert the feed into a string.""" return u''.join(self.generate()) diff --git a/quokka/utils/text.py b/quokka/utils/text.py index 1612f6666..41123a193 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 @@ -91,3 +92,20 @@ 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) + return resp.replace( + '"', '' + ).replace( + ';', '' + ).replace( + '(', '' + ).replace( + ')', '' + ) + diff --git a/tests/utils/test_text.py b/tests/utils/test_text.py new file mode 100644 index 000000000..1d7ada528 --- /dev/null +++ b/tests/utils/test_text.py @@ -0,0 +1,65 @@ +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' + From 24bd94b6502fa50b38d2062ec2e2b0f69979921b Mon Sep 17 00:00:00 2001 From: marcosptf Date: Wed, 17 Jul 2019 08:07:03 -0300 Subject: [PATCH 6/7] fixing pytests --- quokka/utils/atom.py | 1 - quokka/utils/text.py | 1 - tests/__init__.py | 0 tests/conftest.py | 1 + 4 files changed, 1 insertion(+), 2 deletions(-) create mode 100644 tests/__init__.py diff --git a/quokka/utils/atom.py b/quokka/utils/atom.py index 8ef879a84..8039bd164 100644 --- a/quokka/utils/atom.py +++ b/quokka/utils/atom.py @@ -223,7 +223,6 @@ def generate(self): # noqa yield u' ' + line yield u'\n' - def to_string(self): """Convert the feed into a string.""" return u''.join(self.generate()) diff --git a/quokka/utils/text.py b/quokka/utils/text.py index 41123a193..f0a6195cb 100644 --- a/quokka/utils/text.py +++ b/quokka/utils/text.py @@ -108,4 +108,3 @@ def remove_tags_from_string(data): ).replace( ')', '' ) - diff --git a/tests/__init__.py b/tests/__init__.py new file mode 100644 index 000000000..e69de29bb diff --git a/tests/conftest.py b/tests/conftest.py index 03d28b807..2578856e3 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -8,3 +8,4 @@ def app(): """Flask Pytest uses it""" os.chdir('quokka/project_template/') return create_app() + From c8e47e8e8f76240aa7cd56859cc2e0f232415ac5 Mon Sep 17 00:00:00 2001 From: marcosptf Date: Fri, 19 Jul 2019 07:04:52 -0300 Subject: [PATCH 7/7] removing comments unused; --- quokka/admin/actions.py | 3 --- 1 file changed, 3 deletions(-) diff --git a/quokka/admin/actions.py b/quokka/admin/actions.py index 42c4b17de..82a832d3c 100644 --- a/quokka/admin/actions.py +++ b/quokka/admin/actions.py @@ -83,9 +83,6 @@ def action_create_userprofile(self, ids): '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',