Merge pull request #10482 from edx/robrap/SEC-27

SEC-27: Escape json for Studio advanced settings

cms/djangoapps/contentstore/views/course.py

@@ -1148,7 +1148,7 @@ def advanced_settings_handler(request, course_key_string):
 
             return render_to_response('settings_advanced.html', {
                 'context_course': course_module,
-                'advanced_dict': json.dumps(CourseMetadata.fetch(course_module)),
+                'advanced_dict': CourseMetadata.fetch(course_module),
                 'advanced_settings_url': reverse_course_url('advanced_settings_handler', course_key)
             })
         elif 'application/json' in request.META.get('HTTP_ACCEPT', ''):

cms/templates/settings_advanced.html

@@ -4,7 +4,7 @@
 <%!
   from django.utils.translation import ugettext as _
   from contentstore import utils
-  from django.utils.html import escapejs
+  from openedx.core.lib.json_utils import escape_json_dumps
 %>
 <%block name="title">${_("Advanced Settings")}</%block>
 <%block name="bodyclass">is-signedin course advanced view-settings</%block>
@@ -19,7 +19,7 @@
 
 <%block name="requirejs">
     require(["js/factories/settings_advanced"], function(SettingsAdvancedFactory) {
-        SettingsAdvancedFactory(${advanced_dict | n}, "${advanced_settings_url}");
+        SettingsAdvancedFactory(${escape_json_dumps(advanced_dict) | n}, "${advanced_settings_url}");
     });
 </%block>
 

openedx/core/lib/json_utils.py

 """
 Utilities for dealing with JSON.
 """
+import json
 import simplejson
 
 
@@ -20,3 +21,50 @@ class EscapedEdxJSONEncoder(EdxJSONEncoder):
             simplejson.loads(super(EscapedEdxJSONEncoder, self).encode(obj)),
             cls=simplejson.JSONEncoderForHTML
         )
+
+
+def _escape_json_for_html(json_str):
+    """
+    Escape JSON that is safe to be embedded in HTML.
+
+    This implementation is based on escaping performed in simplejson.JSONEncoderForHTML.
+
+    Arguments:
+        json_str (str): The JSON string to be escaped
+
+    Returns:
+        (str) Escaped JSON that is safe to be embedded in HTML.
+
+    """
+    json_str = json_str.replace("&", "\\u0026")
+    json_str = json_str.replace(">", "\\u003e")
+    json_str = json_str.replace("<", "\\u003c")
+    return json_str
+
+
+def escape_json_dumps(obj, cls=EdxJSONEncoder):
+    """
+    JSON dumps encoded JSON that is safe to be embedded in HTML.
+
+    Usage:
+        Can be used inside a Mako template inside a <SCRIPT> as follows:
+            var my_json = ${escape_json_dumps(my_object) | n}
+
+        Use the "n" Mako filter above.  It is possible that the
+            default filter may include html encoding in the future, and
+            we must make sure to get the proper escaping.
+
+        Ensure ascii in json.dumps (ensure_ascii=True) allows safe skipping of Mako's
+            default filter decode.utf8.
+
+    Arguments:
+        obj: The json object to be encoded and dumped to a string
+        cls (class): The JSON encoder class (defaults to EdxJSONEncoder)
+
+    Returns:
+        str: Escaped encoded JSON
+
+    """
+    encoded_json = json.dumps(obj, ensure_ascii=True, cls=cls)
+    encoded_json = _escape_json_for_html(encoded_json)
+    return encoded_json

openedx/core/lib/tests/test_json_utils.py

@@ -3,16 +3,70 @@ Tests for json_utils.py
 """
 import json
 from unittest import TestCase
+from openedx.core.lib.json_utils import (
+    escape_json_dumps, EscapedEdxJSONEncoder
+)
 
-from openedx.core.lib.json_utils import EscapedEdxJSONEncoder
 
+class TestJsonUtils(TestCase):
+    """
+    Test JSON Utils
+    """
+
+    class NoDefaultEncoding(object):
+        """
+        Helper class that has no default JSON encoding
+        """
+        def __init__(self, value):
+            self.value = value
+
+    class SampleJSONEncoder(json.JSONEncoder):
+        """
+        A test encoder that is used to prove that the encoder does its job before the escaping.
+        """
+        # pylint: disable=method-hidden
+        def default(self, noDefaultEncodingObj):
+            return noDefaultEncodingObj.value.replace("<script>", "sample-encoder-was-here")
 
-class TestEscapedEdxJSONEncoder(TestCase):
-    """Test the EscapedEdxJSONEncoder class."""
     def test_escapes_forward_slashes(self):
-        """Verify that we escape forward slashes with backslashes."""
+        """
+        Verify that we escape forward slashes with backslashes.
+        """
         malicious_json = {'</script><script>alert("hello, ");</script>': '</script><script>alert("world!");</script>'}
         self.assertNotIn(
             '</script>',
             json.dumps(malicious_json, cls=EscapedEdxJSONEncoder)
         )
+
+    def test_escape_json_dumps_escapes_unsafe_html(self):
+        """
+        Test escape_json_dumps properly escapes &, <, and >.
+        """
+        malicious_json = {"</script><script>alert('hello, ');</script>": "</script><script>alert('&world!');</script>"}
+        expected_encoded_json = (
+            r'''{"\u003c/script\u003e\u003cscript\u003ealert('hello, ');\u003c/script\u003e": '''
+            r'''"\u003c/script\u003e\u003cscript\u003ealert('\u0026world!');\u003c/script\u003e"}'''
+        )
+
+        encoded_json = escape_json_dumps(malicious_json)
+        self.assertEquals(expected_encoded_json, encoded_json)
+
+    def test_escape_json_dumps_with_custom_encoder_escapes_unsafe_html(self):
+        """
+        Test escape_json_dumps first encodes with custom JSNOEncoder before escaping &, <, and >
+
+        The test encoder class should first perform the replacement of "<script>" with
+        "sample-encoder-was-here", and then should escape the remaining &, <, and >.
+
+        """
+        malicious_json = {
+            "</script><script>alert('hello, ');</script>":
+            self.NoDefaultEncoding("</script><script>alert('&world!');</script>")
+        }
+        expected_custom_encoded_json = (
+            r'''{"\u003c/script\u003e\u003cscript\u003ealert('hello, ');\u003c/script\u003e": '''
+            r'''"\u003c/script\u003esample-encoder-was-herealert('\u0026world!');\u003c/script\u003e"}'''
+        )
+
+        encoded_json = escape_json_dumps(malicious_json, cls=self.SampleJSONEncoder)
+        self.assertEquals(expected_custom_encoded_json, encoded_json)