From 44bd65293a7d40126ee32b3a4428f0e3922ab171 Mon Sep 17 00:00:00 2001
From: Robert Raposa <rraposa@edx.org>
Date: Mon, 26 Oct 2015 17:09:20 -0400
Subject: [PATCH] Escape json for Studio advanced settings

- Resolve SEC-27 by escaping course name in advanced settings
- Add escape_json_dumps to simplify escaping json in Mako templates

SEC-27: XSS/JS Error in Advanced Settings with invalid course name
---
 cms/djangoapps/contentstore/views/course.py |  2 +-
 cms/templates/settings_advanced.html        |  4 +-
 openedx/core/lib/json_utils.py              | 48 ++++++++++++++++
 openedx/core/lib/tests/test_json_utils.py   | 64 +++++++++++++++++++--
 4 files changed, 110 insertions(+), 8 deletions(-)

diff --git a/cms/djangoapps/contentstore/views/course.py b/cms/djangoapps/contentstore/views/course.py
index 6dfe57a41d..8d3ebe08be 100644
--- a/cms/djangoapps/contentstore/views/course.py
+++ b/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', ''):
diff --git a/cms/templates/settings_advanced.html b/cms/templates/settings_advanced.html
index eb108866a3..40f347ca30 100644
--- a/cms/templates/settings_advanced.html
+++ b/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>
 
diff --git a/openedx/core/lib/json_utils.py b/openedx/core/lib/json_utils.py
index 3c89321e99..076d0c1288 100644
--- a/openedx/core/lib/json_utils.py
+++ b/openedx/core/lib/json_utils.py
@@ -1,6 +1,7 @@
 """
 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
diff --git a/openedx/core/lib/tests/test_json_utils.py b/openedx/core/lib/tests/test_json_utils.py
index 9521f58826..855aafc8a0 100644
--- a/openedx/core/lib/tests/test_json_utils.py
+++ b/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 EscapedEdxJSONEncoder
+from openedx.core.lib.json_utils import (
+    escape_json_dumps, EscapedEdxJSONEncoder
+)
 
 
-class TestEscapedEdxJSONEncoder(TestCase):
-    """Test the EscapedEdxJSONEncoder class."""
+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")
+
     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)