From 134cac1945c8523708a2fb63cb5777d5c375d78b Mon Sep 17 00:00:00 2001 From: Johan Cwiklinski Date: Tue, 17 Sep 2024 10:49:52 +0200 Subject: [PATCH 1/3] Ensure htmlspecialchars is used for echo in U* classes --- src/User.php | 178 +++++++++++++++++++++++----------------------- src/UserEmail.php | 8 +-- 2 files changed, 94 insertions(+), 92 deletions(-) diff --git a/src/User.php b/src/User.php index 6269014daff..a0f5d542f7d 100644 --- a/src/User.php +++ b/src/User.php @@ -184,7 +184,7 @@ public function canCreateItem(): bool && (Profile::getDefault() == 0) ) { echo "
" . - __('You must define a default profile to create a new user') . "
"; + __s('You must define a default profile to create a new user') . ""; } return false; @@ -299,7 +299,7 @@ final public function loadPreferencesInSession(): void * Load minimal session for user. * * @param integer $entities_id Entity to use - * @param boolean $is_recursive Whether to load entities recursivly or not + * @param boolean $is_recursive Whether to load entities recursively or not * * @return void * @@ -2543,7 +2543,7 @@ protected function getFormHeaderToolbar(): array if ($ID > 0) { $vcard_lbl = __s('Download user VCard'); - $vcard_url = self::getFormURLWithID($ID) . "&getvcard=1"; + $vcard_url = htmlspecialchars(self::getFormURLWithID($ID) . "&getvcard=1"); $vcard_btn = <<fields["authtype"] == Auth::DB_GLPI) ) { //display login field for new records, or if this is not external auth - echo "fields["name"] . "\" class='form-control'>"; + echo "fields["name"]) . "\" class='form-control'>"; } else { echo "" . $this->fields["name"]; - echo "fields["name"] . "\" class='form-control'>"; + echo "fields["name"]) . "\" class='form-control'>"; } if (!$simplified_form && !empty($this->fields["name"])) { @@ -2675,7 +2676,7 @@ public function showForm($ID, array $options = []) echo self::getPictureForUser($ID); echo Html::file(['name' => 'picture', 'display' => false, 'onlyimages' => true]); - echo " " . __('Clear'); + echo " " . __s('Clear'); echo ""; } else { echo ""; @@ -2690,7 +2691,7 @@ public function showForm($ID, array $options = []) && AuthLDAP::isSyncFieldConfigured($this->fields['auths_id']) ) { $syncrand = mt_rand(); - echo ""; + echo ""; if ( self::canUpdate() && (!$extauth || empty($ID)) @@ -2715,7 +2716,7 @@ public function showForm($ID, array $options = []) } $surnamerand = mt_rand(); - echo ""; + echo ""; echo Html::input( 'realname', [ @@ -2726,7 +2727,7 @@ public function showForm($ID, array $options = []) echo ""; $firstnamerand = mt_rand(); - echo ""; + echo ""; echo Html::input( 'firstname', [ @@ -2766,19 +2767,19 @@ public function showForm($ID, array $options = []) && $caneditpassword ) { echo ""; - echo ""; + echo ""; echo ""; echo ""; - echo ""; + echo ""; echo ""; echo ""; if ($CFG_GLPI["use_password_security"]) { echo ""; echo ""; - echo __('Password security policy'); + echo __s('Password security policy'); echo ""; echo ""; Config::displayPasswordSecurityChecks(); @@ -2792,7 +2793,7 @@ public function showForm($ID, array $options = []) if ((!$simplified_form) && ($DB->use_timezones || Session::haveRight("config", READ))) { echo ""; - echo ""; + echo ""; if ($DB->use_timezones) { $timezones = $DB->getTimezones(); Dropdown::showFromArray( @@ -2806,9 +2807,9 @@ public function showForm($ID, array $options = []) ); } else if (Session::haveRight("config", READ)) { // Display a warning but only if user is more or less an admin - echo __('Timezone usage has not been activated.') + echo __s('Timezone usage has not been activated.') . ' ' - . sprintf(__('Run the "%1$s" command to activate it.'), 'php bin/console database:enable_timezones'); + . sprintf(__s('Run the "%1$s" command to activate it.'), 'php bin/console database:enable_timezones'); } echo ""; } @@ -2816,13 +2817,13 @@ public function showForm($ID, array $options = []) echo ""; if (!GLPI_DEMO_MODE) { $activerand = mt_rand(); - echo ""; + echo ""; Dropdown::showYesNo('is_active', $this->fields['is_active'], -1, ['rand' => $activerand]); echo ""; } else { echo ""; } - echo "" . _n('Email', 'Emails', Session::getPluralNumber()); + echo "" . _sn('Email', 'Emails', Session::getPluralNumber()); UserEmail::showAddEmailButton($this); echo ""; UserEmail::showForUser($this); @@ -2832,7 +2833,7 @@ public function showForm($ID, array $options = []) if ((!$simplified_form) && (!GLPI_DEMO_MODE)) { $sincerand = mt_rand(); echo ""; - echo ""; + echo ""; Html::showDateTimeField("begin_date", ['value' => $this->fields["begin_date"], 'rand' => $sincerand, 'maybeempty' => true @@ -2840,7 +2841,7 @@ public function showForm($ID, array $options = []) echo ""; $untilrand = mt_rand(); - echo ""; + echo ""; Html::showDateTimeField("end_date", ['value' => $this->fields["end_date"], 'rand' => $untilrand, 'maybeempty' => true @@ -2850,7 +2851,7 @@ public function showForm($ID, array $options = []) $phonerand = mt_rand(); echo ""; - echo ""; + echo ""; echo Html::input( 'phone', [ @@ -2863,28 +2864,28 @@ public function showForm($ID, array $options = []) //don't display is creation of a new user' if (!empty($ID)) { if (Session::haveRight(self::$rightname, self::READAUTHENT)) { - echo "" . __('Authentication') . ""; + echo "" . __s('Authentication') . ""; echo Auth::getMethodName($this->fields["authtype"], $this->fields["auths_id"]); if (!empty($this->fields["date_sync"])) { //TRANS: %s is the date of last sync echo '
' . sprintf( - __('Last synchronization on %s'), + __s('Last synchronization on %s'), Html::convDateTime($this->fields["date_sync"]) ); } if (!empty($this->fields["user_dn"])) { //TRANS: %s is the user dn - echo '
' . sprintf(__('%1$s: %2$s'), __('User DN'), $this->fields["user_dn"]); + echo '
' . sprintf(__s('%1$s: %2$s'), __s('User DN'), $this->fields["user_dn"]); } if ($this->fields['is_deleted_ldap']) { - echo '
' . __('User missing in LDAP directory'); + echo '
' . __s('User missing in LDAP directory'); } if ($this->fields['2fa'] !== null) { - echo '
' . __('2FA enabled'); + echo '
' . __s('2FA enabled'); if (Session::haveRight(self::$rightname, self::UPDATEAUTHENT)) { echo ""; } } @@ -2901,7 +2902,7 @@ public function showForm($ID, array $options = []) $mobilerand = mt_rand(); echo ""; - echo ""; + echo ""; echo Html::input( 'mobile', [ @@ -2913,7 +2914,7 @@ public function showForm($ID, array $options = []) echo ""; if (!$simplified_form) { $catrand = mt_rand(); - echo ""; + echo ""; UserCategory::dropdown(['value' => $this->fields["usercategories_id"], 'rand' => $catrand]); } echo ""; @@ -2921,7 +2922,7 @@ public function showForm($ID, array $options = []) if (!$simplified_form) { $phone2rand = mt_rand(); echo ""; - echo ""; + echo ""; echo Html::input( 'phone2', [ @@ -2930,13 +2931,13 @@ public function showForm($ID, array $options = []) ] ); echo ""; - echo ""; + echo ""; echo ""; - echo ""; + echo ""; echo ""; $admnumrand = mt_rand(); - echo ""; + echo ""; echo Html::input( 'registration_number', [ @@ -2947,7 +2948,7 @@ public function showForm($ID, array $options = []) echo ""; $titlerand = mt_rand(); - echo ""; + echo ""; UserTitle::dropdown(['value' => $this->fields["usertitles_id"], 'rand' => $titlerand]); echo ""; } @@ -2955,7 +2956,7 @@ public function showForm($ID, array $options = []) echo ""; if (!empty($ID)) { $locrand = mt_rand(); - echo ""; + echo ""; $entities = $this->getEntities(); if (count($entities) <= 0) { $entities = -1; @@ -2972,19 +2973,19 @@ public function showForm($ID, array $options = []) echo ""; echo "" . _n('Authorization', 'Authorizations', 1) . ""; $recurrand = mt_rand(); - echo ""; + echo ""; Dropdown::showYesNo("_is_recursive", 0, -1, ['rand' => $recurrand]); echo ""; $profilerand = mt_rand(); echo ""; - echo ""; + echo ""; Profile::dropdownUnder(['name' => '_profiles_id', 'rand' => $profilerand, 'value' => Profile::getDefault() ]); $entrand = mt_rand(); - echo ""; + echo ""; Entity::dropdown(['name' => '_entities_id', 'display_emptychoice' => false, 'rand' => $entrand, @@ -2996,7 +2997,7 @@ public function showForm($ID, array $options = []) if ($higherrights || $ismyself) { $profilerand = mt_rand(); echo ""; - echo ""; + echo ""; $options = Dropdown::getDropdownArrayNames( 'glpi_profiles', @@ -3014,7 +3015,7 @@ public function showForm($ID, array $options = []) } if ($higherrights) { $entrand = mt_rand(); - echo ""; + echo ""; $entities = $this->getEntities(); Entity::dropdown(['value' => $this->fields["entities_id"], 'rand' => $entrand, @@ -3024,7 +3025,7 @@ public function showForm($ID, array $options = []) $grouprand = mt_rand(); echo ""; - echo ""; + echo ""; $options = []; foreach (Group_User::getUserGroups($this->fields['id']) as $group) { @@ -3042,7 +3043,7 @@ public function showForm($ID, array $options = []) echo ""; $userrand = mt_rand(); - echo ""; + echo ""; User::dropdown(['name' => 'users_id_supervisor', 'value' => $this->fields["users_id_supervisor"], @@ -3063,7 +3064,7 @@ public function showForm($ID, array $options = []) && Session::getCurrentInterface() == "central" ) { echo ""; - echo ""; + echo ""; echo ""; if ($this->can($ID, UPDATE)) { echo Html::input('nickname', [ @@ -3077,7 +3078,7 @@ public function showForm($ID, array $options = []) } if ($caneditpassword) { - echo "" . __('Remote access keys') . ""; + echo "" . __s('Remote access keys') . ""; echo ""; echo __("API token"); @@ -3090,7 +3091,7 @@ public function showForm($ID, array $options = []) ]); echo ""; echo "(" . sprintf( - __('generated on %s'), + __s('generated on %s'), Html::convDateTime($this->fields["api_token_date"]) ) . ")"; } @@ -3150,15 +3151,15 @@ public function showMyForm($target, $ID) && !empty($this->fields["password"]))); echo "
"; - echo "
"; + echo ""; echo ""; - echo ""; $surnamerand = mt_rand(); - echo ""; if (!empty($this->fields["name"])) { - echo ""; + echo ""; echo ""; } else { @@ -3256,13 +3257,13 @@ public function showMyForm($target, $ID) && Session::haveRight("password_update", "1") ) { echo ""; - echo ""; + echo ""; echo ""; echo ""; echo ""; - echo ""; + echo ""; echo ""; @@ -3284,7 +3285,7 @@ public function showMyForm($target, $ID) if ($DB->use_timezones || Session::haveRight("config", READ)) { echo ""; - echo ""; if ( @@ -3313,13 +3314,13 @@ public function showMyForm($target, $ID) } $phonerand = mt_rand(); - echo ""; - echo ""; $mobilerand = mt_rand(); - echo ""; $phone2rand = mt_rand(); - echo ""; $admnumrand = mt_rand(); - echo ""; $locrand = mt_rand(); - echo ""; - echo ""; + echo ""; echo ""; } - echo ""; + echo ""; echo ""; echo "
" . sprintf(__('%1$s: %2$s'), __('Login'), $this->fields["name"]); - echo ""; + echo "
" . sprintf(__s('%1$s: %2$s'), __s('Login'), htmlspecialchars($this->fields["name"])); + echo ""; echo ""; echo "
"; + echo "
"; if ( $extauth @@ -3178,7 +3179,7 @@ public function showMyForm($target, $ID) echo "" . _n('Picture', 'Pictures', 1) . "" . _sn('Picture', 'Pictures', 1) . ""; echo self::getPictureForUser($ID); @@ -3193,13 +3194,13 @@ public function showMyForm($target, $ID) } $firstnamerand = mt_rand(); - echo "
"; + echo "
"; if ( $extauth && isset($authtype['firstname_field']) && !empty($authtype['firstname_field']) ) { - echo $this->fields["firstname"]; + echo htmlspecialchars($this->fields["firstname"]); } else { echo Html::input( 'firstname', @@ -3216,11 +3217,11 @@ public function showMyForm($target, $ID) && $this->fields['auths_id'] && AuthLDAP::isSyncFieldConfigured($this->fields['auths_id']) ) { - echo "
" . __('Synchronization field') . ""; + echo "
" . __s('Synchronization field') . ""; if (empty($this->fields['sync_field'])) { echo Dropdown::EMPTY_VALUE; } else { - echo $this->fields['sync_field']; + echo htmlspecialchars($this->fields['sync_field']); } echo "
"; echo "
"; echo "
"; + echo ""; if ($DB->use_timezones) { $timezones = $DB->getTimezones(); Dropdown::showFromArray( @@ -3298,9 +3299,9 @@ public function showMyForm($target, $ID) ); } else if (Session::haveRight("config", READ)) { // Display a warning but only if user is more or less an admin - echo __('Timezone usage has not been activated.') + echo __s('Timezone usage has not been activated.') . ' ' - . sprintf(__('Run the "%1$s" command to activate it.'), 'php bin/console database:enable_timezones'); + . sprintf(__s('Run the "%1$s" command to activate it.'), 'php bin/console database:enable_timezones'); } echo "
"; + echo "
"; if ( $extauth && isset($authtype['phone_field']) && !empty($authtype['phone_field']) ) { - echo $this->fields["phone"]; + echo htmlspecialchars($this->fields["phone"]); } else { echo Html::input( 'phone', @@ -3330,7 +3331,7 @@ public function showMyForm($target, $ID) ); } echo "" . _n('Email', 'Emails', Session::getPluralNumber()); + echo "" . _sn('Email', 'Emails', Session::getPluralNumber()); UserEmail::showAddEmailButton($this); echo ""; UserEmail::showForUser($this); @@ -3338,13 +3339,13 @@ public function showMyForm($target, $ID) echo "
"; + echo "
"; if ( $extauth && isset($authtype['mobile_field']) && !empty($authtype['mobile_field']) ) { - echo $this->fields["mobile"]; + echo htmlspecialchars($this->fields["mobile"]); } else { echo Html::input( 'mobile', @@ -3358,7 +3359,7 @@ public function showMyForm($target, $ID) if (count($_SESSION['glpiprofiles']) > 1) { $profilerand = mt_rand(); - echo ""; + echo ""; $options = Dropdown::getDropdownArrayNames( 'glpi_profiles', @@ -3379,13 +3380,13 @@ public function showMyForm($target, $ID) echo "
"; + echo "
"; if ( $extauth && isset($authtype['phone2_field']) && !empty($authtype['phone2_field']) ) { - echo $this->fields["phone2"]; + echo htmlspecialchars($this->fields["phone2"]); } else { echo Html::input( 'phone2', @@ -3403,7 +3404,7 @@ public function showMyForm($target, $ID) && (count($_SESSION['glpiactiveentities']) > 1) ) { $entrand = mt_rand(); - echo ""; + echo ""; Entity::dropdown(['value' => $this->fields['entities_id'], 'rand' => $entrand, 'entity' => $entities @@ -3414,7 +3415,7 @@ public function showMyForm($target, $ID) echo "
"; + echo "
"; if ( $extauth && isset($authtype['registration_number_field']) && !empty($authtype['registration_number_field']) @@ -3432,7 +3433,7 @@ public function showMyForm($target, $ID) echo "
"; + echo "
"; Location::dropdown(['value' => $this->fields['locations_id'], 'rand' => $locrand, 'entity' => $entities @@ -3440,7 +3441,7 @@ public function showMyForm($target, $ID) if (Config::canUpdate()) { $moderand = mt_rand(); - echo ""; + echo ""; $modes = [ Session::NORMAL_MODE => __('Normal'), Session::DEBUG_MODE => __('Debug'), @@ -3459,7 +3460,7 @@ public function showMyForm($target, $ID) && Session::getCurrentInterface() == "central" ) { echo "
"; echo Html::input('nickname', [ 'value' => $this->fields['nickname'] @@ -3468,10 +3469,10 @@ public function showMyForm($target, $ID) echo "
" . __('Remote access keys') . "
" . __s('Remote access keys') . "
"; - echo __("API token"); + echo __s("API token"); echo ""; if (!empty($this->fields["api_token"])) { echo "
"; @@ -3481,7 +3482,7 @@ public function showMyForm($target, $ID) ]); echo "
"; echo "(" . sprintf( - __('generated on %s'), + __s('generated on %s'), Html::convDateTime($this->fields["api_token_date"]) ) . ")"; } @@ -3489,7 +3490,7 @@ public function showMyForm($target, $ID) Html::showCheckbox(['name' => '_reset_api_token', 'title' => __('Regenerate') ]); - echo "  " . __('Regenerate'); + echo "  " . __s('Regenerate'); echo "
"; @@ -4912,7 +4913,7 @@ public static function dropdown($options = []) if (empty($user['comment'])) { $user['comment'] = Toolbox::ucfirst( sprintf( - __('Show %1$s'), + __s('Show %1$s'), self::getTypeName(Session::getPluralNumber()) ) ); @@ -4955,7 +4956,7 @@ public static function dropdown($options = []) $CFG_GLPI["root_doc"] . "/front/ldap.import.php?entity=" . $_SESSION['glpiactive_entity'], - ['title' => __('Import a user'), + ['title' => __s('Import a user'), 'display' => false ] ); @@ -4998,9 +4999,9 @@ public static function showAddExtAuthForm() echo "\n"; echo "\n"; - echo "\n"; + echo "\n"; - echo "\n"; + echo "\n"; echo ""; echo ""; @@ -5765,7 +5766,7 @@ public function showForgetPassword($email) Session::addMessageAfterRedirect(htmlspecialchars($e->getMessage()), false, ERROR); return; } - Session::addMessageAfteRredirect(__s('If the given email address match an exisiting GLPI user, you will receive an email containing the informations required to reset your password. Please contact your administrator if you do not receive any email.')); + Session::addMessageAfteRredirect(__s('If the given email address match an existing GLPI user, you will receive an email containing the informations required to reset your password. Please contact your administrator if you do not receive any email.')); TemplateRenderer::getInstance()->display('forgotpassword.html.twig', [ 'messages_only' => true, @@ -6781,7 +6782,7 @@ public function showSystemUserForm($ID, array $options = []): bool echo ""; $surnamerand = mt_rand(); - echo ""; + echo ""; echo ""; - echo ""; + echo ""; echo ""; $firstnamerand = mt_rand(); - echo ""; @@ -7039,7 +7040,8 @@ private static function createUserFromMail(string $email): ?User return $user; } - /** + + /** * Get name of the user with ID * * @param integer $ID ID of the user. diff --git a/src/UserEmail.php b/src/UserEmail.php index df50dce6558..fe24f000d00 100644 --- a/src/UserEmail.php +++ b/src/UserEmail.php @@ -154,9 +154,9 @@ public static function getJSCodeToAddForItemChild($field_name, $child_count_js_v { return " " . - ""; + " value=\'-'+" . htmlspecialchars($child_count_js_var) . "+'\'> " . + ""; } @@ -176,7 +176,7 @@ public function showChildForItemForm($canedit, $field_name, $id, bool $display = $value = htmlspecialchars($this->fields['email'] ?? ''); } $result = ""; - $field_name = $field_name . "[$id]"; + $field_name = htmlspecialchars($field_name . "[$id]"); $result .= "
"; $result .= " Date: Tue, 17 Sep 2024 10:58:53 +0200 Subject: [PATCH 2/3] Apply suggestions from code review MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Cédric Anne --- src/User.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/User.php b/src/User.php index a0f5d542f7d..eb66af18d07 100644 --- a/src/User.php +++ b/src/User.php @@ -2543,7 +2543,7 @@ protected function getFormHeaderToolbar(): array if ($ID > 0) { $vcard_lbl = __s('Download user VCard'); - $vcard_url = htmlspecialchars(self::getFormURLWithID($ID) . "&getvcard=1"); + $vcard_url = htmlspecialchars(self::getFormURLWithID($ID) . "&getvcard=1"); $vcard_btn = <<fields["user_dn"])) { //TRANS: %s is the user dn - echo '
' . sprintf(__s('%1$s: %2$s'), __s('User DN'), $this->fields["user_dn"]); + echo '
' . sprintf(__s('%1$s: %2$s'), __s('User DN'), htmlspecialchars($this->fields["user_dn"])); } if ($this->fields['is_deleted_ldap']) { echo '
' . __s('User missing in LDAP directory'); From 1fd008c8190f5d71ceada215b90a6e7443fd2601 Mon Sep 17 00:00:00 2001 From: Johan Cwiklinski Date: Tue, 17 Sep 2024 11:12:15 +0200 Subject: [PATCH 3/3] Apply suggestions from code review MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Cédric Anne --- src/User.php | 2 +- src/UserEmail.php | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/User.php b/src/User.php index eb66af18d07..b7fce79fcbc 100644 --- a/src/User.php +++ b/src/User.php @@ -3155,7 +3155,7 @@ public function showMyForm($target, $ID) echo "
" . __('Automatically add a user of an external source') . "
" . __s('Automatically add a user of an external source') . "
" . __('Login') . "
" . __s('Login') . "
"; echo Html::input( 'realname', @@ -6792,7 +6793,7 @@ public function showSystemUserForm($ID, array $options = []): bool ); echo "" . _n('Picture', 'Pictures', 1) . "" . _sn('Picture', 'Pictures', 1) . ""; echo self::getPictureForUser($ID); @@ -6802,7 +6803,7 @@ public function showSystemUserForm($ID, array $options = []): bool echo "
"; + echo "
"; echo Html::input( 'firstname', [ @@ -6814,9 +6815,9 @@ public function showSystemUserForm($ID, array $options = []): bool echo "
"; echo ""; - echo __("This is a special user used for automated actions. "); + echo __s("This is a special user used for automated actions. "); echo '
'; - echo __("You can set its name to your organisation's name. "); + echo __s("You can set its name to your organisation's name. "); echo ""; echo "
"; echo ""; $surnamerand = mt_rand(); diff --git a/src/UserEmail.php b/src/UserEmail.php index fe24f000d00..68218bcfe84 100644 --- a/src/UserEmail.php +++ b/src/UserEmail.php @@ -179,7 +179,7 @@ public function showChildForItemForm($canedit, $field_name, $id, bool $display = $field_name = htmlspecialchars($field_name . "[$id]"); $result .= "
"; $result .= "
" . sprintf(__s('%1$s: %2$s'), __s('Login'), htmlspecialchars($this->fields["name"])); echo ""; - echo ""; + echo ""; echo "