Merge pull request #9350 from edx/jibshet/escape-studio-courses

HTML-escape uses of course display name.

cms/djangoapps/contentstore/tests/test_contentstore.py

@@ -36,7 +36,8 @@ from xmodule.modulestore.exceptions import ItemNotFoundError
 from xmodule.modulestore.inheritance import own_metadata
 from opaque_keys.edx.keys import UsageKey, CourseKey
 from opaque_keys.edx.locations import AssetLocation, CourseLocator
-from xmodule.modulestore.tests.factories import CourseFactory, ItemFactory, check_mongo_calls
+from xmodule.modulestore.tests.factories import CourseFactory, ItemFactory, LibraryFactory, check_mongo_calls
+from xmodule.modulestore.tests.utils import XssTestMixin
 from xmodule.modulestore.xml_exporter import export_course_to_xml
 from xmodule.modulestore.xml_importer import import_course_from_xml, perform_xlint
 
@@ -1115,7 +1116,7 @@ class MiscCourseTests(ContentStoreTestCase):
 
 
 @ddt.ddt
-class ContentStoreTest(ContentStoreTestCase):
+class ContentStoreTest(ContentStoreTestCase, XssTestMixin):
     """
     Tests for the CMS ContentStore application.
     """
@@ -1405,6 +1406,22 @@ class ContentStoreTest(ContentStoreTestCase):
             html=True
         )
 
+    def test_course_index_view_xss(self):
+        """Test that the index page correctly escapes course names with script
+        tags."""
+        CourseFactory.create(
+            display_name='<script>alert("course XSS")</script>'
+        )
+
+        LibraryFactory.create(display_name='<script>alert("library XSS")</script>')
+
+        resp = self.client.get_html('/home/')
+        for xss in ('course', 'library'):
+            html = '<script>alert("{name} XSS")</script>'.format(
+                name=xss
+            )
+            self.assert_xss(resp, html)
+
     def test_course_overview_view_with_course(self):
         """Test viewing the course overview page with an existing course"""
         course = CourseFactory.create()

cms/templates/course-create-rerun.html

@@ -40,7 +40,7 @@ from django.template.defaultfilters import escapejs
       <h2 class="page-header-super course-original">
         <span class="sr">${_("You are creating a re-run from:")}</span>
         <span class="course-original-title-id">${source_course_key.org | h} ${source_course_key.course | h} ${source_course_key.run | h}</span>
-        <span class="course-original-title">${display_name}</span>
+        <span class="course-original-title">${display_name | h}</span>
       </h2>
     </header>
   </div>
@@ -73,7 +73,7 @@ from django.template.defaultfilters import escapejs
                   <ol class="list-input">
                     <li class="field text required" id="field-course-name">
                       <label for="rerun-course-name">${_("Course Name")}</label>
-                      <input class="rerun-course-name" id="rerun-course-name" type="text" name="rerun-course-name" aria-required="true" value="${display_name}" placeholder="${_('e.g. Introduction to Computer Science')}" />
+                      <input class="rerun-course-name" id="rerun-course-name" type="text" name="rerun-course-name" aria-required="true" value="${display_name | h}" placeholder="${_('e.g. Introduction to Computer Science')}" />
                       <span class="tip">
                         ${_("The public display name for the new course. (This name is often the same as the original course name.)")}
                       </span>

cms/templates/index.html

@@ -168,7 +168,7 @@
             <li class="wrapper-course has-status" data-course-key="${course_info['course_key'] | h}">
               <div class="course-item course-rerun is-processing">
                 <div class="course-details" href="#">
-                  <h3 class="course-title">${course_info['display_name']}</h3>
+                  <h3 class="course-title">${course_info['display_name'] | h}</h3>
 
                   <div class="course-metadata">
                     <span class="course-org metadata-item">
@@ -210,7 +210,7 @@
             <li class="wrapper-course has-status" data-course-key="${course_info['course_key'] | h}">
               <div class="course-item course-rerun has-error">
                 <div class="course-details" href="#">
-                  <h3 class="course-title">${course_info['display_name']}</h3>
+                  <h3 class="course-title">${course_info['display_name'] | h}</h3>
 
                   <div class="course-metadata">
                     <span class="course-org metadata-item">
@@ -267,7 +267,7 @@
           %for course_info in sorted(courses, key=lambda s: s['display_name'].lower() if s['display_name'] is not None else ''):
           <li class="course-item" data-course-key="${course_info['course_key'] | h}">
             <a class="course-link" href="${course_info['url']}">
