diff --git a/CHANGELOG.md b/CHANGELOG.md index c95f38e5..9100a937 100644 --- a/CHANGELOG.md +++ b/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 diff --git a/lib/assets/javascripts/_modules/search.js b/lib/assets/javascripts/_modules/search.js index 0fa20ecf..03564058 100644 --- a/lib/assets/javascripts/_modules/search.js +++ b/lib/assets/javascripts/_modules/search.js @@ -169,8 +169,8 @@ this.processContent = function processContent (content, query) { var output - content = '
' + content + '
' - content = $(content).mark(query) + var sanitizedContent = $('
').text(content).html() + content = $('
' + sanitizedContent + '
').mark(query) // Split content by sentence. var sentences = content.html().replace(/(\.+|:|!|\?|\r|\n)("*|'*|\)*|}*|]*)/gm, '|').split('|') diff --git a/spec/javascripts/search-spec.js b/spec/javascripts/search-spec.js index dbebe14a..3792fa20 100644 --- a/spec/javascripts/search-spec.js +++ b/spec/javascripts/search-spec.js @@ -99,5 +99,11 @@ describe('Search', function () { var expectedResults = ' … This is test sentence one … This is test sentence two … This is test sentence three … This is test sentence four … This is test sentence five … ' expect(processedContent).toEqual(expectedResults) }) + + it('sanitises HTML in the search results', function () { + processedContent = module.processContent('It will render multiple `` `` and its accompanying suggestions and `aria-live` region.', 'multi region') + var expectedResults = ' … It will render multiple `<input>` `<script>alert("uhoh")</script>` and its accompanying suggestions and `aria-live` region … ' + expect(processedContent).toEqual(expectedResults) + }) }) })