XSS fixes for AddressPresenter and meeting/event descriptions #2441
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
need to escape each of the individual elements of the address before joining with <br /> tags and then marking as HTML-safe. previously was vulnerable to malicious values in the address fields (street, city etc.)
fix XSS vulnerabilities in meeting/event descriptions replace `html_safe` with `sanitize` so HTML is still allowed in descriptions but is limited to safe HTML.
by separating out the link_to rather than using string interpolation on the whole paragraph we can avoid the need to use html_safe to preserve the link tags. the idea here is that by trying to limit use of `html_safe` to only where it's strictly necessary it makes it easier to keep on top of where it's being used and to spot other vulnerabilities in the future.
olleolleolle
approved these changes
Jan 21, 2026
| Can't make it anymore? | ||
| %p | ||
| = "Please #{link_to 'cancel your attendance', @cancellation_url} by following the instructions on the event page.".html_safe | ||
| Please #{link_to 'cancel your attendance', @cancellation_url} by following the instructions on the event page. |
Collaborator
There was a problem hiding this comment.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Prompted by #2439 I've spotted a couple of other XSS vulnerabilities. This PR fixes some of them, in particular:
AddressPresenterwhere e.g. address elements such as city etc. could contain<script>tagssanitizerather than#html_safe)Where there are existing tests in place I've updated these to cover the changes, but if no test coverage but it's just a straightforward replacement of
#html_safewithsanitizeI thought it was worth getting the fixes in place now as part of this.