From 0c2d6e9f93a08a01769a3572bd5d42d8dc3c3ea0 Mon Sep 17 00:00:00 2001 From: Michael Josephson <30024+mikej@users.noreply.github.com> Date: Wed, 21 Jan 2026 12:14:24 +0000 Subject: [PATCH 1/3] fix XSS vulnerability in AddressPresenter#to_html need to escape each of the individual elements of the address before joining with
tags and then marking as HTML-safe. previously was vulnerable to malicious values in the address fields (street, city etc.) --- app/presenters/address_presenter.rb | 4 +++- spec/presenters/address_presenter_spec.rb | 16 +++++++++++++--- 2 files changed, 16 insertions(+), 4 deletions(-) diff --git a/app/presenters/address_presenter.rb b/app/presenters/address_presenter.rb index 42c836783..8915e6475 100644 --- a/app/presenters/address_presenter.rb +++ b/app/presenters/address_presenter.rb @@ -6,7 +6,9 @@ def to_html .join(', ') [model.flat, model.street, city_and_postal_code, lat, lng] - .delete_if(&:empty?).join('
').html_safe + .delete_if(&:empty?) + .map { |line| ERB::Util.html_escape(line) } + .join('
').html_safe end def for_map diff --git a/spec/presenters/address_presenter_spec.rb b/spec/presenters/address_presenter_spec.rb index c6d697861..01d45ca4a 100644 --- a/spec/presenters/address_presenter_spec.rb +++ b/spec/presenters/address_presenter_spec.rb @@ -2,10 +2,20 @@ let(:address) { Fabricate.build(:address) } let(:presenter) { AddressPresenter.new(address) } - it '#to_html' do - html_address = "#{address.flat}
#{address.street}
#{address.city}, #{address.postal_code}" + describe '#to_html' do + it 'returns the address in HTML with lines separated with
tags' do + html_address = "#{address.flat}
#{address.street}
#{address.city}, #{address.postal_code}" - expect(presenter.to_html).to eq(html_address) + expect(presenter.to_html).to eq(html_address) + end + + it 'escapes HTML in address elements' do + address.street = '' + html_address = "#{address.flat}
<script>alert("XSS");</script>
" + + "#{address.city}, #{address.postal_code}" + + expect(presenter.to_html).to eq(html_address) + end end it '#to_s' do From 53bdc0edde399be533ddbcbf7cc6c3cc2c2e91b6 Mon Sep 17 00:00:00 2001 From: Michael Josephson <30024+mikej@users.noreply.github.com> Date: Wed, 21 Jan 2026 12:16:57 +0000 Subject: [PATCH 2/3] fix XSS vulnerabilities in descriptions 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. --- app/views/admin/events/show.html.haml | 2 +- app/views/admin/meetings/show.html.haml | 2 +- app/views/meeting_invitation_mailer/_agenda.html.haml | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/app/views/admin/events/show.html.haml b/app/views/admin/events/show.html.haml index 6ed0e328f..479c27243 100644 --- a/app/views/admin/events/show.html.haml +++ b/app/views/admin/events/show.html.haml @@ -92,7 +92,7 @@ .row .col - %p.lead= @event.description.html_safe + %p.lead= sanitize(@event.description) - if @event.tito_url.present? .row diff --git a/app/views/admin/meetings/show.html.haml b/app/views/admin/meetings/show.html.haml index 9d678b71a..48d66e9cb 100644 --- a/app/views/admin/meetings/show.html.haml +++ b/app/views/admin/meetings/show.html.haml @@ -51,7 +51,7 @@ .row.mt-3 .col-10 %h4 Agenda - = @meeting.description.html_safe + = sanitize(@meeting.description) - if @invitations.any? .py-4.py-lg-5.bg-light diff --git a/app/views/meeting_invitation_mailer/_agenda.html.haml b/app/views/meeting_invitation_mailer/_agenda.html.haml index b80535f1f..55fc6c9df 100644 --- a/app/views/meeting_invitation_mailer/_agenda.html.haml +++ b/app/views/meeting_invitation_mailer/_agenda.html.haml @@ -3,4 +3,4 @@ %tr %td %h4 Agenda - %p= @meeting.description.html_safe + %p= sanitize(@meeting.description) From 2e1dbe8c894fcd7849729d60c2960ca5dbf5a772 Mon Sep 17 00:00:00 2001 From: Michael Josephson <30024+mikej@users.noreply.github.com> Date: Wed, 21 Jan 2026 12:19:35 +0000 Subject: [PATCH 3/3] adjust HAML to avoid needing to use html_safe 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. --- .../meeting_invitation_mailer/_cancel_attendance.html.haml | 2 +- app/views/meeting_invitation_mailer/invite.html.haml | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/app/views/meeting_invitation_mailer/_cancel_attendance.html.haml b/app/views/meeting_invitation_mailer/_cancel_attendance.html.haml index 6677b09f4..8de8cd843 100644 --- a/app/views/meeting_invitation_mailer/_cancel_attendance.html.haml +++ b/app/views/meeting_invitation_mailer/_cancel_attendance.html.haml @@ -5,4 +5,4 @@ %h4 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. diff --git a/app/views/meeting_invitation_mailer/invite.html.haml b/app/views/meeting_invitation_mailer/invite.html.haml index c3e6b5432..366102d76 100644 --- a/app/views/meeting_invitation_mailer/invite.html.haml +++ b/app/views/meeting_invitation_mailer/invite.html.haml @@ -16,7 +16,7 @@ %p.lead We're back for another instalment of codebar Monthlies on #{humanize_date(@meeting.date_and_time, with_time: true)} at #{@meeting.venue.name}! %p - = "#{link_to 'You can RSVP here', @rsvp_url}, after logging into your codebar account.".html_safe + #{link_to 'You can RSVP here', @rsvp_url} after logging into your codebar account. = render partial: 'agenda'