Merge pull request #378 from peopledoc/prevent-xss

Prevent XSS attacks by enabling a cleanup method

Author: brunobord

CHANGELOG.rst

@@ -6,6 +6,7 @@ master (unreleased)
 ===================
 
 - Drop support for Python 2.7 (EOL: January 1st, 2020)
+- Added an XSS prevention mechanism. See the `security documentation <https://django-formidable.readthedocs.io/en/master/>`_ for more information and details on how to setup your own sanitization process.
 - Removed ``tox.ini`` directive that skipped missing Python interpreters.
 
 Release 3.3.0 (2019-11-29)

demo/demo/security.py

@@ -0,0 +1,37 @@
+"""
+Dummy security module. Provides fake security functions
+"""
+import bleach
+
+BLEACH_VALID_TAGS = ['p', 'b', 'strong', 'em', 'i', 'u', 'strike', 'ul', 'li',
+                     'ol', 'br', 'span', 'blockquote', 'hr', 'a', 'img', 'h1',
+                     'h2', 'h3', 'h4', 'h5', 'h6', 'table', 'caption', 'th',
+                     'tr', 'td', 'tbody']
+BLEACH_VALID_ATTRS = {
+    'span': ['style', ],
+    'p': ['align', 'style'],
+    'a': ['href', 'rel', 'target', 'name'],
+    'img': ['src', 'alt', 'style'],
+    'div': ['style', ],
+}
+BLEACH_VALID_STYLES = ['color', 'cursor', 'float', 'margin', 'width',
+                       'background-color']
+
+
+def clean(obj):
+    """
+    Use tags from settings to bleach an object.
+    """
+    return bleach.clean(
+        obj,
+        BLEACH_VALID_TAGS,
+        BLEACH_VALID_ATTRS,
+        BLEACH_VALID_STYLES,
+    )
+
+
+def clean_alert(input_string):
+    """
+    Fake cleaner. Will remove "alert" in the input_string
+    """
+    return input_string.replace("alert", "")

demo/demo/settings.py

@@ -133,3 +133,6 @@
 LOCALE_PATHS = [
     os.path.join(BASE_DIR, '..', 'formidable', 'locale'),
 ]
+
+
+DJANGO_FORMIDABLE_SANITIZE_FUNCTION = "demo.security.clean"

demo/requirements-demo.pip

@@ -3,6 +3,7 @@ ipdb
 django-extensions
 freezegun
 mock
+bleach
 # added a typing module to fix https://github.com/django-extensions/django-extensions/issues/1176
 typing
 # A temporary workaround for DRF-related error:

demo/tests/__init__.py

@@ -8,7 +8,7 @@
             "slug": "textslug",
             "type_id": "text",
             "placeholder": None,
