Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions app/models/attestation_template.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down 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
2 changes: 1 addition & 1 deletion spec/mailers/notification_mailer_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
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