From 6928035cd1f04bf3871876e704dc262f8d7802a7 Mon Sep 17 00:00:00 2001 From: Ned Batchelder Date: Sat, 14 Mar 2015 17:32:13 -0400 Subject: [PATCH] Make sure slashes in JSON content don't end script tags. PLAT-462 The string "" in JSON data would end the script element we're embedding the data in. To make sure the data doesn't disrupt the script, we escape the slash as \/ . --- common/djangoapps/xmodule_modifiers.py | 5 +- common/templates/xblock_wrapper.html | 2 +- .../courseware/tests/test_module_render.py | 49 +++++++++++++++++++ 3 files changed, 52 insertions(+), 4 deletions(-) diff --git a/common/djangoapps/xmodule_modifiers.py b/common/djangoapps/xmodule_modifiers.py index 8e4a305595..f82a194e93 100644 --- a/common/djangoapps/xmodule_modifiers.py +++ b/common/djangoapps/xmodule_modifiers.py @@ -113,11 +113,10 @@ def wrap_xblock(runtime_class, block, view, frag, context, usage_id_serializer, } if hasattr(frag, 'json_init_args') and frag.json_init_args is not None: - template_context['js_init_parameters'] = json.dumps(frag.json_init_args) - template_context['js_pass_parameters'] = True + # Replace / with \/ so that "" in the data won't break things. + template_context['js_init_parameters'] = json.dumps(frag.json_init_args).replace("/", r"\/") else: template_context['js_init_parameters'] = "" - template_context['js_pass_parameters'] = False return wrap_fragment(frag, render_to_string('xblock_wrapper.html', template_context)) diff --git a/common/templates/xblock_wrapper.html b/common/templates/xblock_wrapper.html index 74574907df..41951dc61c 100644 --- a/common/templates/xblock_wrapper.html +++ b/common/templates/xblock_wrapper.html @@ -1,5 +1,5 @@
-% if js_pass_parameters: +% if js_init_parameters: diff --git a/lms/djangoapps/courseware/tests/test_module_render.py b/lms/djangoapps/courseware/tests/test_module_render.py index c4463166b9..946c428577 100644 --- a/lms/djangoapps/courseware/tests/test_module_render.py +++ b/lms/djangoapps/courseware/tests/test_module_render.py @@ -16,10 +16,12 @@ from django.contrib.auth.models import AnonymousUser from mock import MagicMock, patch, Mock from opaque_keys.edx.keys import UsageKey, CourseKey from opaque_keys.edx.locations import SlashSeparatedCourseKey +from courseware.module_render import hash_resource from xblock.field_data import FieldData from xblock.runtime import Runtime from xblock.fields import ScopeIds from xblock.core import XBlock +from xblock.fragment import Fragment from capa.tests.response_xml_factory import OptionResponseXMLFactory from courseware import module_render as render @@ -660,6 +662,53 @@ class TestHtmlModifiers(ModuleStoreTestCase): ) +class XBlockWithJsonInitData(XBlock): + """ + Pure XBlock to use in tests, with JSON init data. + """ + the_json_data = None + + def student_view(self, context=None): # pylint: disable=unused-argument + """ + A simple view that returns just enough to test. + """ + frag = Fragment(u"Hello there!") + frag.add_javascript(u'alert("Hi!");') + frag.initialize_js('ThumbsBlock', self.the_json_data) + return frag + + +@ddt.ddt +class JsonInitDataTest(ModuleStoreTestCase): + """Tests for JSON data injected into the JS init function.""" + + @ddt.data( + ({'a': 17}, '''{"a": 17}'''), + ({'xss': 'alert("XSS")'}, r'''{"xss": "<\/script>alert(\"XSS\")"}'''), + ) + @ddt.unpack + @XBlock.register_temp_plugin(XBlockWithJsonInitData, identifier='withjson') + def test_json_init_data(self, json_data, json_output): + XBlockWithJsonInitData.the_json_data = json_data + mock_user = UserFactory() + mock_request = MagicMock() + mock_request.user = mock_user + course = CourseFactory() + descriptor = ItemFactory(category='withjson', parent=course) + field_data_cache = FieldDataCache([course, descriptor], course.id, mock_user) # pylint: disable=no-member + module = render.get_module_for_descriptor( + mock_user, + mock_request, + descriptor, + field_data_cache, + course.id, # pylint: disable=no-member + ) + html = module.render(STUDENT_VIEW).content + self.assertIn(json_output, html) + # No matter what data goes in, there should only be one close-script tag. + self.assertEqual(html.count(""), 1) + + class ViewInStudioTest(ModuleStoreTestCase): """Tests for the 'View in Studio' link visiblity."""