-            "help_text": None,
+            "description": None,
             "default": None,
             "accesses": [
                 {"access_id": "padawan", "level": "REQUIRED"},
@@ -28,7 +28,7 @@
         "slug": "dropdown_slug",
         "type_id": "dropdown",
         "placeholder": None,
-        "help_text": "Lesfrites c'est bon",
+        "description": "Lesfrites c'est bon",
         "default": None,
         "accesses": [
             {"access_id": "padawan", "level": "REQUIRED"},

demo/tests/test_integration_xss.py

@@ -0,0 +1,229 @@
+import copy
+
+from django.test import TestCase, override_settings
+from django.urls import reverse
+from django.conf import settings
+
+from rest_framework.test import APITestCase
+
+from formidable.models import Formidable
+from formidable.serializers import FormidableSerializer
+from formidable.security import get_clean_function
+
+from . import form_data
+
+
+XSS = """<IMG SRC=/ onerror="alert(String.fromCharCode(88,83,83))"></img>"""
+XSS_RESULT = '<img src="/">'
+
+
+class XSSLoaderTestCase(TestCase):
+
+    @override_settings()
+    def test_no_settings(self):
+        # Deleting the settings
+        del settings.DJANGO_FORMIDABLE_SANITIZE_FUNCTION
+
+        clean_func = get_clean_function()
+        assert clean_func(XSS) == XSS
+
+    @override_settings(DJANGO_FORMIDABLE_SANITIZE_FUNCTION=None)
+    def test_none_settings(self):
+        clean_func = get_clean_function()
+        assert clean_func(XSS) == XSS
+
+    @override_settings(DJANGO_FORMIDABLE_SANITIZE_FUNCTION="foo.bar")
+    def test_unimportable_settings(self):
+        clean_func = get_clean_function()
+        assert clean_func(XSS) == XSS
+
+    @override_settings(
+        DJANGO_FORMIDABLE_SANITIZE_FUNCTION="demo.security.clean_alert")
+    def test_fake_cleaner_settings(self):
+        clean_func = get_clean_function()
+        assert clean_func(XSS) != XSS, clean_func(XSS)
+
+
+class XSSViewsTestCase(APITestCase):
+
+    def test_create_label_via_view(self):
+        _form_data = copy.deepcopy(form_data)
+        _form_data['label'] = XSS
+        res = self.client.post(
+            reverse('formidable:form_create'), _form_data, format='json'
+        )
+        self.assertEquals(res.status_code, 201)
+        formidable = Formidable.objects.order_by('pk').last()
+        assert formidable.label == XSS_RESULT
+
+    def test_create_description_via_view(self):
+        _form_data = copy.deepcopy(form_data)
+        _form_data['description'] = XSS
+        res = self.client.post(
+            reverse('formidable:form_create'), _form_data, format='json'
+        )
+        self.assertEquals(res.status_code, 201)
+        formidable = Formidable.objects.order_by('pk').last()
+        assert formidable.description == XSS_RESULT
+
+    def test_create_field_label_via_view(self):
+        _form_data = copy.deepcopy(form_data)
+        _form_data['fields'][0]['label'] = XSS
+        res = self.client.post(
+            reverse('formidable:form_create'), _form_data, format='json'
+        )
+        self.assertEquals(res.status_code, 201)
+        formidable = Formidable.objects.order_by('pk').last()
+        field = formidable.fields.first()
+        assert field.label == XSS_RESULT
+
+    def test_create_field_description_via_view(self):
+        _form_data = copy.deepcopy(form_data)
+        _form_data['fields'][0]['description'] = XSS
+        res = self.client.post(
+            reverse('formidable:form_create'), _form_data, format='json'
+        )
+        self.assertEquals(res.status_code, 201)
+        formidable = Formidable.objects.order_by('pk').last()
+        field = formidable.fields.first()
+        # For historical reasons, help_text is mapped to description
+        assert field.help_text == XSS_RESULT
+
+    def test_create_field_defaults_via_view(self):
+        _form_data = copy.deepcopy(form_data)
+        _form_data['fields'][0]['defaults'] = [XSS]
+        res = self.client.post(
+            reverse('formidable:form_create'), _form_data, format='json'
+        )
+        self.assertEquals(res.status_code, 201)
+        formidable = Formidable.objects.order_by('pk').last()
+        field = formidable.fields.first()
+        default = field.defaults.first()
+        assert default.value == XSS_RESULT
+
+    def test_create_field_placeholder_via_view(self):
+        _form_data = copy.deepcopy(form_data)
+        _form_data['fields'][0]['placeholder'] = XSS
+        res = self.client.post(
+            reverse('formidable:form_create'), _form_data, format='json'
+        )
+        self.assertEquals(res.status_code, 201)
+        formidable = Formidable.objects.order_by('pk').last()
+        field = formidable.fields.first()
+        assert field.placeholder == XSS_RESULT
+
+
+class XSSSerializerTestCase(TestCase):
+    def test_create_label_via_serializer(self):
+        _form_data = copy.deepcopy(form_data)
+        _form_data['label'] = XSS
+
+        serializer = FormidableSerializer(data=_form_data)
+        serializer.is_valid()
+        serializer.save()
+        formidable = serializer.instance
+        assert formidable.label == XSS_RESULT
+
+    def test_create_description_via_serializer(self):
+        _form_data = copy.deepcopy(form_data)
+        _form_data['description'] = XSS
+
+        serializer = FormidableSerializer(data=_form_data)
+        serializer.is_valid()
+        serializer.save()
+        formidable = serializer.instance
+        assert formidable.description == XSS_RESULT
+
+    def test_create_field_label_via_serializer(self):
+        _form_data = copy.deepcopy(form_data)
+        _form_data['fields'][0]['label'] = XSS
+
+        serializer = FormidableSerializer(data=_form_data)
+        serializer.is_valid()
+        serializer.save()
+        formidable = serializer.instance
+        field = formidable.fields.first()
+        assert field.label == XSS_RESULT
+
+    def test_create_field_description_via_serializer(self):
+        _form_data = copy.deepcopy(form_data)
+        _form_data['fields'][0]['description'] = XSS
+
+        serializer = FormidableSerializer(data=_form_data)
+        serializer.is_valid()
+        serializer.save()
+        formidable = serializer.instance
+        field = formidable.fields.first()
+        # For historical reasons, help_text is mapped to description
+        assert field.help_text == XSS_RESULT
+
+    def test_create_field_defaults_via_serializer(self):
+        _form_data = copy.deepcopy(form_data)
+        _form_data['fields'][0]['defaults'] = [XSS]
+
+        serializer = FormidableSerializer(data=_form_data)
+        serializer.is_valid()
+        serializer.save()
+        formidable = serializer.instance
+        field = formidable.fields.first()
+        default = field.defaults.first()
+        assert default.value == XSS_RESULT
+
+    def test_create_field_placeholder_via_serializer(self):
+        _form_data = copy.deepcopy(form_data)
+        _form_data['fields'][0]['placeholder'] = XSS
+
+        serializer = FormidableSerializer(data=_form_data)
+        serializer.is_valid()
+        serializer.save()
+        formidable = serializer.instance
+        field = formidable.fields.first()
+        assert field.placeholder == XSS_RESULT
+
+
+class XSSInstructionFieldTestCase(APITestCase):
+    """
+    Tests for XSS on Instruction fields.
+
+    The Instruction fields are an important attack vector, because they often
+    carry HTML that has to be interpreted in the integration application.
+    """
+    def test_create_field_via_serializer(self):
+        _form_data = copy.deepcopy(form_data)
+        BASE_INSTRUCTIONS = "<p>Instructions to fill the form</p>\n"
+        _form_data['fields'][0] = {
+            "validations": [],
+            "slug": "instructions",
+            "description": BASE_INSTRUCTIONS + XSS,
+            "placeholder": None,
+            "type_id": "help_text",
+            "defaults": [],
+            "accesses": []
+        }
+
+        serializer = FormidableSerializer(data=_form_data)
+        serializer.is_valid()
+        serializer.save()
+        formidable = serializer.instance
+        field = formidable.fields.first()
+        assert field.help_text == BASE_INSTRUCTIONS + XSS_RESULT
+
+    def test_create_field_via_view(self):
+        _form_data = copy.deepcopy(form_data)
+        BASE_INSTRUCTIONS = "<p>Instructions to fill the form</p>\n"
+        _form_data['fields'] = [{
+            "validations": [],
+            "slug": "instructions",
+            "description": BASE_INSTRUCTIONS + XSS,
+            "placeholder": None,
+            "type_id": "help_text",
+            "defaults": [],
+            "accesses": []
+        }]
+        res = self.client.post(
+            reverse('formidable:form_create'), _form_data, format='json'
+        )
+        self.assertEquals(res.status_code, 201)
+        formidable = Formidable.objects.order_by('pk').last()
+        field = formidable.fields.first()
+        assert field.help_text == BASE_INSTRUCTIONS + XSS_RESULT

docs/source/index.rst

@@ -11,6 +11,7 @@ Contents:
    forms
    form-specs
    api
+   security
    callbacks
    dev
    translations

docs/source/security.rst

@@ -0,0 +1,50 @@
+==============
+Security setup
+==============
+
+As any other web application, Django Formidable might be targeted by pirates who would try to inject SQL or malicious code through Javascript or any other XSS method.
+
+How to secure your django-formidable installation
+=================================================
+
+Add the following settings: ``DJANGO_FORMIDABLE_SANITIZE_FUNCTION``. It should be a string that points at a function.
+
+.. important::
+
+    We highly recommend to use `bleach <https://pypi.org/project/bleach/>`_, with dedicated adjustments in order to make sure you're sanitizing your content in a proper way.
+
+    See `bleach documentation <https://bleach.readthedocs.io/en/latest/>`_ for creating your own parameters when calling the ``clean()`` function.
+
+Example
+=======
+
+In your :file:`settings.py`, add the following:
+
+.. code-block:: python
+
+    DJANGO_FORMIDABLE_SANITIZE_FUNCTION = "path.to.module.clean_func"
+
+And then in the :file:`path/to/module.py` module, add a function that would look like this:
+
+.. code-block:: python
+
+    import bleach
+
+    def clean_func(obj):
+        """
+        Sanitize API text content
+        """
+        return bleach.clean(obj, strip=True)
+
+.. warning::
+
+    If you don't add this settings or if its value is not importable (typo, missing PYTHONPATH, etc.):
+
+    * an error log will be raised,
+    * django-formidable won't sanitize your contents for you.
+
+Secured fields
+==============
+
+* Form label & description,
+* Field label, description (help text), defaults, placeholder.

formidable/security.py

@@ -0,0 +1,28 @@
+import logging
+
+from django.conf import settings
+from django.utils.module_loading import import_string
+
+logger = logging.getLogger(__name__)
+
+
+def get_clean_function(*args, **kwargs):
+
+    def _no_clean(s):
+        return s
+
+    if not hasattr(settings, 'DJANGO_FORMIDABLE_SANITIZE_FUNCTION'):
+        return _no_clean
+    if not settings.DJANGO_FORMIDABLE_SANITIZE_FUNCTION:
+        return _no_clean
+
+    try:
+        clean_function = import_string(
+            settings.DJANGO_FORMIDABLE_SANITIZE_FUNCTION
+        )
+    except ImportError:
+        logger.error("This application has no sanitization function. "
+                     "There's a risk of XSS attack")
+        clean_function = _no_clean
+
+    return clean_function