From d55d3742110e04075075d955d2a8f24a8b2a889c Mon Sep 17 00:00:00 2001 From: FinnStutzenstein Date: Fri, 20 Jan 2017 11:34:05 +0100 Subject: [PATCH] Prevent XSS-attacks (fixes #2871) --- CHANGELOG | 1 + openslides/core/serializers.py | 5 ++++ openslides/motions/serializers.py | 14 ++++++++++ openslides/topics/serializers.py | 5 ++++ openslides/utils/validate.py | 34 +++++++++++++++++++++++ requirements_production.txt | 1 + tests/integration/motions/test_viewset.py | 2 +- tests/unit/utils/test_validate.py | 11 ++++++++ 8 files changed, 72 insertions(+), 1 deletion(-) create mode 100644 openslides/utils/validate.py create mode 100644 tests/unit/utils/test_validate.py diff --git a/CHANGELOG b/CHANGELOG index 2d971ddd8..028235ecc 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -41,6 +41,7 @@ Core: - Replaced angular-csv-import through Papa Parse for csv parsing. - Added smooth projector scroll. - Added watching permissions in client and change the view immediately on changes. +- Validate HTML strings from CKEditor against XSS attacks. Motions: - Added adjustable line numbering mode (outside, inside, none) for each diff --git a/openslides/core/serializers.py b/openslides/core/serializers.py index 19fbc4042..71f154842 100644 --- a/openslides/core/serializers.py +++ b/openslides/core/serializers.py @@ -1,4 +1,5 @@ from openslides.utils.rest_api import Field, ModelSerializer, ValidationError +from openslides.utils.validate import validate_html from .models import ( ChatMessage, @@ -78,6 +79,10 @@ class ProjectorMessageSerializer(ModelSerializer): model = ProjectorMessage fields = ('id', 'message', ) + def validate(self, data): + data['message'] = validate_html(data.get('message', '')) + return data + class CountdownSerializer(ModelSerializer): """ diff --git a/openslides/motions/serializers.py b/openslides/motions/serializers.py index 4d49c65a1..ef7b90a27 100644 --- a/openslides/motions/serializers.py +++ b/openslides/motions/serializers.py @@ -12,6 +12,7 @@ from openslides.utils.rest_api import ( SerializerMethodField, ValidationError, ) +from openslides.utils.validate import validate_html from .models import ( Category, @@ -257,6 +258,10 @@ class MotionChangeRecommendationSerializer(ModelSerializer): 'text', 'creation_time',) + def validate(self, data): + data['text'] = validate_html(data.get('text', '')) + return data + class MotionSerializer(ModelSerializer): """ @@ -305,6 +310,15 @@ class MotionSerializer(ModelSerializer): 'log_messages',) read_only_fields = ('state', 'recommendation',) # Some other fields are also read_only. See definitions above. + def validate(self, data): + data['text'] = validate_html(data.get('text', '')) + data['reason'] = validate_html(data.get('reason', '')) + validated_comments = [] + for comment in data.get('comments', []): + validated_comments.append(validate_html(comment)) + data['comments'] = validated_comments + return data + @transaction.atomic def create(self, validated_data): """ diff --git a/openslides/topics/serializers.py b/openslides/topics/serializers.py index 6ae92c1e8..bd774c679 100644 --- a/openslides/topics/serializers.py +++ b/openslides/topics/serializers.py @@ -1,4 +1,5 @@ from openslides.utils.rest_api import ModelSerializer +from openslides.utils.validate import validate_html from .models import Topic @@ -10,3 +11,7 @@ class TopicSerializer(ModelSerializer): class Meta: model = Topic fields = ('id', 'title', 'text', 'attachments', 'agenda_item_id') + + def validate(self, data): + data['text'] = validate_html(data.get('text', '')) + return data diff --git a/openslides/utils/validate.py b/openslides/utils/validate.py new file mode 100644 index 000000000..a317ab5aa --- /dev/null +++ b/openslides/utils/validate.py @@ -0,0 +1,34 @@ +import bleach + +allowed_tags = [ + 'a', 'img', # links and images + 'p', 'span', 'blockquote', # text layout + 'strike', 'strong', 'u', 'em', 'sup', 'sub', 'pre', # text formatting + 'h1', 'h2', 'h3', 'h4', 'h5', 'h6', # headings + 'ol', 'ul', 'li', # lists + 'table', 'caption', 'thead', 'tbody', 'th', 'tr', 'td', # tables +] +allowed_attributes = { + '*': ['class', 'style'], + 'img': ['alt', 'src', 'title'], + 'a': ['href', 'title'], + 'th': ['scope'], +} +allowed_styles = [ + 'color', 'background-color', 'height', 'width', 'text-align' +] + + +def validate_html(html): + """ + This method takes a string and escapes all non-whitelisted html entries. + Every field of a model that is loaded trusted in the DOM should be validated. + """ + if isinstance(html, str): + return bleach.clean( + html, + tags=allowed_tags, + attributes=allowed_attributes, + styles=allowed_styles) + else: + return html diff --git a/requirements_production.txt b/requirements_production.txt index b09c5ba5c..23e821f60 100644 --- a/requirements_production.txt +++ b/requirements_production.txt @@ -8,3 +8,4 @@ roman>=2.0,<2.1 setuptools>=18.5,<33.0 Twisted>=16.2,<16.4 Whoosh>=2.7,<2.8 +bleach>=1.5.0,<1.6 diff --git a/tests/integration/motions/test_viewset.py b/tests/integration/motions/test_viewset.py index fda76c7cf..ab3e3014e 100644 --- a/tests/integration/motions/test_viewset.py +++ b/tests/integration/motions/test_viewset.py @@ -235,7 +235,7 @@ class CreateMotion(TestCase): config['motions_comments'] = [ {'name': 'comment1', 'public': True}, {'name': 'comment2', 'public': False}] - comments = ['comemnt1_sdpoiuffo3%7dwDwW&', 'comment2_iusd&D/TdskDWH&5DWas46WAd078'] + comments = ['comemnt1_sdpoiuffo3%7dwDwW)', 'comment2_iusd_D/TdskDWH(5DWas46WAd078'] response = self.client.post( reverse('motion-list'), {'title': 'title_test_sfdAaufd56HR7sd5FDq7av', diff --git a/tests/unit/utils/test_validate.py b/tests/unit/utils/test_validate.py new file mode 100644 index 000000000..7ac25390d --- /dev/null +++ b/tests/unit/utils/test_validate.py @@ -0,0 +1,11 @@ +from unittest import TestCase + +from openslides.utils.validate import validate_html + + +class ValidatorTest(TestCase): + def test_XSS_protection(self): + data = 'tuveegi2Ho

tuveegi2Ho

Boovai7esuee4Yaiw0ei' + self.assertEqual( + validate_html(data), + 'tuveegi2Ho

tuveegi2Ho<script>kekj9(djwk</script>

Boovai7esuee4Yaiw0ei')