Merge pull request #9863 from colinux/fix-message-html-injection

Tech: échappe les tags de données utilisateur dans les modèles pour email

app/models/attestation_template.rb

@@ -18,7 +18,7 @@ class AttestationTemplate < ApplicationRecord
   DOSSIER_STATE = Dossier.states.fetch(:accepte)
 
   def attestation_for(dossier)
-    attestation = Attestation.new(title: replace_tags(title, dossier))
+    attestation = Attestation.new(title: replace_tags(title, dossier, escape: false))
     attestation.pdf.attach(
       io: build_pdf(dossier),
       filename: "attestation-dossier-#{dossier.id}.pdf",
@@ -70,8 +70,8 @@ class AttestationTemplate < ApplicationRecord
 
     if dossier.present?
       attributes.merge({
-        title: replace_tags(title, dossier),
-        body: replace_tags(body, dossier),
+        title: replace_tags(title, dossier, escape: false),
+        body: replace_tags(body, dossier, escape: false),
         signature: signature_to_render(dossier.groupe_instructeur)
       })
     else

app/models/concerns/tags_substitution_concern.rb

@@ -66,6 +66,7 @@ module TagsSubstitutionConcern
       libelle: 'motivation',
       description: 'Motivation facultative associée à la décision finale d’acceptation, refus ou classement sans suite',
       lambda: -> (d) { simple_format(d.motivation) },
+      escapable: false, # sanitized by simple_format
       available_for_states: Dossier::TERMINE
     },
     {
@@ -118,14 +119,16 @@ module TagsSubstitutionConcern
       libelle: 'lien dossier',
       description: '',
       lambda: -> (d) { external_link(dossier_url(d)) },
-      available_for_states: Dossier::SOUMIS
+      available_for_states: Dossier::SOUMIS,
+      escapable: false
     },
     {
       id: 'dossier_attestation_url',
       libelle: 'lien attestation',
       description: '',
       lambda: -> (d) { external_link(attestation_dossier_url(d)) },
-      available_for_states: [Dossier.states.fetch(:accepte)]
+      available_for_states: [Dossier.states.fetch(:accepte)],
+      escapable: false
     },
     {
       id: 'dossier_motivation_url',
@@ -138,7 +141,8 @@ module TagsSubstitutionConcern
           return "[l’instructeur n’a pas joint de document supplémentaire]"
         end
       },
-      available_for_states: Dossier::TERMINE
+      available_for_states: Dossier::TERMINE,
+      escapable: false
     }
   ]
 
@@ -310,11 +314,13 @@ module TagsSubstitutionConcern
     tags
   end
 
-  def replace_tags(text, dossier)
+  def replace_tags(text, dossier, escape: true)
     if text.nil?
       return ''
     end
 
+    @escape_unsafe_tags = escape
+
     tokens = parse_tags(text)
 
     tags_and_datas = [
@@ -352,11 +358,21 @@ module TagsSubstitutionConcern
   end
 
   def replace_tag(tag, data)
-    if tag.key?(:target)
+    value = if tag.key?(:target)
       data.public_send(tag[:target])
     else
       instance_exec(data, &tag[:lambda])
     end
+
+    if escape_unsafe_tags? && tag.fetch(:escapable, true)
+      escape_once(value)
+    else
+      value
+    end
+  end
+
+  def escape_unsafe_tags?
+    @escape_unsafe_tags
   end
 
   def procedure_types_de_champ_tags

spec/mailers/notification_mailer_spec.rb

@@ -142,7 +142,7 @@ RSpec.describe NotificationMailer, type: :mailer do
 
     context "subject has a special character" do
       let(:subject) { '--libellé démarche--' }
-      it { expect(mail.subject).to eq("Mon titre avec l'apostrophe") }
+      it { expect(mail.subject).to eq("Mon titre avec l'apostrophe") }
     end
   end
 end

spec/models/concern/tags_substitution_concern_spec.rb

@@ -411,6 +411,24 @@ describe TagsSubstitutionConcern, type: :model do
         end
       end
     end
+
+    context 'when data contains malicious code' do
+      let(:template) { '--libelleA-- --nom--' }
+      context 'in individual data' do
+        let(:for_individual) { true }
+        let(:individual) { create(:individual, nom: '<a href="https://oops.com">name</a>') }
+
+        it { is_expected.to eq('--libelleA-- &lt;a href=&quot;https://oops.com&quot;&gt;name&lt;/a&gt;') }
+      end
+
+      context 'in a champ' do
+        let(:types_de_champ_public) { [{ libelle: 'libelleA' }] }
+
+        before { dossier.champs_public.first.update(value: 'hey <a href="https://oops.com">anchor</a>') }
+
+        it { is_expected.to eq('hey &lt;a href=&quot;https://oops.com&quot;&gt;anchor&lt;/a&gt; --nom--') }
+      end
+    end
   end
 
   describe 'tags' do