-              <h3 class="course-title">${course_info['display_name']}</h3>
+              <h3 class="course-title">${course_info['display_name'] | h}</h3>
 
               <div class="course-metadata">
                 <span class="course-org metadata-item">
@@ -419,7 +419,7 @@
           %for library_info in sorted(libraries, key=lambda s: s['display_name'].lower() if s['display_name'] is not None else ''):
           <li class="course-item">
             <a class="library-link" href="${library_info['url']}">
-              <h3 class="course-title">${library_info['display_name']}</h3>
+              <h3 class="course-title">${library_info['display_name'] | h}</h3>
 
               <div class="course-metadata">
                 <span class="course-org metadata-item">

cms/templates/library.html

@@ -46,7 +46,7 @@ from django.utils.translation import ugettext as _
             <small class="subtitle">${_("Content Library")}</small>
             <div class="wrapper-xblock-field incontext-editor is-editable"
                  data-field="display_name" data-field-display-name="${_("Display Name")}">
-                <h1 class="page-header-title xblock-field-value incontext-editor-value"><span class="title-value">${context_library.display_name_with_default | h}</span></h1>
+                <h1 class="page-header-title xblock-field-value incontext-editor-value"><span class="title-value">${context_library.display_name_with_default}</span></h1>
             </div>
         </div>
 

common/lib/xmodule/xmodule/modulestore/tests/utils.py

