diff --git a/lms/djangoapps/instructor/tests/test_xss.py b/lms/djangoapps/instructor/tests/test_xss.py new file mode 100644 index 0000000000..d6b8adc908 --- /dev/null +++ b/lms/djangoapps/instructor/tests/test_xss.py @@ -0,0 +1,63 @@ +""" +Tests of various instructor dashboard features that include lists of students +""" + +from django.conf import settings +from django.test import TestCase +from django.test.client import RequestFactory +from django.test.utils import override_settings +from markupsafe import escape + +from courseware.tests.tests import TEST_DATA_MONGO_MODULESTORE +from student.tests.factories import UserFactory, CourseEnrollmentFactory +from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase +from xmodule.modulestore.tests.factories import CourseFactory + +from instructor import views + +@override_settings(MODULESTORE=TEST_DATA_MONGO_MODULESTORE) +class TestXss(ModuleStoreTestCase): + def setUp(self): + self._request_factory = RequestFactory() + self._course = CourseFactory.create() + self._evil_student = UserFactory.create( + email="robot+evil@edx.org", + username="evil-robot", + profile__name='Evil Robot', + ) + self._instructor = UserFactory.create( + email="robot+instructor@edx.org", + username="instructor", + is_staff=True + ) + CourseEnrollmentFactory.create( + user=self._evil_student, + course_id=self._course.id + ) + + def _test_action(self, action): + """ + Test for XSS vulnerability in the given action + + Build a request with the given action, call the instructor dashboard + view, and check that HTML code in a user's name is properly escaped. + """ + req = self._request_factory.post( + "dummy_url", + data={"action": action} + ) + req.user = self._instructor + req.session = {} + resp = views.instructor_dashboard(req, self._course.id) + respUnicode = resp.content.decode(settings.DEFAULT_CHARSET) + self.assertNotIn(self._evil_student.profile.name, respUnicode) + self.assertIn(escape(self._evil_student.profile.name), respUnicode) + + def test_list_enrolled(self): + self._test_action("List enrolled students") + + def test_dump_list_of_enrolled(self): + self._test_action("Dump list of enrolled students") + + def test_dump_grades(self): + self._test_action("Dump Grades for all students in this course") diff --git a/lms/djangoapps/instructor/views.py b/lms/djangoapps/instructor/views.py index 00b1b918b3..f221d35a1b 100644 --- a/lms/djangoapps/instructor/views.py +++ b/lms/djangoapps/instructor/views.py @@ -5,6 +5,7 @@ from collections import defaultdict import csv import json import logging +from markupsafe import escape import os import re import requests @@ -76,10 +77,6 @@ def instructor_dashboard(request, course_id): else: idash_mode = request.session.get('idash_mode', 'Grades') - def escape(s): - """escape HTML special characters in string""" - return str(s).replace('<', '<').replace('>', '>') - # assemble some course statistics for output to instructor datatable = {'header': ['Statistic', 'Value'], 'title': 'Course Statistics At A Glance', @@ -316,7 +313,7 @@ def instructor_dashboard(request, course_id): datatable = {'header': ['Student email', 'Match?']} rg_students = [x['email'] for x in rg_stud_data['retdata']] def domatch(x): - return 'yes' if x.email in rg_students else 'No' + return 'yes' if x.email in rg_students else 'No' datatable['data'] = [[x.email, domatch(x)] for x in stud_data['students']] datatable['title'] = action diff --git a/lms/templates/courseware/instructor_dashboard.html b/lms/templates/courseware/instructor_dashboard.html index 4898914d01..8f84109626 100644 --- a/lms/templates/courseware/instructor_dashboard.html +++ b/lms/templates/courseware/instructor_dashboard.html @@ -539,17 +539,17 @@ function goto( mode)


-

${datatable['title']}

+

${datatable['title'] | h}

%for hname in datatable['header']: - + %endfor %for row in datatable['data']: %for value in row: - + %endfor %endfor
${hname}${hname | h}
${value}${value | h}