Merge pull request #9351 from edx/release

Release
2015-08-17 13:17:55 -04:00
parent 9cbc8564a1 7cdfe2ea95
commit 8b704cc2ec
15 changed files with 130 additions and 30 deletions
--- a/cms/djangoapps/contentstore/tests/test_contentstore.py
+++ b/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()
--- a/cms/templates/course-create-rerun.html
+++ b/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>
--- a/cms/templates/index.html
+++ b/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">
--- a/cms/templates/library.html
+++ b/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>

--- a/common/lib/xmodule/xmodule/modulestore/tests/utils.py
+++ b/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)
--- a/lms/djangoapps/courseware/tests/test_course_survey.py
+++ b/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>')
--- a/lms/djangoapps/instructor/tests/views/test_instructor_dashboard.py
+++ b/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):
        """
--- a/lms/djangoapps/shoppingcart/tests/test_views.py
+++ b/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)
--- a/lms/djangoapps/verify_student/tests/test_views.py
+++ b/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')
--- a/lms/templates/instructor/instructor_dashboard_2/course_info.html
+++ b/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-start-date">
--- a/lms/templates/shoppingcart/receipt.html
+++ b/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">
--- a/lms/templates/shoppingcart/registration_code_receipt.html
+++ b/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>
--- a/lms/templates/shoppingcart/registration_code_redemption.html
+++ b/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>
--- a/lms/templates/survey/survey.html
+++ b/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>
--- a/lms/templates/verify_student/pay_and_verify.html
+++ b/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>