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

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:

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😃😃😃'],
 		];
 	}

webservices/rest.php

@@ -213,10 +213,7 @@ try
 		if ($oRS instanceof iRestInputSanitizer) {
 			$sSanitizedJsonInput = $oRS->SanitizeJsonInput($sJsonString);
 		}
-		else {
-			$sSanitizedJsonInput = $sJsonString;
-		}
-	
+
 		CMDBObject::SetTrackOrigin('webservice-rest');
 		$oResult = $oRS->ExecOperation($sVersion, $sOperation, $aJsonData);
 	}
@@ -275,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', $sSanitizedJsonInput);
+    $oLog->SetTrim('json_input', $sSanitizedJsonInput ?? $sJsonString);
 
 	$oLog->Set('provider', $sProvider);
 	$sMessage = $oResult->message;