Skip to content

Commit

Permalink
fix(tags): escape user data tags for emails
Browse files Browse the repository at this point in the history
  • Loading branch information
colinux committed Dec 15, 2023
1 parent 7ba1350 commit 3fb7ae3
Show file tree
Hide file tree
Showing 3 changed files with 41 additions and 7 deletions.
4 changes: 2 additions & 2 deletions app/models/attestation_template.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
26 changes: 21 additions & 5 deletions app/models/concerns/tags_substitution_concern.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
},
{
Expand Down Expand Up @@ -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',
Expand All @@ -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
}
]

Expand Down Expand Up @@ -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 = [
Expand Down Expand Up @@ -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
Expand Down
18 changes: 18 additions & 0 deletions spec/models/concern/tags_substitution_concern_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down

0 comments on commit 3fb7ae3

Please sign in to comment.