N°8201 - [CVE_Request]_Cross-Site-Script Reflected(XSS Reflected at the name="attr_installed" (Low or Medium)

application/applicationextension.inc.php

@@ -1316,6 +1316,11 @@ interface iRestServiceProvider
 	public function ExecOperation($sVersion, $sVerb, $aParams);
 }
 
+interface iRestInputSanitizer
+{
+	public function SanitizeJsonInput(string $sJsonInput): string;
+}
+
 /**
  * Minimal REST response structure. Derive this structure to add response data and error codes.
  *
@@ -1405,6 +1410,14 @@ class RestResult
 	 * @api
 	 */
 	public $message;
+	
+	/**
+	 * Sanitize the content of this result to hide sensitive information
+	 */
+	public function SanitizeContent()
+	{
+		// The default implementation does nothing
+	}
 }
 
 /**

application/dashboard.class.inc.php

@@ -529,10 +529,7 @@ EOF
 	 */
 	public function Render($oPage, $bEditMode = false, $aExtraParams = array(), $bCanEdit = true)
 	{
-		if (!array_key_exists('dashboard_div_id', $aExtraParams))
-		{
-			$aExtraParams['dashboard_div_id'] = utils::Sanitize($this->GetId(), '', 'element_identifier');
-		}
+		$aExtraParams['dashboard_div_id'] = utils::Sanitize($aExtraParams['dashboard_div_id'] ?? null, $this->GetId(), utils::ENUM_SANITIZATION_FILTER_ELEMENT_IDENTIFIER);
 
 		$oPage->add('<div class="dashboard-title-line"><div class="dashboard-title">'.htmlentities(Dict::S($this->sTitle), ENT_QUOTES, 'UTF-8', false).'</div></div>');
 
@@ -1002,7 +999,7 @@ EOF
 		$sSelectorHtml .= '</div>';
 		$sSelectorHtml = addslashes($sSelectorHtml);
 		$sFile = addslashes($this->GetDefinitionFile());
-		$sReloadURL = $this->GetReloadURL();
+		$sReloadURL = json_encode($this->GetReloadURL());
 
 		$oPage->add_ready_script(
 <<<EOF
@@ -1059,7 +1056,7 @@ EOF
 		$oPage->add_linked_script(utils::GetAbsoluteUrlAppRoot().'js/jquery.iframe-transport.js');
 		$oPage->add_linked_script(utils::GetAbsoluteUrlAppRoot().'js/jquery.fileupload.js');
 		$sEditMenu = "<div id=\"DashboardMenu\"><ul><li><i class=\"top-right-icon icon-additional-arrow fas fa-pencil-alt\"></i><ul>";
-	
+
 		$aActions = array();
 		$sFile = addslashes($this->sDefinitionFile);
 		$sJSExtraParams = json_encode($aExtraParams);
@@ -1091,6 +1088,7 @@ EOF
 	
 EOF
 		);
+        $sReloadURL = json_encode($this->GetReloadURL());
 		$oPage->add_script(
 <<<EOF
 function EditDashboard(sId, sDashboardFile, aExtraParams)
@@ -1188,7 +1186,7 @@ EOF
 		$oPage->add('</div>');
 		$oPage->add('<div id="event_bus"/>'); // For exchanging messages between the panes, same as in the designer
 		$oPage->add('</div>');
-		
+
 		$sDialogTitle = Dict::S('UI:DashboardEdit:Title');
 		$sOkButtonLabel = Dict::S('UI:Button:Save');
 		$sCancelButtonLabel = Dict::S('UI:Button:Cancel');
@@ -1200,7 +1198,7 @@ EOF
 		$sTitle = json_encode($this->sTitle);
 		$sFile = json_encode($this->GetDefinitionFile());
 		$sUrl = utils::GetAbsoluteUrlAppRoot().'pages/ajax.render.php';
-		$sReloadURL = $this->GetReloadURL();
+		$sReloadURL = json_encode($this->GetReloadURL());
 
 		$sExitConfirmationMessage = addslashes(Dict::S('UI:NavigateAwayConfirmationMessage'));
 		$sCancelConfirmationMessage = addslashes(Dict::S('UI:CancelConfirmationMessage'));

application/utils.inc.php

@@ -432,8 +432,8 @@ class utils
 
 			// For URL
 			case static::ENUM_SANITIZATION_FILTER_URL:
-                // N°6350 - returns only valid URLs
-				$retValue = filter_var($value, FILTER_VALIDATE_URL);
+				$retValue = filter_var($value, FILTER_SANITIZE_URL);
+				$retValue = filter_var($retValue, FILTER_VALIDATE_URL);
 				break;
 
 			default:

core/attributedef.class.inc.php

@@ -138,7 +138,7 @@ abstract class AttributeDefinition
 
 	protected $aCSSClasses;
 
-	public function GetType()
+    public function GetType()
 	{
 		return Dict::S('Core:'.get_class($this));
 	}
@@ -3775,7 +3775,7 @@ class AttributeFinalClass extends AttributeString
  */
 class AttributePassword extends AttributeString implements iAttributeNoGroupBy
 {
-	const SEARCH_WIDGET_TYPE = self::SEARCH_WIDGET_TYPE_RAW;
+    const SEARCH_WIDGET_TYPE = self::SEARCH_WIDGET_TYPE_RAW;
 
 	/**
 	 * Useless constructor, but if not present PHP 7.4.0/7.4.1 is crashing :( (N°2329)
@@ -3851,7 +3851,7 @@ class AttributePassword extends AttributeString implements iAttributeNoGroupBy
  */
 class AttributeEncryptedString extends AttributeString implements iAttributeNoGroupBy
 {
-	const SEARCH_WIDGET_TYPE = self::SEARCH_WIDGET_TYPE_RAW;
+    const SEARCH_WIDGET_TYPE = self::SEARCH_WIDGET_TYPE_RAW;
 
 	static $sKey = null; // Encryption key used for all encrypted fields
 	static $sLibrary = null; // Encryption library used for all encrypted fields
@@ -9243,7 +9243,7 @@ class AttributeSubItem extends AttributeDefinition
  */
 class AttributeOneWayPassword extends AttributeDefinition implements iAttributeNoGroupBy
 {
-	const SEARCH_WIDGET_TYPE = self::SEARCH_WIDGET_TYPE_RAW;
+    const SEARCH_WIDGET_TYPE = self::SEARCH_WIDGET_TYPE_RAW;
 
 	/**
 	 * Useless constructor, but if not present PHP 7.4.0/7.4.1 is crashing :( (N°2329)

core/restservices.class.inc.php

@@ -34,7 +34,9 @@
  */
 class ObjectResult
 {
-	public $code;
+    use SanitizeTrait;
+
+    public $code;
 	public $message;
 	public $class;
 	public $key;
@@ -122,6 +124,19 @@ class ObjectResult
 	{
 		$this->fields[$sAttCode] = $this->MakeResultValue($oObject, $sAttCode, $bExtendedOutput);
 	}
+	
+public function SanitizeContent()
+	{
+		foreach($this->fields as $sFieldAttCode => $fieldValue)
+		{
+            try {
+			$oAttDef = MetaModel::GetAttributeDef($this->class, $sFieldAttCode);
+            } catch (Exception $e) { // for special cases like ID
+                continue;
+            }
+                $this->SanitizeFieldIfSensitive($this->fields, $sFieldAttCode, $fieldValue, $oAttDef);
+		}
+	}
 }
 
 
@@ -181,6 +196,16 @@ class RestResultWithObjects extends RestResult
 		$sObjKey = get_class($oObject).'::'.$oObject->GetKey();
 		$this->objects[$sObjKey] = $oObjRes;
 	}
+	
+public function SanitizeContent()
+	{
+		parent::SanitizeContent();
+		
+		foreach($this->objects as $sObjKey => $oObjRes)
+		{
+			$oObjRes->SanitizeContent();
+		}
+	}
 }
 
 class RestResultWithRelations extends RestResultWithObjects
@@ -247,9 +272,10 @@ class RestDelete
  *
  * @package     Core
  */
-class CoreServices implements iRestServiceProvider
+class CoreServices implements iRestServiceProvider, iRestInputSanitizer
 {
-	/**
+    use SanitizeTrait;
+    /**
 	 * Enumerate services delivered by this class
 	 * 	 
 	 * @param string $sVersion The version (e.g. 1.0) supported by the services
@@ -663,6 +689,33 @@ class CoreServices implements iRestServiceProvider
 		}
 		return $oResult;
 	}
+	
+	public function SanitizeJsonInput(string $sJsonInput): string
+	{
+        $sSanitizedJsonInput = $sJsonInput;
+        $aJsonData = json_decode($sSanitizedJsonInput, true);
+        $sOperation = $aJsonData['operation'];
+
+        switch ($sOperation) {
+            case 'core/check_credentials':
+                if (isset($aJsonData['password'])) {
+                    $aJsonData['password'] = '*****';
+                }
+                break;
+            case 'core/update':
+            case 'core/create':
+            default :
+            $sClass = $aJsonData['class'];
+            if (isset($aJsonData['fields'])) {
+                foreach ($aJsonData['fields'] as $sFieldAttCode => $fieldValue) {
+                    $oAttDef = MetaModel::GetAttributeDef($sClass, $sFieldAttCode);
+                    $this->SanitizeFieldIfSensitive($aJsonData['fields'], $sFieldAttCode, $fieldValue, $oAttDef);
+                }
+            }
+            break;
+        }
+		return json_encode($aJsonData, JSON_UNESCAPED_SLASHES | JSON_PRETTY_PRINT | JSON_UNESCAPED_UNICODE);
+	}
 
 	/**
 	 * Helper for object deletion	
@@ -802,3 +855,50 @@ class CoreServices implements iRestServiceProvider
 		return $iLimit * max(0, $iPage - 1);
 	}
 }
+
+trait SanitizeTrait
+{
+    /**
+     * Sanitize a field if it is sensitive.
+     *
+     * @param array $fields The fields array
+     * @param string $sFieldAttCode The attribute code
+     * @param mixed $oAttDef The attribute definition
+     * @throws Exception
+     */
+    private function SanitizeFieldIfSensitive(array &$fields, string $sFieldAttCode, $fieldValue, $oAttDef): void
+    {
+        // for simple attribute
+        if ($oAttDef instanceof iAttributeNoGroupBy) // iAttributeNoGroupBy is equivalent to sensitive attribute
+        {
+            $fields[$sFieldAttCode] = '*****';
+            return;
+        }
+        // for 1-n / n-n relation
+        if ($oAttDef instanceof AttributeLinkedSet) {
+            foreach ($fieldValue as $i => $aLnkValues) {
+                foreach ($aLnkValues as $sLnkAttCode => $sLnkValue) {
+                    $oLnkAttDef = MetaModel::GetAttributeDef($oAttDef->GetLinkedClass(), $sLnkAttCode);
+                    if ($oLnkAttDef instanceof iAttributeNoGroupBy) { // 1-n relation
+                        $fields[$sFieldAttCode][$i][$sLnkAttCode] = '*****';
+                    }
+                    elseif ($oAttDef instanceof AttributeLinkedSetIndirect && $oLnkAttDef instanceof AttributeExternalField) { // for n-n relation
+                        $oExtKeyAttDef = MetaModel::GetAttributeDef($oLnkAttDef->GetTargetClass(), $oLnkAttDef->GetExtAttCode());
+                        if ($oExtKeyAttDef instanceof iAttributeNoGroupBy) {
+                            $fields[$sFieldAttCode][$i][$sLnkAttCode] = '*****';
+                        }
+                    }
+                }
+            }
+            return;
+        }
+
+        // for external attribute
+        if ($oAttDef instanceof AttributeExternalField) {
+            $oExtKeyAttDef = MetaModel::GetAttributeDef($oAttDef->GetTargetClass(), $oAttDef->GetExtAttCode());
+            if ($oExtKeyAttDef instanceof iAttributeNoGroupBy) {
+                $fields[$sFieldAttCode] = '*****';
+            }
+        }
+    }
+}

datamodels/2.x/itop-backup/dbrestore.class.inc.php

@@ -53,14 +53,7 @@ class DBRestore extends DBBackup
 		$sUser = self::EscapeShellArg($this->sDBUser);
 		$sPwd = self::EscapeShellArg($this->sDBPwd);
 		$sDBName = self::EscapeShellArg($this->sDBName);
-		if (empty($this->sMySQLBinDir))
-		{
-			$sMySQLExe = 'mysql';
-		}
-		else
-		{
-			$sMySQLExe = '"'.$this->sMySQLBinDir.'/mysql"';
-		}
+		$sMySQLExe = DBBackup::MakeSafeMySQLCommand($this->sMySQLBinDir, 'mysql');
 		if (is_null($this->iDBPort))
 		{
 			$sPortOption = '';

datamodels/2.x/itop-backup/status.php

@@ -56,15 +56,7 @@ try
 	//
 	$sMySQLBinDir = MetaModel::GetConfig()->GetModuleSetting('itop-backup', 'mysql_bindir', '');
 	$sMySQLBinDir = utils::ReadParam('mysql_bindir', $sMySQLBinDir, true);
-	if (empty($sMySQLBinDir))
-	{
-		$sMySQLDump = 'mysqldump';
-	}
-	else
-	{
-		//echo 'Info - Found mysql_bindir: '.$sMySQLBinDir;
-		$sMySQLDump = '"'.$sMySQLBinDir.'/mysqldump"';
-	}
+	$sMySQLDump = DBBackup::MakeSafeMySQLCommand($sMySQLBinDir, 'mysqldump');
 	$sCommand = "$sMySQLDump -V 2>&1";
 
 	$aOutput = array();

datamodels/2.x/itop-portal-base/portal/src/Form/ObjectFormManager.php

@@ -1107,15 +1107,17 @@ class ObjectFormManager extends FormManager
         // First we need to update the Object with its new values in order to enable the dependents fields to update
         $sObjectClass = get_class($this->oObject);
         foreach ($aCurrentValues as $sAttCode => $value) {
-            if (!array_key_exists($sAttCode, $this->aFieldsAtts)) {
-                continue;
-            }
-            $iAttributeFlags = $this->aFieldsAtts[$sAttCode];
-            if ($iAttributeFlags & OPT_ATT_HIDDEN) {
-                continue;
-            }
-            if ($iAttributeFlags & OPT_ATT_READONLY) {
-                continue;
+            if (count($this->aFieldsAtts) !== 0) {
+                if (!array_key_exists($sAttCode, $this->aFieldsAtts)) {
+                    continue;
+                }
+                $iAttributeFlags = $this->aFieldsAtts[$sAttCode];
+                if ($iAttributeFlags & OPT_ATT_HIDDEN) {
+                    continue;
+                }
+                if ($iAttributeFlags & OPT_ATT_READONLY) {
+                    continue;
+                }
             }
 
             if (MetaModel::IsValidAttCode($sObjectClass, $sAttCode)) {
@@ -1230,55 +1232,56 @@ class ObjectFormManager extends FormManager
         $this->aFieldsAtts = array();
         $this->aExtraData = array();
         $aFieldsDMOnlyAttCodes = array();
-        switch ($this->aFormProperties['type']) {
-            case 'custom_list':
-            case 'static':
-                foreach ($this->aFormProperties['fields'] as $sAttCode => $aOptions) {
-                    // When in a transition and no flags are specified for the field, we will retrieve its flags from DM later
-                    if ($this->IsTransitionForm() && empty($aOptions)) {
-                        $aFieldsDMOnlyAttCodes[] = $sAttCode;
-                        continue;
-                    }
+        if (array_key_exists('type', $this->aFormProperties)) {
+            switch ($this->aFormProperties['type']) {
+                case 'custom_list':
+                case 'static':
+                    foreach ($this->aFormProperties['fields'] as $sAttCode => $aOptions) {
+                        // When in a transition and no flags are specified for the field, we will retrieve its flags from DM later
+                        if ($this->IsTransitionForm() && empty($aOptions)) {
+                            $aFieldsDMOnlyAttCodes[] = $sAttCode;
+                            continue;
+                        }
 
-                    // Otherwise we proceed as usual
-                    $iFieldFlags = OPT_ATT_NORMAL;
-                    // Checking if field should be slave
-                    if (isset($aOptions['slave']) && ($aOptions['slave'] === true)) {
-                        $iFieldFlags = $iFieldFlags | OPT_ATT_SLAVE;
+                        // Otherwise we proceed as usual
+                        $iFieldFlags = OPT_ATT_NORMAL;
+                        // Checking if field should be slave
+                        if (isset($aOptions['slave']) && ($aOptions['slave'] === true)) {
+                            $iFieldFlags = $iFieldFlags | OPT_ATT_SLAVE;
+                        }
+                        // Checking if field should be must_change
+                        if (isset($aOptions['must_change']) && ($aOptions['must_change'] === true)) {
+                            $iFieldFlags = $iFieldFlags | OPT_ATT_MUSTCHANGE;
+                        }
+                        // Checking if field should be must prompt
+                        if (isset($aOptions['must_prompt']) && ($aOptions['must_prompt'] === true)) {
+                            $iFieldFlags = $iFieldFlags | OPT_ATT_MUSTPROMPT;
+                        }
+                        // Checking if field should be hidden
+                        if (isset($aOptions['hidden']) && ($aOptions['hidden'] === true)) {
+                            $iFieldFlags = $iFieldFlags | OPT_ATT_HIDDEN;
+                        }
+                        // Checking if field should be readonly
+                        if (isset($aOptions['read_only']) && ($aOptions['read_only'] === true)) {
+                            $iFieldFlags = $iFieldFlags | OPT_ATT_READONLY;
+                        }
+                        // Checking if field should be mandatory
+                        if (isset($aOptions['mandatory']) && ($aOptions['mandatory'] === true)) {
+                            $iFieldFlags = $iFieldFlags | OPT_ATT_MANDATORY;
+                        }
+                        // Finally, adding the attribute and its flags
+                        $this->aFieldsAtts[$sAttCode] = $iFieldFlags;
                     }
-                    // Checking if field should be must_change
-                    if (isset($aOptions['must_change']) && ($aOptions['must_change'] === true)) {
-                        $iFieldFlags = $iFieldFlags | OPT_ATT_MUSTCHANGE;
-                    }
-                    // Checking if field should be must prompt
-                    if (isset($aOptions['must_prompt']) && ($aOptions['must_prompt'] === true)) {
-                        $iFieldFlags = $iFieldFlags | OPT_ATT_MUSTPROMPT;
-                    }
-                    // Checking if field should be hidden
-                    if (isset($aOptions['hidden']) && ($aOptions['hidden'] === true)) {
-                        $iFieldFlags = $iFieldFlags | OPT_ATT_HIDDEN;
-                    }
-                    // Checking if field should be readonly
-                    if (isset($aOptions['read_only']) && ($aOptions['read_only'] === true)) {
-                        $iFieldFlags = $iFieldFlags | OPT_ATT_READONLY;
-                    }
-                    // Checking if field should be mandatory
-                    if (isset($aOptions['mandatory']) && ($aOptions['mandatory'] === true)) {
-                        $iFieldFlags = $iFieldFlags | OPT_ATT_MANDATORY;
-                    }
-                    // Finally, adding the attribute and its flags
-                    $this->aFieldsAtts[$sAttCode] = $iFieldFlags;
-                }
-                break;
+                    break;
 
-            case 'zlist':
-                foreach (MetaModel::FlattenZList(MetaModel::GetZListItems($sObjectClass, $this->aFormProperties['fields'])) as $sAttCode) {
-                    $this->aFieldsAtts[$sAttCode] = OPT_ATT_NORMAL;
-                }
-                break;
+                case 'zlist':
+                    foreach (MetaModel::FlattenZList(MetaModel::GetZListItems($sObjectClass, $this->aFormProperties['fields'])) as $sAttCode) {
+                        $this->aFieldsAtts[$sAttCode] = OPT_ATT_NORMAL;
+                    }
+                    break;
+            }
         }
-
-        if ($this->aFormProperties['layout'] !== null) {
+        if (isset($this->aFormProperties['layout'])) {
             $oXPath = new DOMXPath($this->oHtmlDocument);
             /** @var \DOMElement $oFieldNode */
             foreach ($oXPath->query('//div[contains(@class, "form_field")][@data-field-id]') as $oFieldNode) {
@@ -1337,7 +1340,7 @@ class ObjectFormManager extends FormManager
         // Also, retrieving mandatory attributes from metamodel to be able to complete the form with them if necessary
         //
         // Note: When in a transition, we don't do this for fields that should be set from DM
-        if ($this->aFormProperties['type'] !== 'static') {
+        if (array_key_exists('type', $this->aFormProperties) && $this->aFormProperties['type'] !== 'static') {
             if ($this->IsTransitionForm()) {
                 $aDatamodelAttCodes = $this->oObject->GetTransitionAttributes($this->aFormProperties['stimulus_code']);
             }
@@ -1448,7 +1451,7 @@ class ObjectFormManager extends FormManager
 
         // - The layout
         $this->oHtmlDocument = new DOMDocument();
-        if ($this->aFormProperties['layout'] !== null) {
+        if (isset($this->aFormProperties['layout'])) {
             // Checking if we need to render the template from twig to html in order to parse the fields
             if ($this->aFormProperties['layout']['type'] === 'twig') {
                 if ($this->oContainer !== null) {

lib/composer/autoload_classmap.php

@@ -1218,6 +1218,7 @@ return array(
     'SQLQuery' => $baseDir . '/core/sqlquery.class.inc.php',
     'SQLUnionQuery' => $baseDir . '/core/sqlunionquery.class.inc.php',
     'SVGDOMSanitizer' => $baseDir . '/core/htmlsanitizer.class.inc.php',
+    'SanitizeTrait' => $baseDir . '/core/restservices.class.inc.php',
     'ScalarExpression' => $baseDir . '/core/oql/expression.class.inc.php',
     'ScalarOqlExpression' => $baseDir . '/core/oql/oqlquery.class.inc.php',
     'ScssPhp\\ScssPhp\\Base\\Range' => $vendorDir . '/scssphp/scssphp/src/Base/Range.php',
@@ -2732,6 +2733,7 @@ return array(
     'iPreferencesExtension' => $baseDir . '/application/applicationextension.inc.php',
     'iProcess' => $baseDir . '/core/backgroundprocess.inc.php',
     'iQueryModifier' => $baseDir . '/core/querymodifier.class.inc.php',
+    'iRestInputSanitizer' => $baseDir . '/application/applicationextension.inc.php',
     'iRestServiceProvider' => $baseDir . '/application/applicationextension.inc.php',
     'iScheduledProcess' => $baseDir . '/core/backgroundprocess.inc.php',
     'iSelfRegister' => $baseDir . '/core/userrights.class.inc.php',

lib/composer/autoload_static.php

@@ -1586,6 +1586,7 @@ class ComposerStaticInit0018331147de7601e7552f7da8e3bb8b
         'SQLQuery' => __DIR__ . '/../..' . '/core/sqlquery.class.inc.php',
         'SQLUnionQuery' => __DIR__ . '/../..' . '/core/sqlunionquery.class.inc.php',
         'SVGDOMSanitizer' => __DIR__ . '/../..' . '/core/htmlsanitizer.class.inc.php',
+        'SanitizeTrait' => __DIR__ . '/../..' . '/core/restservices.class.inc.php',
         'ScalarExpression' => __DIR__ . '/../..' . '/core/oql/expression.class.inc.php',
         'ScalarOqlExpression' => __DIR__ . '/../..' . '/core/oql/oqlquery.class.inc.php',
         'ScssPhp\\ScssPhp\\Base\\Range' => __DIR__ . '/..' . '/scssphp/scssphp/src/Base/Range.php',
@@ -3100,6 +3101,7 @@ class ComposerStaticInit0018331147de7601e7552f7da8e3bb8b
         'iPreferencesExtension' => __DIR__ . '/../..' . '/application/applicationextension.inc.php',
         'iProcess' => __DIR__ . '/../..' . '/core/backgroundprocess.inc.php',
         'iQueryModifier' => __DIR__ . '/../..' . '/core/querymodifier.class.inc.php',
+        'iRestInputSanitizer' => __DIR__ . '/../..' . '/application/applicationextension.inc.php',
         'iRestServiceProvider' => __DIR__ . '/../..' . '/application/applicationextension.inc.php',
         'iScheduledProcess' => __DIR__ . '/../..' . '/core/backgroundprocess.inc.php',
         'iSelfRegister' => __DIR__ . '/../..' . '/core/userrights.class.inc.php',

pages/UI.php

@@ -1989,7 +1989,7 @@ catch(CoreException $e)
 	{
 		$oP->add("<h1>".Dict::S('UI:FatalErrorMessage')."</h1>\n");
 	}	
-	$oP->error(Dict::Format('UI:Error_Details', $e->getHtmlDesc()));	
+	$oP->error(Dict::Format('UI:Error_Details', Str::pure2html($e->getHtmlDesc())));
 	$oP->output();
 
 	if (MetaModel::IsLogEnabledIssue())
@@ -2025,7 +2025,7 @@ catch(Exception $e)
 	require_once(APPROOT.'/setup/setuppage.class.inc.php');
 	$oP = new ErrorPage(Dict::S('UI:PageTitle:FatalError'));
 	$oP->add("<h1>".Dict::S('UI:FatalErrorMessage')."</h1>\n");	
-	$oP->error(Dict::Format('UI:Error_Details', $e->getMessage()));
+	$oP->error(Dict::Format('UI:Error_Details', Str::pure2html($e->getMessage())));
 	$oP->output();
 
 	if (MetaModel::IsLogEnabledIssue())

setup/backup.class.inc.php

@@ -104,6 +104,8 @@ class DBBackup
 	/** @var string */
 	protected $sDBName;
 	/** @var string */
+	protected $sMySQLBinDir = '';
+	/** @var string */
 	protected $sDBSubName;
 
 	/**
@@ -131,7 +133,6 @@ class DBBackup
 		$this->sDBSubName = $oConfig->get('db_subname');
 	}
 
-	protected $sMySQLBinDir = '';
 
 	/**
 	 * Create a normalized backup name, depending on the current date/time and Database
@@ -299,8 +300,9 @@ class DBBackup
 		}
 
 		$this->LogInfo("Starting backup of $this->sDBHost/$this->sDBName(suffix:'$this->sDBSubName')");
+		$sMySQLBinDir = utils::ReadParam('mysql_bindir', $this->sMySQLBinDir, true);
 
-		$sMySQLDump = $this->GetMysqldumpCommand();
+		$sMySQLDump = $this->MakeSafeMySQLCommand($sMySQLBinDir, 'mysqldump');
 
 		// Store the results in a temporary file
 		$sTmpFileName = self::EscapeShellArg($sBackupFileName);
@@ -557,20 +559,22 @@ EOF;
 
 	/**
 	 * @return string the command to launch mysqldump (without its params)
+	 * @throws \BackupException
 	 */
-	private function GetMysqldumpCommand()
+	public static function MakeSafeMySQLCommand($sMySQLBinDir, string $sCmd)
 	{
-		$sMySQLBinDir = utils::ReadParam('mysql_bindir', $this->sMySQLBinDir, true);
-		if (empty($sMySQLBinDir))
-		{
-			$sMysqldumpCommand = 'mysqldump';
+		if (empty($sMySQLBinDir)) {
+			$sMySQLCommand = $sCmd;
 		}
-		else
-		{
-			$sMysqldumpCommand = '"'.$sMySQLBinDir.'/mysqldump"';
+		else {
+			$sMySQLBinDir = escapeshellcmd($sMySQLBinDir);
+			$sMySQLCommand = '"'.$sMySQLBinDir.'/$sCmd"';
+			if (!file_exists($sMySQLCommand)) {
+				throw new BackupException("$sCmd not found in $sMySQLBinDir");
+			}
 		}
 
-		return $sMysqldumpCommand;
+		return $sMySQLCommand;
 	}
 }
 

setup/setuputils.class.inc.php

@@ -484,16 +484,15 @@ class SetupUtils
 		{
 			$sMySQLBinDir = MetaModel::GetConfig()->GetModuleSetting('itop-backup', 'mysql_bindir', '');
 		}
-
-		if (empty($sMySQLBinDir))
-		{
-			$sMySQLDump = 'mysqldump';
-		}
-		else
-		{
-			SetupPage::log('Info - Found mysql_bindir: '.$sMySQLBinDir);
-			$sMySQLDump = '"'.$sMySQLBinDir.'/mysqldump"';
+		try {
+			$sMySQLDump = DBBackup::MakeSafeMySQLCommand($sMySQLBinDir, 'mysqldump');
+		} catch (Exception $e) {
+			$aResult[] = new CheckResult(CheckResult::ERROR, $e->getMessage());
+			return $aResult;
 		}
+        if (!empty($sMySQLBinDir)) {
+            SetupPage::log('Info - Found mysql_bindir: '.$sMySQLBinDir);
+        }
 		$sCommand = "$sMySQLDump -V 2>&1";
 
 		$aOutput = array();

sources/renderer/bootstrap/fieldrenderer/bslinkedsetfieldrenderer.class.inc.php

@@ -611,7 +611,7 @@ JS
 					if ($oAttDef->IsExternalKey())
 					{
 						/** @var \AttributeExternalKey $oAttDef */
-						$aAttProperties['value'] = $oRemoteItem->Get($sAttCode . '_friendlyname');
+						$aAttProperties['value'] = \Str::pure2html($oRemoteItem->Get($sAttCode . '_friendlyname'));
 
 						// Checking if user can access object's external key
 						$sObjectUrl = ApplicationContext::MakeObjectUrl($oAttDef->GetTargetClass(), $oRemoteItem->Get($sAttCode));

tests/php-unit-tests/unitary-tests/application/utilsTest.php

@@ -495,7 +495,7 @@ class utilsTest extends ItopTestCase
 			'good element_identifier' => ['element_identifier', 'AD05nb', 'AD05nb'],
 			'bad element_identifier' => ['element_identifier', 'AD05nb+', 'AD05nb'],
 			'good url' => ['url', 'https://www.w3schools.com', 'https://www.w3schools.com'],
-			'bad url' => ['url', 'https://www.w3schoo<EFBFBD><EFBFBD>ls.co<EFBFBD>m', null],
+			'bad url' => ['url', 'https//www.w3schools.com', null],
 			'raw_data' => ['raw_data', '<Test>\s😃😃😃', '<Test>\s😃😃😃'],
 		];
 	}

tests/php-unit-tests/unitary-tests/core/Delta/delta_test_sanitize_output.xml

@@ -0,0 +1,116 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<itop_design xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" version="1.7">
+    <classes>
+        <class id="TestServer" _delta="define">
+            <parent>cmdbAbstractObject</parent>
+            <properties>
+                <category>bizmodel</category>
+                <abstract>false</abstract>
+                <key_type>autoincrement</key_type>
+                <db_table>test_server</db_table>
+                <db_key_field>id</db_key_field>
+            </properties>
+            <presentation/>
+            <methods/>
+            <fields>
+                <field id="contact_list" xsi:type="AttributeLinkedSetIndirect">
+                    <linked_class>lnkContactTestToServer</linked_class>
+                    <ext_key_to_me>test_server_id</ext_key_to_me>
+                    <ext_key_to_remote>contact_test_id</ext_key_to_remote>
+                    <is_null_allowed>true</is_null_allowed>
+                </field>
+                <field id="password_list" xsi:type="AttributeLinkedSet">
+                    <linked_class>PasswordTest</linked_class>
+                    <ext_key_to_me>server_test_id</ext_key_to_me>
+                    <is_null_allowed>true</is_null_allowed>
+                </field>
+                <field id="name" xsi:type="AttributeString">
+                    <sql>name</sql>
+                    <default_value/>
+                    <is_null_allowed>false</is_null_allowed>
+                </field>
+            </fields>
+        </class>
+
+
+        <class id="ContactTest" _delta="define">
+            <parent>cmdbAbstractObject</parent>
+            <properties>
+                <category>bizmodel</category>
+                <abstract>false</abstract>
+                <key_type>autoincrement</key_type>
+                <db_table>contact_test</db_table>
+                <db_key_field>id</db_key_field>
+            </properties>
+            <presentation/>
+            <methods/>
+            <fields>
+                <field id="password" xsi:type="AttributeEncryptedString">
+                    <sql>password</sql>
+                </field>
+                <field id="server_test_list" xsi:type="AttributeLinkedSetIndirect">
+                    <linked_class>lnkContactTestToServer</linked_class>
+                    <ext_key_to_me>contact_test_id</ext_key_to_me>
+                    <ext_key_to_remote>test_server_id</ext_key_to_remote>
+                    <is_null_allowed>true</is_null_allowed>
+                </field>
+            </fields>
+        </class>
+
+
+        <class id="lnkContactTestToServer" _delta="define">
+            <parent>cmdbAbstractObject</parent>
+            <properties>
+                <category>bizmodel</category>
+                <abstract>false</abstract>
+                <key_type>autoincrement</key_type>
+                <db_table>lnk_contact_server_test</db_table>
+                <db_key_field>id</db_key_field>
+            </properties>
+            <presentation/>
+            <methods/>
+            <fields>
+                <field id="contact_test_password" xsi:type="AttributeExternalField" _delta="define">
+                    <extkey_attcode>contact_test_id</extkey_attcode>
+                    <target_attcode>password</target_attcode>
+                </field>
+                <field id="test_server_id" xsi:type="AttributeExternalKey" _delta="define">
+                    <target_class>TestServer</target_class>
+                    <on_target_delete>DEL_MANUAL</on_target_delete>
+                    <sql>test_server</sql>
+                    <is_null_allowed>false</is_null_allowed>
+
+                </field>
+                <field id="contact_test_id" xsi:type="AttributeExternalKey" _delta="define">
+                    <target_class>ContactTest</target_class>
+                    <on_target_delete>DEL_MANUAL</on_target_delete>
+                    <sql>contact_test</sql>
+                    <is_null_allowed>false</is_null_allowed>
+
+                </field>
+            </fields>
+        </class>
+        <class id="PasswordTest" _delta="define">
+            <parent>cmdbAbstractObject</parent>
+            <properties>
+                <category>bizmodel</category>
+                <abstract>false</abstract>
+                <key_type>autoincrement</key_type>
+                <db_table>password_test</db_table>
+                <db_key_field>id</db_key_field>
+            </properties>
+            <presentation/>
+            <methods/>
+            <fields>
+                <field id="server_test_id" xsi:type="AttributeExternalKey" _delta="define">
+                    <target_class>TestServer</target_class>
+                    <sql>server_test_id</sql>
+                    <on_target_delete>DEL_MANUAL</on_target_delete>
+                </field>
+                <field id="password" xsi:type="AttributeEncryptedString" _delta="define">
+                    <sql>password</sql>
+                </field>
+            </fields>
+        </class>
+    </classes>
+</itop_design>

tests/php-unit-tests/unitary-tests/core/RestServicesSanitizeOutputTest.php

@@ -0,0 +1,164 @@
+<?php
+declare(strict_types=1);
+
+namespace Combodo\iTop\Test\UnitTest\Core;
+
+use ArchivedObjectException;
+use AttributeEncryptedString;
+use Combodo\iTop\Test\UnitTest\ItopCustomDatamodelTestCase;
+use CoreException;
+use CoreUnexpectedValue;
+use Exception;
+use MetaModel;
+use ormLinkSet;
+use PasswordTest;
+use RestResultWithObjects;
+
+/**
+ * @runTestsInSeparateProcesses
+ * @preserveGlobalState disabled
+ * @backupGlobals disabled
+ */
+class RestServicesSanitizeOutputTest extends ItopCustomDatamodelTestCase
+{
+    private const SIMPLE_PASSWORD = '123456';
+
+    /**
+     * @throws Exception
+     */
+    protected function setUp(): void
+    {
+        parent::setUp();
+        // Workaround to cope with inconsistent settings in itop-config files from the CI
+        AttributeEncryptedString::$sKey = '6eb9d9afa3ee0fbcebe622a33bf57aaeafb7c37998fd24c403c2522c2d60117f';
+    }
+
+    /**
+     * @return void
+     * @throws CoreException
+     */
+    public function testSanitizeAttributeOnRequestedObject()
+    {
+        $oContactTest = MetaModel::NewObject('ContactTest', [
+                        'password' => self::SIMPLE_PASSWORD]
+        );
+        $oRestResultWithObject = new RestResultWithObjects();
+        $oRestResultWithObject->AddObject(0, 'ok', $oContactTest, ['ContactTest' => ['password']]);
+        $oRestResultWithObject->SanitizeContent();
+        static::assertEquals(
+                '{"objects":{"ContactTest::-1":{"code":0,"message":"ok","class":"ContactTest","key":-1,"fields":{"password":"*****"}}},"code":0,"message":null}',
+                json_encode($oRestResultWithObject));
+    }
+
+    /**
+     * @return void
+     * @throws Exception
+     */
+    public function testSanitizeAttributeExternalFieldOnLink()
+    {
+        $oContactTest = $this->createObject('ContactTest', [
+                        'password' => self::SIMPLE_PASSWORD]
+        );
+
+        $oTestServer = $this->createObject('TestServer', [
+                'name' => 'test_server',
+        ]);
+
+
+        // create lnkContactTestToServer
+        $oLnkContactTestToServer = $this->createObject('lnkContactTestToServer', [
+                'contact_test_id' => $oContactTest->GetKey(),
+                'test_server_id' => $oTestServer->GetKey()
+        ]);
+
+        $oRestResultWithObject = new RestResultWithObjects();
+        $oRestResultWithObject->AddObject(0, 'ok', $oLnkContactTestToServer,
+                ['lnkContactTestToServer' => ['contact_test_password']]);
+
+        $oRestResultWithObject->SanitizeContent();
+
+        static::assertContains(
+                '*****',
+                json_encode($oRestResultWithObject));
+
+        static::assertNotContains(
+                self::SIMPLE_PASSWORD,
+                json_encode($oRestResultWithObject));
+    }
+
+    /**
+     * @throws Exception
+     */
+    public function testSanitizeAttributeOnObjectRelatedThroughNNRelation()
+    {
+        $oContactTest = $this->createObject('ContactTest', [
+                'password' => self::SIMPLE_PASSWORD]);
+
+        $oTestServer = $this->createObject('TestServer', [
+                'name' => 'test_server',
+        ]);
+
+        // create lnkContactTestToServer
+        $this->createObject('lnkContactTestToServer', [
+                'contact_test_id' => $oContactTest->GetKey(),
+                'test_server_id' => $oTestServer->GetKey()
+        ]);
+
+        $oRestResultWithObject = new RestResultWithObjects();
+        $oRestResultWithObject->AddObject(0, 'ok', $oTestServer,
+                ['TestServer' => ['contact_list']]);
+
+        $oRestResultWithObject->SanitizeContent();
+        static::assertContains(
+                '*****',
+                json_encode($oRestResultWithObject));
+
+        static::assertNotContains(
+                self::SIMPLE_PASSWORD,
+                json_encode($oRestResultWithObject));
+    }
+
+
+    /**
+     * @throws CoreException
+     * @throws CoreUnexpectedValue
+     * @throws ArchivedObjectException
+     * @throws Exception
+     */
+    public function testSanitizeOnObjectRelatedThrough1NRelation()
+    {
+        $oTestServer = $this->createObject('TestServer', [
+                'name' => 'my_server',
+        ]);
+
+        $oPassword = new PasswordTest();
+        $oPassword->Set('password', self::SIMPLE_PASSWORD);
+        $oPassword->Set('server_test_id', $oTestServer->GetKey());
+
+        /** @var ormLinkSet $oContactList */
+        $oContactList = $oTestServer->Get('password_list');
+        $oContactList->AddItem($oPassword);
+        $oTestServer->Set('password_list', $oContactList);
+
+        $oRestResultWithObject = new RestResultWithObjects();
+        $oRestResultWithObject->AddObject(0, 'ok', $oTestServer, ['TestServer' => ['id', 'password_list']]);
+        $oRestResultWithObject->SanitizeContent();
+
+        static::assertContains(
+                '*****',
+                json_encode($oRestResultWithObject));
+
+        static::assertNotContains(
+                self::SIMPLE_PASSWORD,
+                json_encode($oRestResultWithObject));
+
+    }
+
+    /**
+     * @return string Abs path to the XML delta to use for the tests of that class
+     */
+    public function GetDatamodelDeltaAbsPath(): string
+    {
+        return __DIR__ . '/Delta/delta_test_sanitize_output.xml';
+    }
+}

tests/php-unit-tests/unitary-tests/core/RestServicesTest.php

@@ -0,0 +1,125 @@
+<?php
+declare(strict_types=1);
+
+namespace Combodo\iTop\Test\UnitTest\Core;
+
+use Combodo\iTop\Test\UnitTest\ItopDataTestCase;
+use CoreException;
+use CoreServices;
+use CoreUnexpectedValue;
+use RestResultWithObjects;
+use UserLocal;
+
+/**
+ * @runTestsInSeparateProcesses
+ * @preserveGlobalState disabled
+ * @backupGlobals disabled
+ */
+class RestServicesTest extends ItopDataTestCase
+{
+    /**
+     * @return void
+     * @dataProvider providerTestSanitizeJsonInput
+     */
+    public function testSanitizeJsonInput($sJsonData, $sExpectedJsonDataSanitized)
+    {
+        $oRS = new CoreServices();
+        $sOutputJson = $oRS->SanitizeJsonInput($sJsonData);
+        static::assertEquals($sExpectedJsonDataSanitized, $sOutputJson);
+    }
+
+    /**
+     * @return array[]
+     */
+    public function providerTestSanitizeJsonInput(): array
+    {
+        return [
+                'core/check_credentials' => [
+                        '{"operation": "core/check_credentials", "user": "admin", "password": "admin"}',
+                        '{
+    "operation": "core/check_credentials",
+    "user": "admin",
+    "password": "*****"
+}'
+                ],
+                'core/update' => [
+                        '{"operation": "core/update", "comment": "Update user", "class": "UserLocal", "key": {"id":1}, "output_fields": "first_name, password", "fields": {"password" : "123456"}}',
+                        '{
+    "operation": "core/update",
+    "comment": "Update user",
+    "class": "UserLocal",
+    "key": {
+        "id": 1
+    },
+    "output_fields": "first_name, password",
+    "fields": {
+        "password": "*****"
+    }
+}'
+                ],
+                'core/create' => [
+                        '{"operation": "core/create", "comment": "Create user", "class": "UserLocal", "fields": {"first_name": "John", "last_name": "Doe", "email": "jd@example/com", "password" : "123456"}}',
+                        '{
+    "operation": "core/create",
+    "comment": "Create user",
+    "class": "UserLocal",
+    "fields": {
+        "first_name": "John",
+        "last_name": "Doe",
+        "email": "jd@example/com",
+        "password": "*****"
+    }
+}'
+                ],
+        ];
+    }
+
+    /**
+     * @param $sOperation
+     * @param $aJsonData
+     * @param $sExpectedJsonDataSanitized
+     * @return void
+     * @throws CoreException
+     * @throws CoreUnexpectedValue
+     * @dataProvider providerTestSanitizeJsonOutput
+     */
+    public function testSanitizeJsonOutput($sOperation, $aJsonData, $sExpectedJsonDataSanitized)
+    {
+        $oUser = new UserLocal();
+        $oUser->Set('password', '123456');
+        $oRestResultWithObject = new RestResultWithObjects();
+        $oRestResultWithObject->AddObject(0, 'ok', $oUser, ['UserLocal' => ['login', 'password']]);
+        $oRestResultWithObject->SanitizeContent();
+        static::assertEquals($sExpectedJsonDataSanitized, json_encode($oRestResultWithObject));
+    }
+
+    /**
+     * @return array[]
+     */
+    public function providerTestSanitizeJsonOutput(): array
+    {
+        return [
+
+                'core/update' => [
+                        'core/update',
+                        ['comment' => 'Update user', 'class' => 'UserLocal', 'key' => ['login' => 'my_example'], 'output_fields' => 'password', 'fields' => ['password' => 'opkB!req57']],
+                        '{"objects":{"UserLocal::-1":{"code":0,"message":"ok","class":"UserLocal","key":-1,"fields":{"login":"","password":"*****"}}},"code":0,"message":null}'
+                ],
+                'core/create' => [
+                        'core/create',
+                        ['comment' => 'Create user', 'class' => 'UserLocal', 'fields' => ['password' => 'Azertyuiiop*12', 'login' => 'toto', 'profile_list' => [1]]],
+                        '{"objects":{"UserLocal::-1":{"code":0,"message":"ok","class":"UserLocal","key":-1,"fields":{"login":"","password":"*****"}}},"code":0,"message":null}'
+                ],
+                'core/get' => [
+                        'core/get',
+                        ['comment' => 'Get user', 'class' => 'UserLocal', 'key' => ['login' => 'my_example'], 'output_fields' => 'first_name, password'],
+                        '{"objects":{"UserLocal::-1":{"code":0,"message":"ok","class":"UserLocal","key":-1,"fields":{"login":"","password":"*****"}}},"code":0,"message":null}'
+                ],
+                'core/check_credentials' => [
+                        'core/check_credentials',
+                        ['user' => 'admin', 'password' => 'admin'],
+                        '{"objects":{"UserLocal::-1":{"code":0,"message":"ok","class":"UserLocal","key":-1,"fields":{"login":"","password":"*****"}}},"code":0,"message":null}'
+                ],
+        ];
+    }
+}

webservices/rest.php

@@ -209,7 +209,11 @@ try
 		/** @var iRestServiceProvider $oRS */
 		$oRS = $aOpToRestService[$sOperation]['service_provider'];
 		$sProvider = get_class($oRS);
-	
+		
+		if ($oRS instanceof iRestInputSanitizer) {
+			$sSanitizedJsonInput = $oRS->SanitizeJsonInput($sJsonString);
+		}
+
 		CMDBObject::SetTrackOrigin('webservice-rest');
 		$oResult = $oRS->ExecOperation($sVersion, $sOperation, $aJsonData);
 	}
@@ -234,6 +238,7 @@ catch(Exception $e)
 //
 $sResponse = json_encode($oResult);
 
+
 if ($sResponse === false)
 {
 	$oJsonIssue = new RestResult();
@@ -267,7 +272,7 @@ if (MetaModel::GetConfig()->Get('log_rest_service'))
 	$oLog->SetTrim('userinfo', UserRights::GetUser());
 	$oLog->Set('version', $sVersion);
 	$oLog->Set('operation', $sOperation);
-	$oLog->SetTrim('json_input', $sJsonString);
+    $oLog->SetTrim('json_input', $sSanitizedJsonInput ?? $sJsonString);
 
 	$oLog->Set('provider', $sProvider);
 	$sMessage = $oResult->message;
@@ -277,7 +282,8 @@ if (MetaModel::GetConfig()->Get('log_rest_service'))
 	}
 	$oLog->SetTrim('message', $sMessage);
 	$oLog->Set('code', $oResult->code);
-	$oLog->SetTrim('json_output', $sResponse);
+	$oResult->SanitizeContent();
+	$oLog->SetTrim('json_output', json_encode($oResult, JSON_UNESCAPED_SLASHES | JSON_PRETTY_PRINT | JSON_UNESCAPED_UNICODE));
 
 	$oLog->DBInsertNoReload();
 	$oKPI->ComputeAndReport('Log inserted');