Merge pull request demarche-numerique#12834 from demarche-numerique/fix/xss-monavis-regex

Tech: fix regex MonAvis permettant un bypass de validation du href

Author: mfo

app/validators/mon_avis_embed_validator.rb

@@ -4,8 +4,8 @@
 class MonAvisEmbedValidator < ActiveModel::Validator
   # from time to time, they decide to change domain just for fun. if it breaks, check the new subdomain
   KNOWN_SUBDOMAIN = ['jedonnemonavis', 'monavis', 'voxusagers']
-  HREF_CHECKER = /https:\/\/#{KNOWN_SUBDOMAIN.join('|')}.numerique.gouv.fr\/Demarches\/\d+.*key=[[:alnum:]]+.*/
-  IMG_CHECKER = /https:\/\/#{KNOWN_SUBDOMAIN.join('|')}.numerique.gouv.fr\/(monavis-)?static\/bouton-blanc|bleu.png|svg/
+  HREF_CHECKER = /https:\/\/(?:#{KNOWN_SUBDOMAIN.join('|')}).numerique.gouv.fr\/Demarches\/\d+.*key=[[:alnum:]]+.*/
+  IMG_CHECKER = /https:\/\/(?:#{KNOWN_SUBDOMAIN.join('|')}).numerique.gouv.fr\/(monavis-)?static\/bouton-(?:blanc|bleu).(?:png|svg)/
 
   ALLOWED_TAGS = %w[a img].freeze
   ALLOWED_ATTRIBUTES = %w[href src alt title target rel].freeze

spec/models/procedure_spec.rb

@@ -422,7 +422,17 @@
           </a>
         MSG
         let(:procedure) { build(:procedure, monavis_embed: monavis_jedonnemonavis) }
-        it { is_expected.to eq(["Le code MonAvis contient un lien pointant vers un domaine invalide"]) }
+        it { is_expected.to include("Le code MonAvis contient un lien pointant vers un domaine invalide") }
+      end
+
+      context 'rejects a link to an arbitrary domain containing monavis as a substring (regex bypass)' do
+        malicious_embed = <<-MSG
+          <a href="https://evil.com/phishing?ref=monavis&nd_source=button&key=abc123">
+            <img src="https://monavis.numerique.gouv.fr/monavis-static/bouton-bleu.png" alt="avis" />
+          </a>
+        MSG
+        let(:procedure) { build(:procedure, monavis_embed: malicious_embed) }
+        it { is_expected.to be_present }
       end
 
       context 'when YWH-PGM5381-46 pentester won' do