[#183] Fix possible XSS by sanitizing HTML content

Author: fallen

pytition/petition/helpers.py

@@ -1,4 +1,6 @@
 import requests
+import lxml
+from lxml.html.clean import Cleaner
 from django.http import Http404, HttpResponseForbidden
 from django.conf import settings
 from django.urls import reverse
@@ -7,10 +9,20 @@
 from django.core.mail import get_connection, EmailMultiAlternatives, EmailMessage
 from django.utils.translation import ugettext as _
 from django.contrib.auth.models import User
-from .models import PytitionUser, Petition
-from .forms import UpdateInfoForm
 
 
+# Remove all javascripts from HTML code
+def sanitize_html(unsecure_html_content):
+    cleaner = Cleaner(inline_style=False, scripts=True, javascript=True,
+                      safe_attrs=lxml.html.defs.safe_attrs | set(['style']),
+                      frames=False, embedded=False,
+                      meta=True, links=True, page_structure=True)
+    try:
+        secure_html_content = lxml.html.tostring(cleaner.clean_html(lxml.html.fromstring(unsecure_html_content)), method="html")
+    except:
+        secure_html_content = b''
+    return secure_html_content.decode()
+
 # Get the client IP address, considering proxies and RP
 def get_client_ip(request):
     x_forwarded_for = request.META.get('HTTP_X_FORWARDED_FOR')
@@ -22,6 +34,7 @@ def get_client_ip(request):
 
 # Get the user of the current session
 def get_session_user(request):
+    from .models import PytitionUser
     try:
         pytitionuser = PytitionUser.objects.get(user__username=request.user.username)
     except User.DoesNotExist:
@@ -38,6 +51,7 @@ def check_user_in_orga(user, orga):
 
 # Return a 404 if a petition does not exist
 def petition_from_id(id):
+    from .models import Petition
     petition = Petition.by_id(id)
     if petition is None:
         raise Http404(_("Petition does not exist"))
@@ -114,6 +128,7 @@ def subscribe_to_newsletter(petition, email):
                          connection=connection).send(fail_silently=True)
 
 def get_update_form(user, data=None):