@@ -2,6 +2,7 @@
 Helper classes and methods for running modulestore tests without Django.
 """
 from importlib import import_module
+from markupsafe import escape
 from opaque_keys.edx.keys import UsageKey
 from unittest import TestCase
 from xblock.fields import XBlockMixin
@@ -174,3 +175,25 @@ class ProceduralCourseTestMixin(object):
 
         with self.store.bulk_operations(self.course.id, emit_signals=emit_signals):
             descend(self.course, ['chapter', 'sequential', 'vertical', 'problem'])
+
+
+class XssTestMixin(object):
+    """
+    Mixin for testing XSS vulnerabilities.
+    """
+
+    def assert_xss(self, response, xss_content):
+        """Assert that `xss_content` is not present in the content of
+        `response`, and that its escaped version is present. Uses the
+        same `markupsafe.escape` function as Mako templates.
+
+        Args:
+          response (Response): The HTTP response
+          xss_content (str): The Javascript code to check for.
+
+        Returns:
+          None
+
+        """
+        self.assertContains(response, escape(xss_content))
+        self.assertNotContains(response, xss_content)

lms/djangoapps/courseware/tests/test_course_survey.py

@@ -11,11 +11,12 @@ from survey.models import SurveyForm
 
 from xmodule.modulestore.tests.factories import CourseFactory
 from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase
+from xmodule.modulestore.tests.utils import XssTestMixin
 from courseware.tests.helpers import LoginEnrollmentTestCase
 
 
 @attr('shard_1')
-class SurveyViewsTests(LoginEnrollmentTestCase, ModuleStoreTestCase):
+class SurveyViewsTests(LoginEnrollmentTestCase, ModuleStoreTestCase, XssTestMixin):
     """
     All tests for the views.py file
     """
@@ -39,6 +40,7 @@ class SurveyViewsTests(LoginEnrollmentTestCase, ModuleStoreTestCase):
         })
 
         self.course = CourseFactory.create(
+            display_name='<script>alert("XSS")</script>',
             course_survey_required=True,
             course_survey_name=self.test_survey_name
         )
@@ -172,3 +174,13 @@ class SurveyViewsTests(LoginEnrollmentTestCase, ModuleStoreTestCase):
             resp,
             reverse('info', kwargs={'course_id': unicode(self.course_without_survey.id)})
         )
+
+    def test_survey_xss(self):
+        """Test that course display names are correctly HTML-escaped."""
+        response = self.client.get(
+            reverse(
+                'course_survey',
+                kwargs={'course_id': unicode(self.course.id)}
+            )
+        )
+        self.assert_xss(response, '<script>alert("XSS")</script>')

lms/djangoapps/instructor/tests/views/test_instructor_dashboard.py

@@ -15,6 +15,7 @@ from courseware.tests.helpers import LoginEnrollmentTestCase
 
 from student.tests.factories import AdminFactory
 from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase
+from xmodule.modulestore.tests.utils import XssTestMixin
 from xmodule.modulestore.tests.factories import CourseFactory
 from shoppingcart.models import PaidCourseRegistration, Order, CourseRegCodeItem
 from course_modes.models import CourseMode
@@ -23,7 +24,7 @@ from student.models import CourseEnrollment
 
 
 @ddt.ddt
-class TestInstructorDashboard(ModuleStoreTestCase, LoginEnrollmentTestCase):
+class TestInstructorDashboard(ModuleStoreTestCase, LoginEnrollmentTestCase, XssTestMixin):
     """
     Tests for the instructor dashboard (not legacy).
     """
@@ -34,7 +35,8 @@ class TestInstructorDashboard(ModuleStoreTestCase, LoginEnrollmentTestCase):
         """
         super(TestInstructorDashboard, self).setUp()
         self.course = CourseFactory.create(
-            grading_policy={"GRADE_CUTOFFS": {"A": 0.75, "B": 0.63, "C": 0.57, "D": 0.5}}
+            grading_policy={"GRADE_CUTOFFS": {"A": 0.75, "B": 0.63, "C": 0.57, "D": 0.5}},
+            display_name='<script>alert("XSS")</script>'
         )
 
         self.course_mode = CourseMode(course_id=self.course.id,
@@ -87,6 +89,13 @@ class TestInstructorDashboard(ModuleStoreTestCase, LoginEnrollmentTestCase):
         response = self.client.get(self.url)
         self.assertTrue('${amount}'.format(amount=total_amount) in response.content)
 
+    def test_course_name_xss(self):
+        """Test that the instructor dashboard correctly escapes course names
+        with script tags.
+        """
+        response = self.client.get(self.url)
+        self.assert_xss(response, '<script>alert("XSS")</script>')
+
     @override_settings(PAID_COURSE_REGISTRATION_CURRENCY=['PKR', 'Rs'])
     def test_override_currency_settings_in_the_html_response(self):
         """

lms/djangoapps/shoppingcart/tests/test_views.py

@@ -26,6 +26,7 @@ import ddt
 
 from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase
 from xmodule.modulestore.tests.factories import CourseFactory
+from xmodule.modulestore.tests.utils import XssTestMixin
 from student.roles import CourseSalesAdminRole
 from util.date_utils import get_default_time_display
 from util.testing import UrlResetMixin
@@ -65,7 +66,7 @@ postpay_mock = Mock()
 
 @patch.dict('django.conf.settings.FEATURES', {'ENABLE_PAID_COURSE_REGISTRATION': True})
 @ddt.ddt
-class ShoppingCartViewsTests(ModuleStoreTestCase):
+class ShoppingCartViewsTests(ModuleStoreTestCase, XssTestMixin):
     def setUp(self):
         super(ShoppingCartViewsTests, self).setUp()
 
@@ -98,7 +99,12 @@ class ShoppingCartViewsTests(ModuleStoreTestCase):
 
         verified_course = CourseFactory.create(org='org', number='test', display_name='Test Course')
         self.verified_course_key = verified_course.id
+
+        xss_course = CourseFactory.create(org='xssorg', number='test', display_name='<script>alert("XSS")</script>')
+        self.xss_course_key = xss_course.id
+
         self.cart = Order.get_cart_for_user(self.user)
+
         self.addCleanup(patcher.stop)
 
         self.now = datetime.now(pytz.UTC)
@@ -884,6 +890,31 @@ class ShoppingCartViewsTests(ModuleStoreTestCase):
                 'course_key': unicode(self.verified_course_key)
             })
 
+    def test_show_receipt_xss(self):
+        CertificateItem.add_to_order(self.cart, self.xss_course_key, self.cost, 'honor')
+        self.cart.purchase()
+
+        self.login_user()
+        url = reverse('shoppingcart.views.show_receipt', args=[self.cart.id])
+        resp = self.client.get(url)
+        self.assert_xss(resp, '<script>alert("XSS")</script>')
+
+    @patch('shoppingcart.views.render_to_response', render_mock)
+    def test_reg_code_xss(self):
+        self.add_reg_code(self.xss_course_key)
+
+        # One courses in user shopping cart
+        self.add_course_to_user_cart(self.xss_course_key)
+        self.assertEquals(self.cart.orderitem_set.count(), 1)
+
+        post_response = self.client.post(reverse('shoppingcart.views.use_code'), {'code': self.reg_code})
+        self.assertEqual(post_response.status_code, 200)
+
+        redeem_url = reverse('register_code_redemption', args=[self.reg_code])
+        redeem_response = self.client.get(redeem_url)
+
+        self.assert_xss(redeem_response, '<script>alert("XSS")</script>')
+
     def test_show_receipt_json_multiple_items(self):
         # Two different item types
         PaidCourseRegistration.add_to_order(self.cart, self.course_key)

lms/djangoapps/verify_student/tests/test_views.py

@@ -51,6 +51,7 @@ from verify_student.models import (
 )
 from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase
 from xmodule.modulestore.tests.factories import CourseFactory, ItemFactory
+from xmodule.modulestore.tests.utils import XssTestMixin
 from xmodule.modulestore.django import modulestore
 from xmodule.modulestore import ModuleStoreEnum
 from xmodule.modulestore.tests.factories import check_mongo_calls
@@ -81,7 +82,7 @@ class StartView(TestCase):
 
 
 @ddt.ddt
-class TestPayAndVerifyView(UrlResetMixin, ModuleStoreTestCase):
+class TestPayAndVerifyView(UrlResetMixin, ModuleStoreTestCase, XssTestMixin):
     """
     Tests for the payment and verification flow views.
     """
@@ -258,6 +259,7 @@ class TestPayAndVerifyView(UrlResetMixin, ModuleStoreTestCase):
         response = self._get_page('verify_student_verify_now', course.id)
 
         self._assert_messaging(response, PayAndVerifyView.VERIFY_NOW_MSG)
+        self.assert_xss(response, '<script>alert("XSS")</script>')
 
         # Expect that *all* steps are displayed,
         # but we start after the payment step (because it's already completed).
@@ -339,6 +341,8 @@ class TestPayAndVerifyView(UrlResetMixin, ModuleStoreTestCase):
 
         self._assert_messaging(response, PayAndVerifyView.PAYMENT_CONFIRMATION_MSG)
 
+        self.assert_xss(response, '<script>alert("XSS")</script>')
+
         # Expect that *all* steps are displayed,
         # but we start at the payment confirmation step
         self._assert_steps_displayed(
@@ -371,6 +375,8 @@ class TestPayAndVerifyView(UrlResetMixin, ModuleStoreTestCase):
 
         self._assert_messaging(response, PayAndVerifyView.FIRST_TIME_VERIFY_MSG)
 
+        self.assert_xss(response, '<script>alert("XSS")</script>')
+
         # Expect that *all* steps are displayed,
         # but we start on the first verify step
         self._assert_steps_displayed(
@@ -456,6 +462,7 @@ class TestPayAndVerifyView(UrlResetMixin, ModuleStoreTestCase):
             PayAndVerifyView.WEBCAM_REQ,
         ])
         self._assert_upgrade_session_flag(True)
+        self.assert_xss(response, '<script>alert("XSS")</script>')
 
     def test_upgrade_already_verified(self):
         course = self._create_course("verified")
@@ -724,7 +731,7 @@ class TestPayAndVerifyView(UrlResetMixin, ModuleStoreTestCase):
 
     def _create_course(self, *course_modes, **kwargs):
         """Create a new course with the specified course modes. """
-        course = CourseFactory.create()
+        course = CourseFactory.create(display_name='<script>alert("XSS")</script>')
 
         if kwargs.get('course_start'):
             course.start = kwargs.get('course_start')

lms/templates/instructor/instructor_dashboard_2/course_info.html

@@ -55,7 +55,7 @@
 
     <li class="field text is-not-editable" id="field-course-display-name">
       <label for="course-display-name">${_("Course Display Name:")}</label>
-      <b>${ section_data['course_display_name'] }</b>
+      <b>${ section_data['course_display_name'] | h}</b>
     </li>
 
     <li class="field text is-not-editable" id="field-course-started">

lms/templates/shoppingcart/receipt.html

@@ -4,6 +4,7 @@ from django.utils.translation import ugettext as _
 from django.utils.translation import ungettext
 from django.core.urlresolvers import reverse
 from courseware.courses import course_image_url, get_course_about_section, get_course_by_id
+from markupsafe import escape
 from microsite_configuration import microsite
 %>
 
@@ -35,7 +36,7 @@ from microsite_configuration import microsite
             ${_(u"You have successfully been enrolled for {course_names}. "
                 u"The following receipt has been emailed to {receipient_emails}").format(
                   course_names=u"<b>{course_names}</b>".format(
-                    course_names=appended_course_names
+                    course_names=escape(appended_course_names)
                   ),
                   receipient_emails=u"<strong>{receipient_emails}</strong>".format(
                     receipient_emails=appended_recipient_emails
@@ -50,7 +51,7 @@ from microsite_configuration import microsite
           ).format(
             number=total_registration_codes,
             course_names=u"<b>{course_names}</b>".format(
-              course_names=appended_course_names
+              course_names=escape(appended_course_names)
             )
           )}
           ${_("The following receipt has been emailed to {receipient_emails}").format(
@@ -297,7 +298,7 @@ from microsite_configuration import microsite
 
             <h3 class="course-title-info" id="course-title">
               <span class="course-registration-title">${_('Registration for:')}</span>
-              <span class="course-display-name">${ course.display_name }</span>
+              <span class="course-display-name">${ course.display_name | h }</span>
             </h3>
             <p class="course-meta-info" aria-describedby="course-title">
               <span class="course-dates-title">

lms/templates/shoppingcart/registration_code_receipt.html

@@ -33,7 +33,7 @@ from courseware.courses import course_image_url, get_course_about_section
         </div>
         <div class="course-title">
           <h1>
-            ${_("{course_name}").format(course_name=course.display_name)}
+            ${_("{course_name}").format(course_name=course.display_name) | h}
               <span class="course-dates">
                 ${_("{start_date} - {end_date}").format(
                   start_date=course.start_datetime_text(),
@@ -55,7 +55,7 @@ from courseware.courses import course_image_url, get_course_about_section
               )}
             % elif redemption_success:
               ${_("You have successfully enrolled in {course_name}."
-              " This course has now been added to your dashboard.").format(course_name=course.display_name)}
+              " This course has now been added to your dashboard.").format(course_name=course.display_name) | h}
             % elif registered_for_course:
               ${_("You're already enrolled for this course."
               " Visit your {link_start}dashboard{link_end} to see the course."
@@ -72,7 +72,7 @@ from courseware.courses import course_image_url, get_course_about_section
             % else:
               ${_("You're about to activate an enrollment code for {course_name} by {site_name}. "
               "This code can only be used one time, so you should only activate this code if you're its intended"
-              " recipient.").format(course_name=course.display_name, site_name=site_name)}
+              " recipient.").format(course_name=course.display_name, site_name=site_name) | h}
             % endif
           </p>
         </div>

lms/templates/shoppingcart/registration_code_redemption.html

@@ -33,7 +33,7 @@ from courseware.courses import course_image_url, get_course_about_section
         </div>
         <div class="course-title">
           <h1>
-            ${course.display_name}
+            ${course.display_name | h}
               <span class="course-dates">
                 ${course.start_datetime_text()}
                 -
@@ -60,7 +60,7 @@ from courseware.courses import course_image_url, get_course_about_section
                 "This course has now been added to your dashboard."
               ).format(
                 course_name=course.display_name,
-              )}
+              ) | h}
             % elif registered_for_course:
               ${_(
                 "You're already enrolled for this course. "
@@ -80,7 +80,7 @@ from courseware.courses import course_image_url, get_course_about_section
               ).format(
                 course_name=course.display_name,
                 site_name=site_name,
-              )}
+              ) | h}
             % endif
           </p>
         </div>

lms/templates/survey/survey.html

@@ -24,7 +24,7 @@ from django.utils import html
         <div class="header-survey">
           <h4 class="course-info">
             <span class="course-org">${course.display_org_with_default}</span><span class="course-number"> ${course.display_number_with_default}</span>
-            <span class="course-name">${course.display_name}</span>
+            <span class="course-name">${course.display_name | h}</span>
           </h4>
           <h3 class="title">${_("Pre-Course Survey")}</h3>
         </div>

lms/templates/verify_student/pay_and_verify.html

@@ -10,13 +10,13 @@ from verify_student.views import PayAndVerifyView
 
 <%block name="pagetitle">
   % if message_key == PayAndVerifyView.UPGRADE_MSG:
-    ${_("Upgrade Your Enrollment For {course_name}.").format(course_name=course.display_name)}
+    ${_("Upgrade Your Enrollment For {course_name}.").format(course_name=course.display_name) | h}
   % elif message_key == PayAndVerifyView.PAYMENT_CONFIRMATION_MSG:
-    ${_("Receipt For {course_name}").format(course_name=course.display_name)}
+    ${_("Receipt For {course_name}").format(course_name=course.display_name) | h}
   % elif message_key in [PayAndVerifyView.VERIFY_NOW_MSG, PayAndVerifyView.VERIFY_LATER_MSG]:
-    ${_("Verify For {course_name}").format(course_name=course.display_name)}
+    ${_("Verify For {course_name}").format(course_name=course.display_name) | h}
   % else:
-    ${_("Enroll In {course_name}").format(course_name=course.display_name)}
+    ${_("Enroll In {course_name}").format(course_name=course.display_name) | h}
   % endif
 </%block>