Merge pull request #5098 from edx/adam/forums-script-escape

escape html for inline discussions (TNL-182)
openedx · Sep 5, 2014 · 2bf9404 · 2bf9404
2 parents 08f851a + 58553a7
commit 2bf9404
Show file tree

Hide file tree

Showing 2 changed files with 28 additions and 2 deletions.
diff --git a/common/static/coffee/spec/discussion/view/discussion_thread_view_spec.coffee b/common/static/coffee/spec/discussion/view/discussion_thread_view_spec.coffee
@@ -124,6 +124,32 @@ describe "DiscussionThreadView", ->
                 expect($(".post-body").text()).toEqual(expectedAbbreviation)
                 expect(DiscussionThreadShowView.prototype.convertMath).toHaveBeenCalled()
 
+            it "strips script tags appropriately", ->
+                DiscussionViewSpecHelper.setNextResponseContent({resp_total: 0, children: []})
+                longMaliciousBody = new Array(100).join("<script>alert('Until they think warm days will never cease');</script>\n")
+                @thread.set("body", longMaliciousBody)
+                maliciousAbbreviation = DiscussionUtil.abbreviateString(@thread.get('body'), 140)
+
+                # The nodes' html should be different than the strings, but
+                # their texts should be the same, indicating that they've been
+                # properly escaped. To be safe, make sure the string "<script"
+                # isn't present, either
+
+                @view.render()
+                expect($(".post-body").html()).not.toEqual(maliciousAbbreviation)
+                expect($(".post-body").text()).toEqual(maliciousAbbreviation)
+                expect($(".post-body").html()).not.toContain("<script")
+
+                @view.expand()
+                expect($(".post-body").html()).not.toEqual(longMaliciousBody)
+                expect($(".post-body").text()).toEqual(longMaliciousBody)
+                expect($(".post-body").html()).not.toContain("<script")
+
+                @view.collapse()
+                expect($(".post-body").html()).not.toEqual(maliciousAbbreviation)
+                expect($(".post-body").text()).toEqual(maliciousAbbreviation)
+                expect($(".post-body").html()).not.toContain("<script")
+
     describe "for question threads", ->
         beforeEach ->
             @thread.set("thread_type", "question")

diff --git a/common/static/coffee/src/discussion/views/discussion_thread_view.coffee b/common/static/coffee/src/discussion/views/discussion_thread_view.coffee
@@ -62,7 +62,7 @@ if Backbone?
       if event
         event.preventDefault()
       @$el.addClass("expanded")
-      @$el.find(".post-body").html(@model.get("body"))
+      @$el.find(".post-body").text(@model.get("body"))
       @showView.convertMath()
       @$el.find(".forum-thread-expand").hide()
       @$el.find(".forum-thread-collapse").show()
@@ -74,7 +74,7 @@ if Backbone?
       if event
         event.preventDefault()
       @$el.removeClass("expanded")
-      @$el.find(".post-body").html(@getAbbreviatedBody())
+      @$el.find(".post-body").text(@getAbbreviatedBody())
       @showView.convertMath()
       @$el.find(".forum-thread-expand").show()
       @$el.find(".forum-thread-collapse").hide()