+    from .forms import UpdateInfoForm
     if not data:
         _data = {
             'first_name': user.first_name,

pytition/petition/models.py

@@ -10,12 +10,12 @@
 from django.contrib.auth.hashers import get_hasher
 from django.db import transaction
 from django.urls import reverse
-from django.db.models import Q
-
 
 from tinymce import models as tinymce_models
 from colorfield.fields import ColorField
 
+from .helpers import sanitize_html
+
 import html
 
 
@@ -298,11 +298,11 @@ def signature_number(self):
 
     @property
     def raw_twitter_description(self):
-        return html.unescape(mark_safe(strip_tags(self.twitter_description)))
+        return html.unescape(mark_safe(strip_tags(sanitize_html(self.twitter_description))))
 
     @property
     def raw_text(self):
-        return html.unescape(mark_safe(strip_tags(self.text)))
+        return html.unescape(mark_safe(strip_tags(sanitize_html(self.text))))
 
     def __str__(self):
         return self.title
@@ -363,7 +363,7 @@ def save(self, *args, **kwargs):
             if not self.salt:
                 hasher = get_hasher()
                 self.salt = hasher.salt().decode('utf-8')
-            super(Petition, self).save(*args, **kwargs)
+        super(Petition, self).save(*args, **kwargs)
 
 
 # --------------------------------- Signature ---------------------------------

pytition/petition/templates/petition/petition_detail.html

@@ -2,19 +2,20 @@
 {% load static %}
 {% load i18n %}
 {% load widget_tweaks %}
+{% load petition_extras %}
 
-{% block title %}{% autoescape off %}{{ petition.title }}{% endautoescape %}{% endblock title %}
+{% block title %}{{ petition.title }}{% endblock title %}
 
 <!-- Extra meta -->
 {% block extrameta %}
-<meta name="description" content="{% autoescape off %}{{ petition.title }}{% endautoescape %}"/>
+<meta name="description" content="{{ petition.title }}"/>
 
-<meta property="og:title" content="{% autoescape off %}{{ petition.title }}{% endautoescape %}"/>
+<meta property="og:title" content="{{ petition.title }}"/>
 {% if petition.twitter_description %}
 <meta property="og:description"
-content="{% autoescape off%}{{ petition.raw_twitter_description }}{% endautoescape %}"/>
+content="{{ petition.raw_twitter_description|safe }}"/>
 {% else %}
-<meta property="og:description" content="{% autoescape off %}{{ petition.raw_text }}{% endautoescape %}"/>
+<meta property="og:description" content="{{ petition.raw_text|safe }}"/>
 {% endif %}
 <meta property="og:type" content="website"/>
 <meta property="og:url" content="{{ meta.petition_url }}" />
@@ -23,24 +24,24 @@
 <meta property="og:image" content="{{ petition.twitter_image }}"/>
 {% endif %}
 
-<meta itemprop="name" content="{% autoescape off %}{{ petition.title }}{% endautoescape %}"/>
+<meta itemprop="name" content="{{ petition.title }}"/>
 {% if petition.twitter_description %}
 <meta itemprop="description"
-content="{% autoescape off %}{{ petition.raw_twitter_description }}{% endautoescape %}"/>
+content="{{ petition.raw_twitter_description|safe }}"/>
 {% else %}
-<meta itemprop="description" content="{% autoescape off %}{{ petition.raw_text }}{% endautoescape %}"/>
+<meta itemprop="description" content="{{ petition.raw_text|safe }}"/>
 {% endif %}
 <!-- FIXME: <meta itemprop="url" content=""/> //-->
 
 <meta name="twitter:card" content="summary"/>
 <meta name="twitter:creator" content="@yannsionneau"/>
-<meta name="twitter:site" content="{% autoescape off %}{{ petition.org_twitter_handle }}{% endautoescape %}"/>
-<meta name="twitter:title" content="{% autoescape off %}{{ petition.title }}{% endautoescape %}"/>
+<meta name="twitter:site" content="{{ petition.org_twitter_handle }}"/>
+<meta name="twitter:title" content="{{ petition.title }}"/>
 {% if petition.twitter_description %}
 <meta name="twitter:description"
-content="{% autoescape off %}{{ petition.raw_twitter_description }}{% endautoescape %}"/>
+content="{{ petition.raw_twitter_description|safe }}"/>
 {% else %}
-<meta name="twitter:description" content="{% autoescape off %}{{ petition.raw_text }}{% endautoescape %}"/>
+<meta name="twitter:description" content="{{ petition.raw_text|safe }}"/>
 {% endif %}
 {% if petition.twitter_image %}
 <meta name="twitter:image" content="{{ petition.twitter_image }}"/>
@@ -86,15 +87,15 @@
 
 <div class="container">
   <div class="jumbotron text-center">
-    <h1 class="jumbotron-heading">{% autoescape off%}{{ petition.title|striptags }}{% endautoescape %}</h1>
+    <h1 class="jumbotron-heading">{{ petition.title|html_sanitize|striptags|safe }}</h1>
   </div>
   <div class="petition-wrapper">
     <div class="content">
       <div class="formular" id="petition">
         <div class="form-wrapper">
           {% if petition.side_text %}
             <p class="intro" id="intro">
-              {% autoescape off%}{{ petition.side_text }}{% endautoescape %}
+                {{ petition.side_text|html_sanitize|safe }}
             </p>
           {% endif %}
           <p class="sign text-primary"><strong>Signez la pétition&nbsp;!</strong></p>
@@ -176,7 +177,7 @@ <h1 class="jumbotron-heading">{% autoescape off%}{{ petition.title|striptags }}
         </div>
       </div>
       <div class="presentation">
-        {% autoescape off%}{{ petition.text }}{% endautoescape %}
+        {{ petition.text|html_sanitize|safe }}
       </div>
     </div>
   </div>
@@ -193,10 +194,10 @@ <h1 class="jumbotron-heading">{% autoescape off%}{{ petition.title|striptags }}
 <div class="footer-wrapper bg-dark">
     <footer role="contentinfo" class="footer">
         <div class="footer-links">
-            {% autoescape off %}{{ petition.footer_links }}{% endautoescape %}
+            {{ petition.footer_links|html_sanitize|safe }}
         </div>
         <div class="footer-text">
-            {% autoescape off %}{{ petition.footer_text }}{% endautoescape %}
+            {{ petition.footer_text|html_sanitize|safe }}
         </div>
     </footer>
 </div>

pytition/petition/templatetags/petition_extras.py

@@ -1,4 +1,5 @@
 from django import template
+from petition.helpers import sanitize_html
 
 register = template.Library()
 
@@ -24,4 +25,8 @@ def bootstrap(field):
             return add_class(field, "form-check-input")
         if widget == "fileinput":
             return add_class(field, "form-control-file")
-    return field
+    return field
+
+@register.filter
+def html_sanitize(html):
+    return sanitize_html(html)

pytition/pytition/settings/base.py

@@ -190,6 +190,8 @@
     'insert_toolbar': 'forecolor backcolor',
     'fontsize_formats': '8pt 10pt 12pt 14pt 18pt 24pt 36pt',
     'entity_encoding': 'raw',
+    'relative_urls' : False,
+    'convert_urls': True,
     'setup': """function(ed) {
        ed.on('change', function(e) {
             set_mce_changed(ed);

requirements.txt

@@ -7,4 +7,5 @@ mysqlclient==1.3.13
 django-widget-tweaks==1.4.3
 beautifulsoup4~=4.6.3
 django-formtools==2.1
-bcrypt
+bcrypt
+lxml