bug 1272791 - Encode attachment data for JS

Data about attachments are assigned to a JavaScript variable so that
they are available to CKEditor. The JSON encoder does not escape
characters for this context, allowing user-entered data to be injected
into the page in an unsafe way, allowing XSS attacks. This code switches
to explicit escaping of strings using escapejs, removing the XSS vector.

r=jgmize

kuma/attachments/jinja2/attachments/includes/attachment_list.html

@@ -23,7 +23,16 @@ <h3><i aria-hidden="true" class="icon-paperclip"></i>{{ _('Attachments') }} <a h
   </p>
 
   <script>
-    mdn.wiki.attachments = {{ attachments_payload(document_attachments)|jsonencode }};
+    mdn.wiki.attachments = [
+        {% for attachment in document_attachments -%}
+        {
+          "title": "{{ attachment.file.title | escapejs }}",
+          "description": "{{ attachment.file.current_revision.description | escapejs }}",
+          "mime": "{{ attachment.file.current_revision.mime_type | escapejs }}",
+          "url": "{{ attachment.file.get_file_url() | escapejs }}"
+        }{% if not loop.last %},{% endif %}
+        {%- endfor %}
+    ];
     mdn.wiki.attachments_enabled = {{ show_attach_button|jsonencode }};
   </script>
 

kuma/attachments/templatetags/jinja_helpers.py

@@ -1,7 +1,6 @@
 from django_jinja import library
 
-from ..utils import allow_add_attachment_by, attachments_payload
+from ..utils import allow_add_attachment_by
 
 
 library.global_function(allow_add_attachment_by)
-library.global_function(attachments_payload)

kuma/attachments/tests/test_templates.py

@@ -59,3 +59,6 @@ def test_xss_file_attachment_title(self):
             '><img src=x onerror=prompt(navigator.userAgent);>',
             doc('.page-attachments-table .attachment-name-cell').html()
         )
+        # security bug 1272791
+        for script in doc('script'):
+            self.assertNotIn(title, script.text_content())

kuma/attachments/utils.py

@@ -72,16 +72,3 @@ def attachment_upload_to(instance, filename):
         'md5': hashlib.md5(str(now)).hexdigest(),
         'filename': filename
     }
-
-
-def attachments_payload(document_attachments):
-    """
-    Given a list of document attachments make some output that can be used
-    by the CKeditor plugins.
-    """
-    return [{
-        'title': attachment.file.title,
-        'description': attachment.file.current_revision.description,
-        'mime': attachment.file.current_revision.mime_type,
-        'url': attachment.file.get_file_url()
-    } for attachment in document_attachments]

kuma/core/templatetags/jinja_helpers.py

@@ -27,6 +27,7 @@
 
 
 # Yanking filters from Django.
+library.filter(defaultfilters.escapejs)
 library.filter(defaultfilters.linebreaksbr)
 library.filter(strip_tags)
 library.filter(defaultfilters.truncatewords)