Merge pull request #323 from alphagov/fix-xss

Fix XSS vulnerability on search results page

Author: lfdebrux

CHANGELOG.md

@@ -2,6 +2,10 @@
 
 ## Unreleased
 
+This change solves a potential security issue with HTML snippets. Pages indexed in search results have their entire contents indexed, including any HTML code snippets. These HTML snippets would appear in the search results unsanitised, making it possible to render arbitrary HTML or run arbitrary scripts.
+
+You can see more detail about this issue at [#323: Fix XSS vulnerability on search results page](https://github.com/alphagov/tech-docs-gem/pull/323)
+
 ## 3.3.0
 
 ### New features

lib/assets/javascripts/_modules/search.js

@@ -169,8 +169,8 @@
 
     this.processContent = function processContent (content, query) {
       var output
-      content = '<div>' + content + '</div>'
-      content = $(content).mark(query)
+      var sanitizedContent = $('<div></div>').text(content).html()
+      content = $('<div>' + sanitizedContent + '</div>').mark(query)
 
       // Split content by sentence.
       var sentences = content.html().replace(/(\.+|:|!|\?|\r|\n)("*|'*|\)*|}*|]*)/gm, '|').split('|')

spec/javascripts/search-spec.js

@@ -99,5 +99,11 @@ describe('Search', function () {
       var expectedResults = ' … This is <mark data-markjs="true">test</mark> sentence one … This is <mark data-markjs="true">test</mark> sentence two … This is <mark data-markjs="true">test</mark> sentence three … This is <mark data-markjs="true">test</mark> sentence four … This is <mark data-markjs="true">test</mark> sentence five … '
       expect(processedContent).toEqual(expectedResults)
     })
+
+    it('sanitises HTML in the search results', function () {
+      processedContent = module.processContent('It will render multiple `<input>` `<script>alert("uhoh")</script>` and its accompanying suggestions and `aria-live` region.', 'multi region')
+      var expectedResults = ' … It will render <mark data-markjs="true">multi</mark>ple `&lt;input&gt;` `&lt;script&gt;alert("uhoh")&lt;/script&gt;` and its accompanying suggestions and `aria-live` <mark data-markjs="true">region</mark> … '
+      expect(processedContent).toEqual(expectedResults)
+    })
   })
 })