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/Controllers/AuthController.php, galette/lib/Galette/Controllers/Crud/MembersController.php, galette/lib/Galette/Controllers/DynamicTranslationsController.php, galette/lib/Galette/Controllers/GaletteController.php, galette/lib/Galette/Controllers/HistoryController.php, galette/lib/Galette/Controllers/PdfController.php, galette/lib/Galette/Controllers/PluginsController.php, galette/lib/Galette/Entity/Adherent.php, galette/templates/default/gestion_adherents.tpl, galette/templates/default/history.tpl, galette/templates/default/voir_adherent.tpl
Commit date: 2021-04-05 09:47:02 +0200
Commit message: Prevent some possible XSS

Few fixes
Enhance type hinting

Diff: diff --git a/galette/lib/Galette/Controllers/AuthController.php b/galette/lib/Galette/Controllers/AuthController.php
index 59410efb..a7cfb294 100644
--- a/galette/lib/Galette/Controllers/AuthController.php
+++ b/galette/lib/Galette/Controllers/AuthController.php
@@ -303,11 +303,11 @@ class AuthController extends AbstractController
         if (($this->login->isAdmin() || $this->login->isStaff()) && $id_adh !== null) {
             $adh = new Adherent($this->zdb, $id_adh);
             $login_adh = $adh->login;
         } else {
             $post = $request->getParsedBody();
-            $login_adh = $post['login'];
+            $login_adh = htmlspecialchars($post['login'], ENT_QUOTES);
             $adh = new Adherent($this->zdb, $login_adh);
         }
 
         if ($adh->id != '') {
             //account has been found, proceed
diff --git a/galette/lib/Galette/Controllers/Crud/MembersController.php b/galette/lib/Galette/Controllers/Crud/MembersController.php
index 9debace2..7b6ec402 100644
--- a/galette/lib/Galette/Controllers/Crud/MembersController.php
+++ b/galette/lib/Galette/Controllers/Crud/MembersController.php
@@ -395,11 +395,11 @@ class MembersController extends CrudController
         if (isset($post['clear_filter'])) {
             $filters->reinit();
         } else {
             //number of rows to show
             if (isset($post['nbshow'])) {
-                $filters->show = $post['nbshow'];
+                $filters->show = (int)$post['nbshow'];
             }
         }
 
         $this->session->$varname = $filters;
 
@@ -652,11 +652,11 @@ class MembersController extends CrudController
             ) {
                 $filters->group_filter = (int)$post['group_filter'];
             }
             //number of rows to show
             if (isset($post['nbshow'])) {
-                $filters->show = $post['nbshow'];
+                $filters->show = (int)$post['nbshow'];
             }
 
             if (isset($post['advanced_filtering'])) {
                 if (!$filters instanceof AdvancedMembersList) {
                     $filters = new AdvancedMembersList($filters);
@@ -672,11 +672,11 @@ class MembersController extends CrudController
                             foreach ($post['free_field'] as $f) {
                                 if (
                                     trim($f) !== ''
                                     && trim($post['free_text'][$i]) !== ''
                                 ) {
-                                    $fs_search = $post['free_text'][$i];
+                                    $fs_search = htmlspecialchars($post['free_text'][$i], ENT_QUOTES);
                                     $log_op
                                         = (int)$post['free_logical_operator'][$i];
                                     $qry_op
                                         = (int)$post['free_query_operator'][$i];
                                     $type = (int)$post['free_type'][$i];
@@ -869,11 +869,11 @@ class MembersController extends CrudController
             $filters->current_page = (int)$value;
         }
 
         //numbers of rows to display
         if (isset($post['nbshow']) && is_numeric($post['nbshow'])) {
-            $filters->show = $post['nbshow'];
+            $filters->show = (int)$post['nbshow'];
         }
 
         $members = new Members($filters);
         if (!$this->login->isAdmin() && !$this->login->isStaff()) {
             if ($this->login->isGroupManager()) {
@@ -964,15 +964,15 @@ class MembersController extends CrudController
         if (isset($post['multiple'])) {
             $params['multiple'] = true;
         }
 
         if (isset($post['gid'])) {
-            $params['the_id'] = $post['gid'];
+            $params['the_id'] = (int)$post['gid'];
         }
 
         if (isset($post['id_adh'])) {
-            $params['excluded'] = $post['id_adh'];
+            $params['excluded'] = (int)$post['id_adh'];
         }
 
         // display page
         $this->view->render(
             $response,
diff --git a/galette/lib/Galette/Controllers/DynamicTranslationsController.php b/galette/lib/Galette/Controllers/DynamicTranslationsController.php
index 16c6bf52..fce42440 100644
--- a/galette/lib/Galette/Controllers/DynamicTranslationsController.php
+++ b/galette/lib/Galette/Controllers/DynamicTranslationsController.php
@@ -168,10 +168,11 @@ class DynamicTranslationsController extends AbstractController
      * @return Response
      */
     public function doDynamicTranslations(Request $request, Response $response): Response
     {
         $post = $request->getParsedBody();
+        $post['text_orig'] = htmlspecialchars($post['text_orig'], ENT_QUOTES);
         $error_detected = [];
 
         if (isset($post['trans']) && isset($post['text_orig'])) {
             if (isset($post['new']) && $post['new'] == 'true') {
                 //create translation if it does not exists yet
diff --git a/galette/lib/Galette/Controllers/GaletteController.php b/galette/lib/Galette/Controllers/GaletteController.php
index d38264dc..dd08a63a 100644
--- a/galette/lib/Galette/Controllers/GaletteController.php
+++ b/galette/lib/Galette/Controllers/GaletteController.php
@@ -538,11 +538,11 @@ class GaletteController extends AbstractController
                 $required = false;
             }
 
             $res[$current_cat][] = array(
                 'field_id'  =>  $field,
-                'label'     =>  $post[$field . '_label'],
+                'label'     =>  htmlspecialchars($post[$field . '_label'], ENT_QUOTES),
                 'category'  =>  $post[$field . '_category'],
                 'visible'   =>  $post[$field . '_visible'],
                 'required'  =>  $required
             );
             $pos++;
diff --git a/galette/lib/Galette/Controllers/HistoryController.php b/galette/lib/Galette/Controllers/HistoryController.php
index bfcf8e3d..ad9587ac 100644
--- a/galette/lib/Galette/Controllers/HistoryController.php
+++ b/galette/lib/Galette/Controllers/HistoryController.php
@@ -139,11 +139,11 @@ class HistoryController extends AbstractController
             $filters->reinit();
         } else {
             if (
                 (isset($post['nbshow']) && is_numeric($post['nbshow']))
             ) {
-                $filters->show = $post['nbshow'];
+                $filters->show = (int)$post['nbshow'];
             }
 
             if (isset($post['end_date_filter']) || isset($post['start_date_filter'])) {
                 try {
                     if (isset($post['start_date_filter'])) {
diff --git a/galette/lib/Galette/Controllers/PdfController.php b/galette/lib/Galette/Controllers/PdfController.php
index 62d4a3eb..15658dd2 100644
--- a/galette/lib/Galette/Controllers/PdfController.php
+++ b/galette/lib/Galette/Controllers/PdfController.php
@@ -578,11 +578,11 @@ class PdfController extends AbstractController
             $error_detected[] = _T("Missing PDF model type!");
         } else {
             $type = (int)$post['model_type'];
             $class = PdfModel::getTypeClass($type);
             if (isset($post[PdfModel::PK])) {
-                $model = new $class($this->zdb, $this->preferences, (int)$_POST[PdfModel::PK]);
+                $model = new $class($this->zdb, $this->preferences, (int)$post[PdfModel::PK]);
             } else {
                 $model = new $class($this->zdb, $this->preferences);
             }
 
             try {
diff --git a/galette/lib/Galette/Controllers/PluginsController.php b/galette/lib/Galette/Controllers/PluginsController.php
index 1b3ad938..59d56442 100644
--- a/galette/lib/Galette/Controllers/PluginsController.php
+++ b/galette/lib/Galette/Controllers/PluginsController.php
@@ -201,12 +201,12 @@ class PluginsController extends AbstractController
 
         if (isset($post['stepback_btn'])) {
             $install->atPreviousStep();
         } elseif (isset($post['install_prefs_ok'])) {
             $install->atEndStep();
-        } elseif (isset($_POST['previous_version'])) {
-            $install->setInstalledVersion($_POST['previous_version']);
+        } elseif (isset($post['previous_version'])) {
+            $install->setInstalledVersion($post['previous_version']);
             $install->atDbUpgradeStep();
         } elseif (isset($post['install_dbperms_ok'])) {
             if ($install->isInstall()) {
                 $install->atDbInstallStep();
             } elseif ($install->isUpgrade()) {
@@ -284,11 +284,11 @@ class PluginsController extends AbstractController
                 include GALETTE_ROOT . 'includes/sql_parse.php';
                 if ($step == 'u4') {
                     $update_scripts = Install::getUpdateScripts(
                         $plugin['root'],
                         TYPE_DB,
-                        $_POST['previous_version']
+                        $post['previous_version']
                     );
                 } else {
                     $update_scripts['current'] = TYPE_DB . '.sql';
                 }
 
diff --git a/galette/lib/Galette/Entity/Adherent.php b/galette/lib/Galette/Entity/Adherent.php
index 85ca9202..af66dd63 100644
--- a/galette/lib/Galette/Entity/Adherent.php
+++ b/galette/lib/Galette/Entity/Adherent.php
@@ -887,11 +887,11 @@ class Adherent
             $str .= $id;
         }
         if ($id !== false || $nick !== false) {
             $str .= ')';
         }
-        return $str;
+        return strip_tags($str);
     }
 
     /**
      * Change password for a given user
      *
@@ -1677,11 +1677,11 @@ class Adherent
                     case 'saddress':
                         $address = $this->_address;
                         if ($this->_address_continuation !== '' && $this->_address_continuation !== null) {
                             $address .= "\n" . $this->_address_continuation;
                         }
-                        return $address;
+                        return htmlspecialchars($address, ENT_QUOTES);
                         break;
                     case 'sname':
                         return $this->getNameWithCase($this->_name, $this->_surname);
                         break;
                     case 'rbirthdate':
diff --git a/galette/templates/default/gestion_adherents.tpl b/galette/templates/default/gestion_adherents.tpl
index 13c6068b..8e338e10 100644
--- a/galette/templates/default/gestion_adherents.tpl
+++ b/galette/templates/default/gestion_adherents.tpl
@@ -219,20 +219,22 @@ We have to use a template file, so Smarty will do its work (like replacing varia
                         </span>
             {else}
                         <img src="{base_url}/{$template_subdir}images/icon-empty.png" alt="" width="16" height="16"/>
             {/if}
                         {assign var="mid" value=$member->id}
-                        <a href="{path_for name="member" data=["id" => $member->id]}">{$member->sname}{if $member->company_name} ({$member->company_name}){/if}</a>
+                        <a href="{path_for name="member" data=["id" => $member->id]}">{$member->sname}{if $member->company_name} ({$member->company_name|escape}){/if}</a>
                     </td>
         {else}
             {assign var="lrclass" value=$rclass}
             {assign var="propname" value=$column->propname}
-            {assign var=value value=$member->$propname}
+            {assign var=value value=$member->$propname|escape}
 
-            {if $column->field_id eq 'pseudo_adh'}
+            {if $column->field_id eq 'nom_adh'}
+                {assign var="value" value=$member->sfullname}
+            {elseif $column->field_id eq 'pseudo_adh'}
                 {assign var="lrclass" value="$rclass nowrap"}
-                {assign var=value value=$member->$propname|htmlspecialchars}
+                {assign var=value value=$member->$propname|escape}
             {elseif $column->field_id eq 'tel_adh' or $column->field_id eq 'gsm_adh'}
                 {assign var="lrclass" value="$rclass nowrap"}
             {elseif $column->field_id eq 'id_statut'}
                 {assign var="lrclass" value="$rclass nowrap"}
                 {assign var=value value={statusLabel id=$member->$propname}}
diff --git a/galette/templates/default/history.tpl b/galette/templates/default/history.tpl
index 17efef45..f97961dc 100644
--- a/galette/templates/default/history.tpl
+++ b/galette/templates/default/history.tpl
@@ -24,11 +24,11 @@
     {if $actions|@count gt 0}
             <label for="action_filter">{_T string="Action"}</label>&nbsp;
             <select name="action_filter" id="action_filter">
                 <option value="0">{_T string="Select an action"}</option>
         {foreach from=$actions item=$action}
-                <option value="{$action}"{if $history->filters->action_filter eq $action} selected="selected"{/if}>{$action}</option>
+                <option value="{$action|escape}"{if $history->filters->action_filter eq $action} selected="selected"{/if}>{$action|escape}</option>
         {/foreach}
             </select>
     {/if}
 
 
@@ -124,13 +124,13 @@
                         </span>
                     </td>
                     <td class="nowrap" data-title="{_T string="Date"}">{$log.date_log|date_format:"%a %d/%m/%Y - %R"}</td>
                     <td class="nowrap" data-title="{_T string="IP"}">{$log.ip_log}</td>
                     <td data-title="{_T string="User"}">{$log.adh_log}</td>
-                    <td data-title="{_T string="Action"}">{$log.action_log}</td>
+                    <td data-title="{_T string="Action"}">{$log.action_log|escape}</td>
                     <td data-title="{_T string="Description"}">
-                        {$log.text_log}
+                        {$log.text_log|escape}
         {if $log.sql_log}
                         <span class="sql_log">{$log.sql_log|escape:"htmlall"}</span>
         {/if}
                     </td>
                 </tr>
diff --git a/galette/templates/default/voir_adherent.tpl b/galette/templates/default/voir_adherent.tpl
index 74b98c94..0f6e2c74 100644
--- a/galette/templates/default/voir_adherent.tpl
+++ b/galette/templates/default/voir_adherent.tpl
@@ -91,11 +91,11 @@
                 </a>
             </li>
             <li>
                 <a
                     href="{path_for name="duplicateMember" data=["id_adh" => $member->id]}"
-                    title="{_T string="Create a new member with %name information." pattern="/%name/" replace=$member->sname}"
+                    title="{_T string="Create a new member with %name information." pattern="/%name/" replace=$member->sfullname}"
                     class="button bigbutton tooltip"
                 >
                     <i class="fas fa-clone fa-fw fa-2x" aria-hidden="true"></i>
                     {_T string="Duplicate"}
                 </a>
@@ -141,11 +141,11 @@ We have to use a template file, so Smarty will do its work (like replacing varia
         {/if}
         {assign var="propname" value=$element->propname}
         {assign var="value" value=$member->$propname|escape}
 
         {if $element->field_id eq 'nom_adh'}
-            {assign var="value" value=$member->sfullname|escape}
+            {assign var="value" value=$member->sfullname}
         {elseif $element->field_id eq 'pref_lang'}
             {assign var="value" value=$pref_lang}
         {elseif $element->field_id eq 'adresse_adh'}
             {assign var="value" value=$member->saddress|escape|nl2br}
         {elseif $element->field_id eq 'bool_display_info'}

Vulnerable code:             $login_adh = $post['login'];
                $filters->show = $post['nbshow'];
                $filters->show = $post['nbshow'];
                                    $fs_search = $post['free_text'][$i];
            $filters->show = $post['nbshow'];
            $params['the_id'] = $post['gid'];
            $params['excluded'] = $post['id_adh'];
                'label'     =>  $post[$field . '_label'],
                $filters->show = $post['nbshow'];
                $model = new $class($this->zdb, $this->preferences, (int)$_POST[PdfModel::PK]);
        } elseif (isset($_POST['previous_version'])) {
            $install->setInstalledVersion($_POST['previous_version']);
                        $_POST['previous_version']
        return $str;
                        return $address;
                        <a href="{path_for name="member" data=["id" => $member->id]}">{$member->sname}{if $member->company_name} ({$member->company_name}){/if}</a>
            {assign var=value value=$member->$propname}
            {if $column->field_id eq 'pseudo_adh'}
                {assign var=value value=$member->$propname|htmlspecialchars}
                <option value="{$action}"{if $history->filters->action_filter eq $action} selected="selected"{/if}>{$action}</option>
                    <td data-title="{_T string="Action"}">{$log.action_log}</td>
                        {$log.text_log}
                    title="{_T string="Create a new member with %name information." pattern="/%name/" replace=$member->sname}"
            {assign var="value" value=$member->sfullname|escape}
Fixed code:             $login_adh = htmlspecialchars($post['login'], ENT_QUOTES);
                $filters->show = (int)$post['nbshow'];
                $filters->show = (int)$post['nbshow'];
                                    $fs_search = htmlspecialchars($post['free_text'][$i], ENT_QUOTES);
            $filters->show = (int)$post['nbshow'];
            $params['the_id'] = (int)$post['gid'];
            $params['excluded'] = (int)$post['id_adh'];
        $post['text_orig'] = htmlspecialchars($post['text_orig'], ENT_QUOTES);
                'label'     =>  htmlspecialchars($post[$field . '_label'], ENT_QUOTES),
                $filters->show = (int)$post['nbshow'];
                $model = new $class($this->zdb, $this->preferences, (int)$post[PdfModel::PK]);
        } elseif (isset($post['previous_version'])) {
            $install->setInstalledVersion($post['previous_version']);
                        $post['previous_version']
        return strip_tags($str);
                        return htmlspecialchars($address, ENT_QUOTES);
                        <a href="{path_for name="member" data=["id" => $member->id]}">{$member->sname}{if $member->company_name} ({$member->company_name|escape}){/if}</a>
            {assign var=value value=$member->$propname|escape}
            {if $column->field_id eq 'nom_adh'}
                {assign var="value" value=$member->sfullname}
            {elseif $column->field_id eq 'pseudo_adh'}
                {assign var=value value=$member->$propname|escape}
                <option value="{$action|escape}"{if $history->filters->action_filter eq $action} selected="selected"{/if}>{$action|escape}</option>
                    <td data-title="{_T string="Action"}">{$log.action_log|escape}</td>
                        {$log.text_log|escape}
                    title="{_T string="Create a new member with %name information." pattern="/%name/" replace=$member->sfullname}"
            {assign var="value" value=$member->sfullname}