Commit 977cc27
committed
Fix XSS vulnerability on search results page
Pages that are indexed in search results have their entire
contents indexed, including any HTML code snippets. These HTML
snippets would appear in the search results unsanitised, so it
was possible to render arbitrary HTML or run arbitrary scripts:
> 
> 
This is a largely theoretical security issue; to exploit it, an
attacker would need to find a way of committing malicious code
to a page indexed by a site that uses tech-docs-gem (which are
typically not editable by untrusted users). Their code would
also be limited by the relatively short length that's rendered
in the corresponding search result. Nevertheless, the XSS would
then be triggerable by visiting a pre-constructed URL
(`/search/index.html?q=some+search+term`), which users could be
tricked into clicking on through social engineering.
This commit sanitises the HTML before rendering it to the page.
It does so whilst retaining the `<mark data-markjs="true">`
behaviour that highlights the search term in the result:
> 
I've used jQuery's `text()` function for sanitisation, as that is
the approach used elsewhere in the project ([1]).
I did consider using native JavaScript (using the same approach as
in Mustache [2]) to avoid the jQuery dependency, but this itself may
contain bugs and would lead to having two sanitisation approaches to
maintain, so I opted against it. For future reference, the code in
this commit can be swapped out with:
```js
var entityMap = {
'&': '&',
'<': '<',
'>': '>',
'"': '"',
"'": '&alphagov#39;',
'/': '/',
'`': '`',
'=': '='
};
var sanitizedContent = String(content).replace(/[&<>"'`=\/]/g, function (s) {
return entityMap[s];
});
```
[1]: https://github.com/alphagov/tech-docs-gem/blob/66cc7ab0a06dc2f1fe89de8cba2270fcf46f6466/lib/assets/javascripts/_modules/search.js#L202-L204
[2]: https://github.com/janl/mustache.js/blob/972fd2b27a036888acfcb60d6119317744fac7ee/mustache.js#L60-L751 parent 7c29396 commit 977cc27
File tree
2 files changed
+8
-2
lines changed- lib/assets/javascripts/_modules
- spec/javascripts
2 files changed
+8
-2
lines changed| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
169 | 169 | | |
170 | 170 | | |
171 | 171 | | |
172 | | - | |
173 | | - | |
| 172 | + | |
| 173 | + | |
174 | 174 | | |
175 | 175 | | |
176 | 176 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
99 | 99 | | |
100 | 100 | | |
101 | 101 | | |
| 102 | + | |
| 103 | + | |
| 104 | + | |
| 105 | + | |
| 106 | + | |
| 107 | + | |
102 | 108 | | |
103 | 109 | | |
0 commit comments