From 738a88a5aad40607890abf8e62c4e68242d641be Mon Sep 17 00:00:00 2001
From: Ross Poulton <ross@rossp.org>
Date: Mon, 19 Jan 2009 09:40:14 +0000
Subject: [PATCH] * Fix an XSS hole: No user-sourced HTML is displayed at all.
 Descriptions, resolutions and followup comments are treated as text using
 `force_escape` and `linebreaksbr` template filters. (Issue #39) * Incoming
 email also handled slightly differently: If an email has both HTML and
 Plain-text parts, the plain text is used in the ticket description and/or
 followup comment fields. The HTML portion is attached as
 'email_html_body.html' so it can be viewed by the user. If an HTML-only email
 is received, the body is entered as "View attachment for body". (Issue #39)

---
 management/commands/get_email.py           | 20 ++++++++++++++++++--
 templates/helpdesk/create_ticket.html      |  9 ---------
 templates/helpdesk/public_homepage.html    |  9 ---------
 templates/helpdesk/public_view_ticket.html | 12 ++----------
 templates/helpdesk/ticket.html             | 10 ++++------
 5 files changed, 24 insertions(+), 36 deletions(-)

diff --git a/management/commands/get_email.py b/management/commands/get_email.py
index 71aa1fdf..d7142356 100644
--- a/management/commands/get_email.py
+++ b/management/commands/get_email.py
@@ -133,6 +133,8 @@ def ticket_from_message(message, queue):
 
     sender_email = parseaddr(sender)[1]
 
+    body_plain, body_html = '', ''
+
     for ignore in IgnoreEmail.objects.filter(Q(queues=queue) | Q(queues__isnull=True)):
         if ignore.test(sender_email):
             return False
@@ -154,8 +156,10 @@ def ticket_from_message(message, queue):
         name = part.get_param("name")
 
         if part.get_content_maintype() == 'text' and name == None:
-            body = part.get_payload(decode=True)
-            body = decodeUnknown(part.get_charset(), body)
+            if part.get_content_subtype() == 'plain':
+                body_plain = decodeUnknown(part.get_charset(), part.get_payload(decode=True))
+            else:
+                body_html = decodeUnknown(part.get_charset(), part.get_payload(decode=True))
         else:
             if not name:
                 ext = mimetypes.guess_extension(part.get_content_type())
@@ -169,6 +173,18 @@ def ticket_from_message(message, queue):
 
         counter += 1
 
+    if body_plain:
+        body = body_plain
+    else:
+        body = _('No plain-text email body available. Please see attachment email_html_body.html.')
+
+    if body_html:
+        files.append({
+            'filename': _("email_html_body.html"),
+            'content': body_html,
+            'type': 'text/html',
+        })
+
     now = datetime.now()
 
     if ticket:
diff --git a/templates/helpdesk/create_ticket.html b/templates/helpdesk/create_ticket.html
index 167492f9..a82dfae2 100644
--- a/templates/helpdesk/create_ticket.html
+++ b/templates/helpdesk/create_ticket.html
@@ -2,15 +2,6 @@
 
 {% block helpdesk_title %}{% trans "Create Ticket" %}{% endblock %}
 
-{% block helpdesk_head %}
-<script src="{{ MEDIA_URL }}helpdesk/nicEdit.js" type="text/javascript"></script>
-<script type="text/javascript">
-    $(document).ready(function() {
-        nic = new nicEditor({buttonList: ['bold','italic','underline','strikeThrough','undo','redo','subscript','superscript','html']}).panelInstance('id_body');
-    });
-</script>
-{% endblock %}
-
 {% block helpdesk_body %}
 {% blocktrans %}<h2>Submit a Ticket</h2>
 
diff --git a/templates/helpdesk/public_homepage.html b/templates/helpdesk/public_homepage.html
index 249a3380..0f4b40cf 100644
--- a/templates/helpdesk/public_homepage.html
+++ b/templates/helpdesk/public_homepage.html
@@ -1,14 +1,5 @@
 {% extends "helpdesk/public_base.html" %}{% load i18n %}
 
-{% block helpdesk_head %}
-<script src="{{ MEDIA_URL }}helpdesk/nicEdit.js" type="text/javascript"></script>
-<script type="text/javascript">
-    $(document).ready(function() {
-        nic = new nicEditor({buttonList: ['bold','italic','underline','strikeThrough','undo','redo','subscript','superscript','html']}).panelInstance('id_body');
-    });
-    </script>
-{% endblock %}
-
 {% block helpdesk_body %}
 <h2>{% trans "View a Ticket" %}</h2>
 
diff --git a/templates/helpdesk/public_view_ticket.html b/templates/helpdesk/public_view_ticket.html
index a100070b..79719024 100644
--- a/templates/helpdesk/public_view_ticket.html
+++ b/templates/helpdesk/public_view_ticket.html
@@ -1,13 +1,5 @@
 {% extends "helpdesk/public_base.html" %}{% load i18n %}
 {% block helpdesk_title %}{% trans "View a Ticket" %}{% endblock %}
-{% block helpdesk_head %}
-<script src="{{ MEDIA_URL }}helpdesk/nicEdit.js" type="text/javascript"></script>
-<script type="text/javascript">
-    $(document).ready(function() {
-        nic = new nicEditor({buttonList: ['bold','italic','underline','strikeThrough','undo','redo','subscript','superscript','html']}).panelInstance('commentBox');
-    });
-</script>
-{% endblock %}
 
 {% block helpdesk_body %}
 
@@ -34,7 +26,7 @@
     <th colspan='2'>{% trans "Description" %}</th>
 </tr>
 <tr class='row_odd'>
-    <td colspan='2'>{{ ticket.description }}</td>
+    <td colspan='2'>{{ ticket.description|linebreaksbr }}</td>
 </tr>
 
 {% if ticket.resolution %}<tr class='row_even'>
@@ -52,7 +44,7 @@
 {% for followup in ticket.followup_set.public_followups %}
 <div class='followup'>
 <div class='title'>{{ followup.title }} <span class='byline'>{% if followup.user %}by {{ followup.user }}{% endif %} <span title='{{ followup.date|date:"r" }}'>{{ followup.date|timesince }} ago</span></span></div>
-{{ followup.comment|num_to_link }}
+{{ followup.comment|force_escape|num_to_link|linebreaksbr }}
 {% if followup.ticketchange_set.all %}<div class='changes'><ul>
 {% for change in followup.ticketchange_set.all %}
 <li>{% blocktrans %}Changed {{ change.field }} from {{ change.old_value }} to {{ change.new_value }}.{% endblocktrans %}</li>
diff --git a/templates/helpdesk/ticket.html b/templates/helpdesk/ticket.html
index d38dfc93..aab442ed 100644
--- a/templates/helpdesk/ticket.html
+++ b/templates/helpdesk/ticket.html
@@ -1,10 +1,8 @@
 {% extends "helpdesk/base.html" %}{% load i18n %}
 {% block helpdesk_title %}{% trans "View Ticket Details" %}{% endblock %}
 {% block helpdesk_head %}
-<script src="{{ MEDIA_URL }}helpdesk/nicEdit.js" type="text/javascript"></script>
 <script type="text/javascript">
     $(document).ready(function() {
-        nic = new nicEditor({buttonList: ['bold','italic','underline','strikeThrough','undo','redo','subscript','superscript','html']}).panelInstance('commentBox');
         $("#ShowFurtherEditOptions").click(function() {
             $("#FurtherEditOptions").fadeIn();
             $("#ShowFurtherOptPara").hide();
@@ -22,7 +20,7 @@
             preset = $('#id_preset').val();
             if (preset != '') {
                 $.get("{% url helpdesk_raw "preset" %}?id=" + preset, function(data) {
-                    nic.nicInstances[0].setContent(data);
+                    $("#commentBox").value(data)
                 });
             }
         });
@@ -72,14 +70,14 @@
     <th colspan='2'>{% trans "Description" %}</th>
 </tr>
 <tr class='row_even'>
-    <td colspan='2'>{{ ticket.description|safe }}</td>
+    <td colspan='2'>{{ ticket.description|force_escape|linebreaksbr }}</td>
 </tr>
 
 {% if ticket.resolution %}<tr class='row_odd'>
     <th colspan='2'>{% trans "Resolution" %}{% ifequal ticket.get_status_display "Resolved" %} <a href='?close'><img src='{{ MEDIA_URL }}helpdesk/buttons/accept.png' alt='{% trans "Accept" %}' title='{% trans "Accept and Close" %}' width='60' height='15' /></a>{% endifequal %}</th>
 </tr>
 <tr class='row_even'>
-    <td colspan='2'>{{ ticket.resolution|safe }}</td>
+    <td colspan='2'>{{ ticket.resolution|force_escape }}</td>
 </tr>{% endif %}
 
 </table>
@@ -90,7 +88,7 @@
 {% for followup in ticket.followup_set.all %}
 <div class='followup'>
 <div class='title'>{{ followup.title }} <span class='byline'>{% if followup.user %}by {{ followup.user }}{% endif %} <span title='{{ followup.date|date:"r" }}'>{{ followup.date|timesince }} ago</span>{% if not followup.public %} <span class='private'>({% trans "Private" %})</span>{% endif %}</span></div>
-{% if followup.comment %}{{ followup.comment|num_to_link|safe }}{% endif %}
+{% if followup.comment %}{{ followup.comment|force_escape|num_to_link|linebreaksbr }}{% endif %}
 {% for change in followup.ticketchange_set.all %}
 {% if forloop.first %}<div class='changes'><ul>{% endif %}
 <li>{% blocktrans with change.field as field and change.old_value as old_value and change.new_value as new_value %}Changed {{ field }} from {{ old_value }} to {{ new_value }}.{% endblocktrans %}</li>