From 4f0f221e4665d5c506f83482740ab79ac221fe6e Mon Sep 17 00:00:00 2001 From: simon lehericey Date: Thu, 2 Feb 2023 11:04:57 +0100 Subject: [PATCH 1/3] secu: remove a balise from sane user input --- config/application.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/config/application.rb b/config/application.rb index a4ad9717a..cf437e1c9 100644 --- a/config/application.rb +++ b/config/application.rb @@ -41,7 +41,7 @@ module TPS config.assets.precompile += ['.woff'] default_allowed_tags = ActionView::Base.sanitized_allowed_tags - config.action_view.sanitized_allowed_tags = default_allowed_tags + ['u'] - ['img'] + config.action_view.sanitized_allowed_tags = default_allowed_tags + ['u'] - ['img', 'a'] # ActionDispatch's IP spoofing detection is quite limited, and often rejects # legitimate requests from misconfigured proxies (such as mobile telcos). From a1487e99237f48d6a210dd9826d285ef6403fe8c Mon Sep 17 00:00:00 2001 From: simon lehericey Date: Fri, 3 Feb 2023 15:17:12 +0100 Subject: [PATCH 2/3] fix: allow a tag in various admin text --- .../champ_label_component.html.haml | 2 +- .../editable_champ_component.html.haml | 2 +- .../explication_component.html.haml | 2 +- .../linked_drop_down_list_component.html.haml | 2 +- app/helpers/string_to_html_helper.rb | 11 +++++++++-- app/views/shared/_procedure_description.html.haml | 2 +- spec/helpers/string_to_html_helper_spec.rb | 13 +++++++++++-- 7 files changed, 25 insertions(+), 9 deletions(-) diff --git a/app/components/editable_champ/champ_label_component/champ_label_component.html.haml b/app/components/editable_champ/champ_label_component/champ_label_component.html.haml index 0786f1349..1e5f85d2b 100644 --- a/app/components/editable_champ/champ_label_component/champ_label_component.html.haml +++ b/app/components/editable_champ/champ_label_component/champ_label_component.html.haml @@ -7,4 +7,4 @@ = render EditableChamp::ChampLabelContentComponent.new champ: @champ, seen_at: @seen_at - if @champ.description.present? - .notice{ id: @champ.describedby_id }= string_to_html(@champ.description) + .notice{ id: @champ.describedby_id }= string_to_html(@champ.description, allow_a: true) diff --git a/app/components/editable_champ/editable_champ_component/editable_champ_component.html.haml b/app/components/editable_champ/editable_champ_component/editable_champ_component.html.haml index b98f43654..081642438 100644 --- a/app/components/editable_champ/editable_champ_component/editable_champ_component.html.haml +++ b/app/components/editable_champ/editable_champ_component/editable_champ_component.html.haml @@ -2,7 +2,7 @@ - if @champ.block? %h3.header-subsection= @champ.libelle - if @champ.description.present? - %p.notice= string_to_html(@champ.description, false) + %p.notice= string_to_html(@champ.description, false, allow_a: true) - elsif has_label?(@champ) = render EditableChamp::ChampLabelComponent.new form: @form, champ: @champ, seen_at: @seen_at diff --git a/app/components/editable_champ/explication_component/explication_component.html.haml b/app/components/editable_champ/explication_component/explication_component.html.haml index 9af357344..7e4822b3c 100644 --- a/app/components/editable_champ/explication_component/explication_component.html.haml +++ b/app/components/editable_champ/explication_component/explication_component.html.haml @@ -1,7 +1,7 @@ = render Dsfr::CalloutComponent.new(title: @champ.libelle, extra_class_names: ['fr-mb-2w', 'fr-callout--blue-cumulus']) do |c| - c.with_body do - = string_to_html(@champ.description) + = string_to_html(@champ.description, allow_a: true) - if @champ.collapsible_explanation_enabled? && @champ.collapsible_explanation_text.present? %div diff --git a/app/components/editable_champ/linked_drop_down_list_component/linked_drop_down_list_component.html.haml b/app/components/editable_champ/linked_drop_down_list_component/linked_drop_down_list_component.html.haml index 04d9962e1..cf451f3b0 100644 --- a/app/components/editable_champ/linked_drop_down_list_component/linked_drop_down_list_component.html.haml +++ b/app/components/editable_champ/linked_drop_down_list_component/linked_drop_down_list_component.html.haml @@ -8,7 +8,7 @@ = @form.label :secondary_value, for: "#{@champ.input_id}-secondary" do - sanitize((@champ.drop_down_secondary_libelle.presence || "Valeur secondaire dépendant de la première") + (@champ.type_de_champ.mandatory? ? tag.span(' *', class: 'mandatory') : '')) - if @champ.drop_down_secondary_description.present? - .notice{ id: "#{@champ.describedby_id}-secondary" }= string_to_html(@champ.drop_down_secondary_description) + .notice{ id: "#{@champ.describedby_id}-secondary" }= string_to_html(@champ.drop_down_secondary_description, allow_a: true) = @form.select :secondary_value, @champ.secondary_options[@champ.primary_value], {}, diff --git a/app/helpers/string_to_html_helper.rb b/app/helpers/string_to_html_helper.rb index 8fc99684f..d41c8300a 100644 --- a/app/helpers/string_to_html_helper.rb +++ b/app/helpers/string_to_html_helper.rb @@ -1,8 +1,15 @@ module StringToHtmlHelper - def string_to_html(str, wrapper_tag = 'p') + def string_to_html(str, wrapper_tag = 'p', allow_a: false) return nil if str.blank? html_formatted = simple_format(str, {}, { wrapper_tag: wrapper_tag }) with_links = Anchored::Linker.auto_link(html_formatted, target: '_blank', rel: 'noopener') - sanitize(with_links, attributes: ['target', 'rel', 'href']) + + tags = if allow_a + Rails.configuration.action_view.sanitized_allowed_tags + ['a'] + else + Rails.configuration.action_view.sanitized_allowed_tags + end + + sanitize(with_links, tags:, attributes: ['target', 'rel', 'href']) end end diff --git a/app/views/shared/_procedure_description.html.haml b/app/views/shared/_procedure_description.html.haml index 6d4fb669e..e10d4607f 100644 --- a/app/views/shared/_procedure_description.html.haml +++ b/app/views/shared/_procedure_description.html.haml @@ -23,5 +23,5 @@ .procedure-description .procedure-description-body.read-more-enabled.read-more-collapsed - = h string_to_html(procedure.description) + = h string_to_html(procedure.description, allow_a: true) = button_tag "Afficher la description complète", class: 'button read-more-button' diff --git a/spec/helpers/string_to_html_helper_spec.rb b/spec/helpers/string_to_html_helper_spec.rb index 545bc0cbb..513fdf75c 100644 --- a/spec/helpers/string_to_html_helper_spec.rb +++ b/spec/helpers/string_to_html_helper_spec.rb @@ -1,6 +1,7 @@ RSpec.describe StringToHtmlHelper, type: :helper do describe "#string_to_html" do - subject { string_to_html(description) } + let(:allow_a) { false } + subject { string_to_html(description, allow_a:) } context "with some simple texte" do let(:description) { "1er ligne \n 2ieme ligne" } @@ -11,7 +12,15 @@ RSpec.describe StringToHtmlHelper, type: :helper do context "with a link" do context "using an authorized scheme" do let(:description) { "Cliquez sur https://d-s.fr pour continuer." } - it { is_expected.to eq("

