CVE-2021-21319: Improper Neutralization of Input During Web Page Generation ('Cross-site Scripting') (CWE-79)
value: Galette is a membership management web application geared towards non profit organizations. In versions prior to 0.9.5, malicious javascript code can be stored to be displayed later on self subscription page. The self subscription feature can be disabled as a workaround (this is the default state). Malicious javascript code can be executed (not stored) on login and retrieve password pages. This issue is patched in version 0.9.5.
CWE description: The product does not neutralize or incorrectly neutralizes user-controllable input before it is placed in output that is used as a web page that is served to other users.
CVSS v3 base score: 5.4
Published: 2021-10-25T16:15Z
Repository: https://github.com/galette/galette
Language: PHP
Files changed: galette/lib/Galette/DynamicFields/DynamicField.php, galette/lib/Galette/Entity/TranslatableTrait.php, galette/templates/default/configurer_fiche_content.tpl
Commit date: 2021-04-05 09:47:02 +0200
Commit message: Fix stored XSS on dynamic fields configuration

Diff: diff --git a/galette/lib/Galette/DynamicFields/DynamicField.php b/galette/lib/Galette/DynamicFields/DynamicField.php
index b825df3cf..cb58df00b 100644
--- a/galette/lib/Galette/DynamicFields/DynamicField.php
+++ b/galette/lib/Galette/DynamicFields/DynamicField.php
@@ -696,11 +696,11 @@ abstract class DynamicField
             $this->addTranslation($this->name);
         }
 
         try {
             $values = array(
-                'field_name'        => $this->name,
+                'field_name'        => strip_tags($this->name),
                 'field_perm'        => $this->perm,
                 'field_required'    => $this->required,
                 'field_width'       => ($this->width === null ? new Expression('NULL') : $this->width),
                 'field_height'      => ($this->height === null ? new Expression('NULL') : $this->height),
                 'field_size'        => ($this->size === null ? new Expression('NULL') : $this->size),
diff --git a/galette/lib/Galette/Entity/TranslatableTrait.php b/galette/lib/Galette/Entity/TranslatableTrait.php
index 0194792ab..3007a0539 100644
--- a/galette/lib/Galette/Entity/TranslatableTrait.php
+++ b/galette/lib/Galette/Entity/TranslatableTrait.php
@@ -64,11 +64,11 @@ trait TranslatableTrait
      * @return String
      */
     public function getName($translated = true)
     {
         if ($translated === true) {
-            return _T($this->name);
+            return _T(strip_tags($this->name));
         } else {
-            return $this->name;
+            return strip_tags($this->name);
         }
     }
 }
diff --git a/galette/templates/default/configurer_fiche_content.tpl b/galette/templates/default/configurer_fiche_content.tpl
index 477d78681..dc14d9075 100644
--- a/galette/templates/default/configurer_fiche_content.tpl
+++ b/galette/templates/default/configurer_fiche_content.tpl
@@ -25,46 +25,46 @@
                         <a
                             href="{path_for name="editDynamicField" data=["form_name" => $form_name, "id" => $field->getId()]}"
                             class="tooltip action"
                         >
                             <i class="fas fa-user-edit fa-fw" aria-hidden="true"></i>
-                            <span class="sr-only">{_T string="Edit '%s' field" pattern="/%s/" replace=$field->getName()}</span>
+                            <span class="sr-only">{_T string="Edit '%s' field" pattern="/%s/" replace=$field->getName()|escape}</span>
                         </a>
                         <a
                             href="{path_for name="dynamicTranslations" data=["text_orig" => {$field->getName(false)|escape}]}"
                             class="tooltip"
                         >
                             <i class="fas fa-language fa-fw" aria-hidden="true"></i>
-                            <span class="sr-only">{_T string="Translate '%s' field" pattern="/%s/" replace=$field->getName()}</span>
+                            <span class="sr-only">{_T string="Translate '%s' field" pattern="/%s/" replace=$field->getName()|escape}</span>
                         </a>
                         <a
                             href="{path_for name="removeDynamicField" data=["form_name" => $form_name, "id" => $field->getId()]}"
                             class="delete tooltip"
                         >
                             <i class="fas fa-trash" aria-hidden="true"></i>
-                            <span class="sr-only">{_T string="Delete '%s' field" pattern="/%s/" replace=$field->getName()}</span>
+                            <span class="sr-only">{_T string="Delete '%s' field" pattern="/%s/" replace=$field->getName()|escape}</span>
                         </a>
     {if $field->getIndex() eq 1}
                         <i class="fas fa-fw">&nbsp;</i>
     {else}
                         <a
                             href="{path_for name="moveDynamicField" data=["form_name" => $form_name, "direction" => "up", "id" => $field->getId()]}"
                             class="tooltip action"
                         >
                             <i class="fas fa-caret-up fa-fw"></i>
-                            <span class="sr-only">{_T string="Move up '%s' field" pattern="/%s/" replace=$field->getName()}</span>
+                            <span class="sr-only">{_T string="Move up '%s' field" pattern="/%s/" replace=$field->getName()|escape}</span>
                         </a>
     {/if}
     {if $field->getIndex() eq $fields_list|@count}
                         <i class="fas fa-fw">&nbsp;</i>
     {else}
                         <a
                             href="{path_for name="moveDynamicField" data=["form_name" => $form_name, "direction" => "down", "id" => $field->getId()]}"
                             class="tooltip"
                         >
                             <i class="fas fa-caret-down fa-fw"></i>
-                            <span class="sr-only">{_T string="Move down '%s' field" pattern="/%s/" replace=$field->getName()}</span>
+                            <span class="sr-only">{_T string="Move down '%s' field" pattern="/%s/" replace=$field->getName()|escape}</span>
                         </a>
     {/if}
                     </td>
                 </tr>
 {foreachelse}

Vulnerable code:                 'field_name'        => $this->name,
            return _T($this->name);
            return $this->name;
                            <span class="sr-only">{_T string="Edit '%s' field" pattern="/%s/" replace=$field->getName()}</span>
                            <span class="sr-only">{_T string="Translate '%s' field" pattern="/%s/" replace=$field->getName()}</span>
                            <span class="sr-only">{_T string="Delete '%s' field" pattern="/%s/" replace=$field->getName()}</span>
                            <span class="sr-only">{_T string="Move up '%s' field" pattern="/%s/" replace=$field->getName()}</span>
                            <span class="sr-only">{_T string="Move down '%s' field" pattern="/%s/" replace=$field->getName()}</span>
Fixed code:                 'field_name'        => strip_tags($this->name),
            return _T(strip_tags($this->name));
            return strip_tags($this->name);
                            <span class="sr-only">{_T string="Edit '%s' field" pattern="/%s/" replace=$field->getName()|escape}</span>
                            <span class="sr-only">{_T string="Translate '%s' field" pattern="/%s/" replace=$field->getName()|escape}</span>
                            <span class="sr-only">{_T string="Delete '%s' field" pattern="/%s/" replace=$field->getName()|escape}</span>
                            <span class="sr-only">{_T string="Move up '%s' field" pattern="/%s/" replace=$field->getName()|escape}</span>
                            <span class="sr-only">{_T string="Move down '%s' field" pattern="/%s/" replace=$field->getName()|escape}</span>