Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

🤝 Sync alphagov/main into this repository #5

Merged
merged 21 commits into from
Dec 8, 2023
Merged

🤝 Sync alphagov/main into this repository #5

merged 21 commits into from
Dec 8, 2023

Conversation

Gary-H9
Copy link

@Gary-H9 Gary-H9 commented Dec 8, 2023

We're 20 commits behind, this brings this repository up to date and also includes the changes which were made in this repository.

ChrisBAshton and others added 21 commits April 11, 2023 09:56
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:

> ![script being invoked](https://user-images.githubusercontent.com/5111927/230888935-0367b598-eda7-4f67-afb5-799b41684ee3.png)
> ![HTML being rendered](https://user-images.githubusercontent.com/5111927/230888939-f0056edc-6955-4f10-8aee-c93414b1cb69.png)

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:

> ![sanitised HTML with highlights](https://user-images.githubusercontent.com/5111927/230888944-9aaf4920-cddd-43f9-8ef5-17f15785af73.png)

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 = {
  '&': '&amp;',
  '<': '&lt;',
  '>': '&gt;',
  '"': '&quot;',
  "'": '&alphagov#39;',
  '/': '&#x2F;',
  '`': '&#x60;',
  '=': '&#x3D;'
};
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-L75
Fix XSS vulnerability on search results page
Use latest long term support (LTS) version of Node.js for tests and
publishing. Also use the new version 2 lockfile format for current
versions of npm.
Use current latest stable release of Ruby for tests.

Ruby 2.7 reached end of life on 2023-03-31. We still test on the older
version for completeness, as Middleman v4 plans to keep support for Ruby
2.7 indefinitely [[1]].

[1]: middleman/middleman#2614 (comment)
Ran `bundle exec rubocop -a` then fixed the rest by hand.
middleman-syntax does not support Haml 6 [[1]]. However, it does not
place any restriction on the version of Haml used, so bundler will
try and install the latest version of Haml. This commit adds a
restriction on the version of Haml in our gemspec. This is a bit of a
hack, as we don't actually require Haml ourselves, but it will make our
tests pass and provide a better experience for our users until
middleman-syntax is fixed.

[1]: middleman/middleman-syntax#80
GitHub Actions is warning about deprecation of Node 12 actions, bump
actions/checkout to fix this.
GitHub Actions has deprecated the use of the `set-output` command [[1]],
we should use environment files instead.

[1]: https://github.blog/changelog/2022-10-11-github-actions-deprecating-save-state-and-set-output-commands/
The ruby/setup-ruby action needs a version of Ruby specified to work.
Previously it was reading the `.ruby-version` file, however we removed
that in commit 2aa58cc. We could restore that file, but because
`ruby-install` doesn't support loose version specifiers, we'd have to
keep it maintained. Instead we just tell the action to install the
latest version of Ruby 3.
…-when-generating-relative-links

TRG-668: Support sites deployed on a path other than "/" when generating header and footer links.
Copy link
Member

@jacobwoffenden jacobwoffenden left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@Gary-H9 Gary-H9 merged commit 18bc97a into main Dec 8, 2023
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants