From fa37c6c62bcee7b062fa491d9316a6f5d8d9b0fb Mon Sep 17 00:00:00 2001
From: Colin Darie <colin@darie.eu>
Date: Fri, 15 Dec 2023 12:40:50 +0100
Subject: [PATCH] fix(tags): escape user data tags for emails

---
 app/models/attestation_template.rb            |  6 ++---
 .../concerns/tags_substitution_concern.rb     | 26 +++++++++++++++----
 spec/mailers/notification_mailer_spec.rb      |  2 +-
 .../concern/tags_substitution_concern_spec.rb | 18 +++++++++++++
 4 files changed, 43 insertions(+), 9 deletions(-)

diff --git a/app/models/attestation_template.rb b/app/models/attestation_template.rb
index b81b5c43781..056587eedf5 100644
--- a/app/models/attestation_template.rb
+++ b/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 @@ def render_attributes_for(params = {})
 
     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
diff --git a/app/models/concerns/tags_substitution_concern.rb b/app/models/concerns/tags_substitution_concern.rb
index 8ded0c3e9a8..bf827a77003 100644
--- a/app/models/concerns/tags_substitution_concern.rb
+++ b/app/models/concerns/tags_substitution_concern.rb
@@ -66,6 +66,7 @@ def self.normalize(str)
       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 @@ def self.normalize(str)
       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 @@ def self.normalize(str)
           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 @@ def types_de_champ_tags(types_de_champ, available_for_states)
     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 @@ def replace_tags(text, dossier)
   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
diff --git a/spec/mailers/notification_mailer_spec.rb b/spec/mailers/notification_mailer_spec.rb
index 44bb32a4266..d0b01158144 100644
--- a/spec/mailers/notification_mailer_spec.rb
+++ b/spec/mailers/notification_mailer_spec.rb
@@ -130,7 +130,7 @@
 
     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&#39;apostrophe") }
     end
   end
 end
diff --git a/spec/models/concern/tags_substitution_concern_spec.rb b/spec/models/concern/tags_substitution_concern_spec.rb
index 405d743f496..929982b0bfd 100644
--- a/spec/models/concern/tags_substitution_concern_spec.rb
+++ b/spec/models/concern/tags_substitution_concern_spec.rb
@@ -411,6 +411,24 @@ def procedure
         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