Skip to content

Commit

Permalink
Merge pull request #323 from alphagov/fix-xss
Browse files Browse the repository at this point in the history
Fix XSS vulnerability on search results page
  • Loading branch information
lfdebrux authored Apr 11, 2023
2 parents 7c29396 + 60e6e2d commit a51c705
Show file tree
Hide file tree
Showing 3 changed files with 12 additions and 2 deletions.
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
4 changes: 2 additions & 2 deletions lib/assets/javascripts/_modules/search.js
Original file line number Diff line number Diff line change
Expand Up @@ -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('|')
Expand Down
6 changes: 6 additions & 0 deletions spec/javascripts/search-spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -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)
})
})
})

0 comments on commit a51c705

Please sign in to comment.