Cliquez sur https://d-s.fr pour continuer.

") } + + context 'with a tag authorized' do + let(:allow_a) { true } + it { is_expected.to eq("

Cliquez sur https://d-s.fr pour continuer.

") } + end + + context 'without a tag' do + it { is_expected.to eq("

Cliquez sur https://d-s.fr pour continuer.

") } + end end context "using a non-authorized scheme" do From 1ee67d511dc8a2d855a375cba0a0c3fe3ab93296 Mon Sep 17 00:00:00 2001 From: Martin Date: Mon, 6 Feb 2023 09:41:45 +0100 Subject: [PATCH 3/3] =?UTF-8?q?amelioration(mails):=20permet=20a=20nos=20m?= =?UTF-8?q?ails=20d'inclure=20des=20balises=20=20pour=20faciliter=20l'u?= =?UTF-8?q?sage=20des=20mail=20envoy=C3=A9s?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../administrateurs/mail_templates_controller.rb | 2 +- app/lib/sanitizers/mail_scrubber.rb | 12 ++++++++++++ app/mailers/notification_mailer.rb | 2 +- 3 files changed, 14 insertions(+), 2 deletions(-) create mode 100644 app/lib/sanitizers/mail_scrubber.rb diff --git a/app/controllers/administrateurs/mail_templates_controller.rb b/app/controllers/administrateurs/mail_templates_controller.rb index b4b1bee36..37716eec4 100644 --- a/app/controllers/administrateurs/mail_templates_controller.rb +++ b/app/controllers/administrateurs/mail_templates_controller.rb @@ -40,7 +40,7 @@ module Administrateurs @dossier = dossier @logo_url = procedure.logo_url @service = procedure.service - @rendered_template = sanitize(mail_template.body_for_dossier(dossier)) + @rendered_template = sanitize(mail_template.body_for_dossier(dossier), scrubber: Sanitizers::MailScrubber.new) @actions = mail_template.actions_for_dossier(dossier) render(template: 'notification_mailer/send_notification', layout: 'mailers/notifications_layout') diff --git a/app/lib/sanitizers/mail_scrubber.rb b/app/lib/sanitizers/mail_scrubber.rb new file mode 100644 index 000000000..89a1eeaf5 --- /dev/null +++ b/app/lib/sanitizers/mail_scrubber.rb @@ -0,0 +1,12 @@ +module Sanitizers + class MailScrubber < Rails::Html::PermitScrubber + def initialize + super + self.tags = Rails.application.config.action_view.sanitized_allowed_tags + ['a'] + end + + def skip_node?(node) + node.text? + end + end +end diff --git a/app/mailers/notification_mailer.rb b/app/mailers/notification_mailer.rb index 706da2efd..2d4e36e0e 100644 --- a/app/mailers/notification_mailer.rb +++ b/app/mailers/notification_mailer.rb @@ -20,7 +20,7 @@ class NotificationMailer < ApplicationMailer def send_notification @service = @dossier.procedure.service @logo_url = attach_logo(@dossier.procedure) - @rendered_template = sanitize(@body) + @rendered_template = sanitize(@body, scrubber: Sanitizers::MailScrubber.new) attachments[@attachment[:filename]] = @attachment[:content] if @attachment.present? I18n.with_locale(@dossier.user_locale) do