From 76545ef49ce0759590ba0f4610d2b0b6da431788 Mon Sep 17 00:00:00 2001
From: cahrens <christina@edx.org>
Date: Mon, 2 May 2016 14:18:03 -0400
Subject: [PATCH] HTML-encode string values in keyword substitution.

TNL-4193
---
 lms/djangoapps/bulk_email/models.py           |  7 ++++-
 .../bulk_email/tests/test_models.py           | 29 +++++++++++++++++++
 2 files changed, 35 insertions(+), 1 deletion(-)

diff --git a/lms/djangoapps/bulk_email/models.py b/lms/djangoapps/bulk_email/models.py
index bf79d023b0..488ba9700b 100644
--- a/lms/djangoapps/bulk_email/models.py
+++ b/lms/djangoapps/bulk_email/models.py
@@ -12,6 +12,7 @@ file and check it in at the same time as your model changes. To do that,
 
 """
 import logging
+import markupsafe
 from django.conf import settings
 from django.contrib.auth.models import User
 from django.db import models
@@ -176,7 +177,7 @@ class CourseEmailTemplate(models.Model):
         which is rendered using format() with the provided `context` dict.
 
         Any keywords encoded in the form %%KEYWORD%% found in the message
-        body are subtituted with user data before the body is inserted into
+        body are substituted with user data before the body is inserted into
         the template.
 
         Output is returned as a unicode string.  It is not encoded as utf-8.
@@ -215,6 +216,10 @@ class CourseEmailTemplate(models.Model):
         Convert HTML text body (`htmltext`) into HTML email message using the
         stored HTML template and the provided `context` dict.
         """
+        # HTML-escape string values in the context (used for keyword substitution).
+        for key, value in context.iteritems():
+            if isinstance(value, basestring):
+                context[key] = markupsafe.escape(value)
         return CourseEmailTemplate._render(self.html_template, htmltext, context)
 
 
diff --git a/lms/djangoapps/bulk_email/tests/test_models.py b/lms/djangoapps/bulk_email/tests/test_models.py
index ef274558e2..f382071f68 100644
--- a/lms/djangoapps/bulk_email/tests/test_models.py
+++ b/lms/djangoapps/bulk_email/tests/test_models.py
@@ -97,6 +97,15 @@ class CourseEmailTemplateTest(TestCase):
         context['course_image_url'] = "/location/of/course/image/url"
         return context
 
+    def _add_xss_fields(self, context):
+        """ Add fields to the context for XSS testing. """
+        context['course_title'] = "<script>alert('Course Title!');</alert>"
+        context['name'] = "<script>alert('Profile Name!');</alert>"
+        # Must have user_id and course_id present in order to do keyword substitution
+        context['user_id'] = 12345
+        context['course_id'] = "course-v1:edx+100+1"
+        return context
+
     def test_get_template(self):
         # Get the default template, which has name=None
         template = CourseEmailTemplate.get_template()
@@ -134,11 +143,31 @@ class CourseEmailTemplateTest(TestCase):
         context = self._get_sample_html_context()
         template.render_htmltext("My new html text.", context)
 
+    def test_render_html_xss(self):
+        template = CourseEmailTemplate.get_template()
+        context = self._add_xss_fields(self._get_sample_html_context())
+        message = template.render_htmltext(
+            "Dear %%USER_FULLNAME%%, thanks for enrolling in %%COURSE_DISPLAY_NAME%%.", context
+        )
+        self.assertNotIn("<script>", message)
+        self.assertIn("&lt;script&gt;alert(&#39;Course Title!&#39;);&lt;/alert&gt;", message)
+        self.assertIn("&lt;script&gt;alert(&#39;Profile Name!&#39;);&lt;/alert&gt;", message)
+
     def test_render_plain(self):
         template = CourseEmailTemplate.get_template()
         context = self._get_sample_plain_context()
         template.render_plaintext("My new plain text.", context)
 
+    def test_render_plain_no_escaping(self):
+        template = CourseEmailTemplate.get_template()
+        context = self._add_xss_fields(self._get_sample_plain_context())
+        message = template.render_plaintext(
+            "Dear %%USER_FULLNAME%%, thanks for enrolling in %%COURSE_DISPLAY_NAME%%.", context
+        )
+        self.assertNotIn("&lt;script&gt;", message)
+        self.assertIn(context['course_title'], message)
+        self.assertIn(context['name'], message)
+
 
 @attr('shard_1')
 class CourseAuthorizationTest(TestCase):