N°9379 - PHP unserialze function - security hardening

- typo

application/loginexternal.class.inc.php

@@ -75,10 +75,13 @@ class LoginExternal extends AbstractLoginFSMExtension
 	}
 
 	/**
-	 * @return bool|mixed
+	 * @return bool
 	 */
 	private function GetAuthUser()
 	{
-		return MetaModel::GetConfig()->GetExternalAuthenticationVariable();
+		$sExtAuthVar = MetaModel::GetConfig()->GetExternalAuthenticationVariable(); // In which variable is the info passed ?
+		eval('$sAuthUser = isset('.$sExtAuthVar.') ? '.$sExtAuthVar.' : false;'); // Retrieve the value
+		/** @var string $sAuthUser */
+		return $sAuthUser; // Retrieve the value
 	}
 }

application/menunode.class.inc.php

@@ -1546,7 +1546,16 @@ class ShortcutMenuNode extends MenuNode
 	public function GetHyperlink($aExtraParams)
 	{
 		$sContext = $this->oShortcut->Get('context');
-		$aContext = unserialize($sContext);
+		try {
+			$aContext = utils::Unserialize($sContext, ['allowed_classes' => false]);
+		} catch (Exception $e) {
+			IssueLog::Warning("User shortcut corrupted, delete the shortcut", LogChannels::CONSOLE, [
+				'shortcut_name' => $this->oShortcut->GetName(),
+				'root_cause' => $e->getMessage(),
+			]);
+			// delete the shortcut
+			$this->oShortcut->DBDelete();
+		}
 		if (isset($aContext['menu'])) {
 			unset($aContext['menu']);
 		}

application/utils.inc.php

@@ -3252,4 +3252,50 @@ TXT
 
 		return $aTrace;
 	}
+
+	/**
+	 * PHP unserialize encapsulation, allow throwing exception when not allowed object class is detected (for security hardening)
+	 *
+	 * @param mixed $data data to unserialize
+	 * @param array $aOptions PHP @unserialise options
+	 * @param bool $bThrowNotAllowedObjectClassException flag to throw exception
+	 *
+	 * @return mixed PHP @unserialise return
+	 * @throws Exception
+	 */
+	public static function Unserialize(mixed $data, array $aOptions, bool $bThrowNotAllowedObjectClassException = true): mixed
+	{
+		$data = unserialize($data, $aOptions);
+
+		if ($bThrowNotAllowedObjectClassException) {
+			try {
+				self::AssertNoIncompleteClassDetected($data);
+			} catch (Exception $e) {
+				throw new CoreException('Unserialization failed because an incomplete class was detected.', [], '', $e);
+			}
+		}
+
+		return $data;
+	}
+
+	/**
+	 * Assert that data provided doesn't contain any incomplete class.
+	 *
+	 * @throws Exception
+	 */
+	public static function AssertNoIncompleteClassDetected(mixed $data): void
+	{
+		if (is_object($data)) {
+			if ($data instanceof __PHP_Incomplete_Class) {
+				throw new Exception('__PHP_Incomplete_Class_Name object detected');
+			}
+			foreach (get_object_vars($data) as $property) {
+				self::AssertNoIncompleteClassDetected($property);
+			}
+		} elseif (is_array($data)) {
+			foreach ($data as $value) {
+				self::AssertNoIncompleteClassDetected($value);
+			}
+		}
+	}
 }

core/attributedef.class.inc.php

@@ -4829,7 +4829,7 @@ class AttributeCaseLog extends AttributeLongText
 		}
 
 		if (strlen($sIndex) > 0) {
-			$aIndex = unserialize($sIndex);
+			$aIndex = utils::Unserialize($sIndex, ['allowed_classes' => false], false);
 			$value = new ormCaseLog($sLog, $aIndex);
 		} else {
 			$value = new ormCaseLog($sLog);

core/config.class.inc.php

@@ -75,7 +75,6 @@ define('DEFAULT_EXT_AUTH_VARIABLE', '$_SERVER[\'REMOTE_USER\']');
 define('DEFAULT_ENCRYPTION_KEY', '@iT0pEncr1pti0n!'); // We'll use a random generated key later (if possible)
 define('DEFAULT_ENCRYPTION_LIB', 'Mcrypt'); // We'll define the best encryption available later
 define('DEFAULT_HASH_ALGO', PASSWORD_DEFAULT);
-
 /**
  * Config
  * configuration data (this class cannot not be localized, because it is responsible for loading the dictionaries)
@@ -870,14 +869,6 @@ class Config
 			'source_of_value' => '',
 			'show_in_conf_sample' => false,
 		],
-		'ext_auth_variable' => [
-			'type' => 'string',
-			'description' => 'External authentication expression (allowed: $_SERVER[\'key\'], $_COOKIE[\'key\'], $_REQUEST[\'key\'], getallheaders()[\'Header-Name\'])',
-			'default' => '$_SERVER[\'REMOTE_USER\']',
-			'value' => '$_SERVER[\'REMOTE_USER\']',
-			'source_of_value' => '',
-			'show_in_conf_sample' => false,
-		],
 		'login_debug' => [
 			'type' => 'bool',
 			'description' => 'Activate the login FSM debug',
@@ -1612,7 +1603,7 @@ class Config
 			'show_in_conf_sample' => false,
 		],
 		'search_manual_submit' => [
-			'type' => 'bool',
+			'type' => 'array',
 			'description' => 'Force manual submit of search all requests',
 			'default' => false,
 			'value' => true,
@@ -1975,6 +1966,11 @@ class Config
 	 */
 	protected $m_sAllowedLoginTypes;
 
+	/**
+	 * @var string Name of the PHP variable in which external authentication information is passed by the web server
+	 */
+	protected $m_sExtAuthVariable;
+
 	/**
 	 * @var string Encryption key used for all attributes of type "encrypted string". Can be set to a random value
 	 *             unless you want to import a database from another iTop instance, in which case you must use
@@ -2047,6 +2043,7 @@ class Config
 		$this->m_bSecureConnectionRequired = DEFAULT_SECURE_CONNECTION_REQUIRED;
 		$this->m_sDefaultLanguage = 'EN US';
 		$this->m_sAllowedLoginTypes = DEFAULT_ALLOWED_LOGIN_TYPES;
+		$this->m_sExtAuthVariable = DEFAULT_EXT_AUTH_VARIABLE;
 		$this->m_aCharsets = [];
 		$this->m_bQueryCacheEnabled = DEFAULT_QUERY_CACHE_ENABLED;
 		$this->m_iPasswordHashAlgo = DEFAULT_HASH_ALGO;
@@ -2200,6 +2197,7 @@ class Config
 
 		$this->m_sDefaultLanguage = isset($MySettings['default_language']) ? trim($MySettings['default_language']) : 'EN US';
 		$this->m_sAllowedLoginTypes = isset($MySettings['allowed_login_types']) ? trim($MySettings['allowed_login_types']) : DEFAULT_ALLOWED_LOGIN_TYPES;
+		$this->m_sExtAuthVariable = isset($MySettings['ext_auth_variable']) ? trim($MySettings['ext_auth_variable']) : DEFAULT_EXT_AUTH_VARIABLE;
 		$this->m_sEncryptionKey = isset($MySettings['encryption_key']) ? trim($MySettings['encryption_key']) : $this->m_sEncryptionKey;
 		$this->m_sEncryptionLibrary = isset($MySettings['encryption_library']) ? trim($MySettings['encryption_library']) : $this->m_sEncryptionLibrary;
 		$this->m_aCharsets = isset($MySettings['csv_import_charsets']) ? $MySettings['csv_import_charsets'] : [];
@@ -2352,73 +2350,9 @@ class Config
 		return explode('|', $this->m_sAllowedLoginTypes);
 	}
 
-	/**
-	 * @return bool|mixed
-	 * @since 3.2.3 return the parsed value instead of an unsecured variable name
-	 */
 	public function GetExternalAuthenticationVariable()
 	{
-		$sExpression = $this->Get('ext_auth_variable');
-		$aParsed = $this->ParseExternalAuthVariableExpression($sExpression);
-		if ($aParsed === null) {
-			return false;
-		}
-
-		$sKey = $aParsed['key'];
-		switch ($aParsed['type']) {
-			case 'server':
-				return $_SERVER[$sKey] ?? false;
-			case 'cookie':
-				return $_COOKIE[$sKey] ?? false;
-			case 'request':
-				return $_REQUEST[$sKey] ?? false;
-			case 'header':
-				if (!function_exists('getallheaders')) {
-					return false;
-				}
-				$aHeaders = getallheaders();
-				if (!is_array($aHeaders)) {
-					return false;
-				}
-				return $aHeaders[$sKey] ?? false;
-		}
-		return false;
-	}
-
-	/**
-	 * @param $sExpression
-	 * @return array|null
-	 */
-	private function ParseExternalAuthVariableExpression($sExpression)
-	{
-		// If it's a configuration parameter it's probably already trimmed, but just in case
-		$sExpression = trim((string) $sExpression);
-		if ($sExpression === '') {
-			return null;
-		}
-
-		// Match $_SERVER/$_COOKIE/$_REQUEST['key'] with optional whitespace and single/double quotes.
-		if (preg_match('/^\$_(SERVER|COOKIE|REQUEST)\s*\[\s*(["\'])\s*([^"\']+)\2\s*\]\s*$/', $sExpression, $aMatches) === 1) {
-			$sContext = strtoupper($aMatches[1]);
-			$sKey = $aMatches[3];
-			return [
-				'type' => strtolower($sContext),
-				'key' => $sKey,
-				'normalized' => '$_'.$sContext.'[\''.$sKey.'\']',
-			];
-		}
-
-		// Match getallheaders()['Header-Name'] in a case-insensitive way.
-		if (preg_match('/^getallheaders\(\)\s*\[\s*(["\'])\s*([^"\']+)\1\s*\]\s*$/i', $sExpression, $aMatches) === 1) {
-			$sKey = $aMatches[2];
-			return [
-				'type' => 'header',
-				'key' => $sKey,
-				'normalized' => 'getallheaders()[\''.$sKey.'\']',
-			];
-		}
-
-		return null;
+		return $this->m_sExtAuthVariable;
 	}
 
 	public function GetCSVImportCharsets()
@@ -2514,7 +2448,7 @@ class Config
 
 	public function SetExternalAuthenticationVariable($sExtAuthVariable)
 	{
-		$this->Set('ext_auth_variable', $sExtAuthVariable);
+		$this->m_sExtAuthVariable = $sExtAuthVariable;
 	}
 
 	public function SetEncryptionKey($sKey)
@@ -2569,6 +2503,7 @@ class Config
 		$aSettings['secure_connection_required'] = $this->m_bSecureConnectionRequired;
 		$aSettings['default_language'] = $this->m_sDefaultLanguage;
 		$aSettings['allowed_login_types'] = $this->m_sAllowedLoginTypes;
+		$aSettings['ext_auth_variable'] = $this->m_sExtAuthVariable;
 		$aSettings['encryption_key'] = $this->m_sEncryptionKey;
 		$aSettings['encryption_library'] = $this->m_sEncryptionLibrary;
 		$aSettings['csv_import_charsets'] = $this->m_aCharsets;
@@ -2673,6 +2608,7 @@ class Config
 			$aOtherValues = [
 				'default_language' => $this->m_sDefaultLanguage,
 				'allowed_login_types' => $this->m_sAllowedLoginTypes,
+				'ext_auth_variable' => $this->m_sExtAuthVariable,
 				'encryption_key' => $this->m_sEncryptionKey,
 				'encryption_library' => $this->m_sEncryptionLibrary,
 				'csv_import_charsets' => $this->m_aCharsets,

css/backoffice/vendors/_bulma-variables-overload.scss

@@ -21,7 +21,6 @@ $text-strong: inherit !default;
  * See https://bulma.io/documentation/elements/content/
  */
 $content-block-margin-bottom: 0 !default;
-$content-blockquote-background-color: $ibo-color-grey-200 !default;
 
 /* Table: Reset style as much as possible to match rich text editor preview, which is the browser's default stylesheet.
  * As there is no way to avoid bulma rules, we simply make them invalid by setting an invalid variable value, the rules will then be ignored by the browser.

datamodels/2.x/combodo-backoffice-darkmoon-theme/scss/scss-variables.scss

@@ -53,7 +53,6 @@ $text-strong: inherit !default;
 $code: $ibo-color-primary-400 !default;
 $code-background: $ibo-color-grey-700 !default;
 $pre-background: $ibo-color-grey-600 !default;
-$content-blockquote-background-color: $ibo-color-grey-600 !default;
 
 $ibo-scrollbar--scrollbar-track-background-color: $ibo-color-grey-700 !default;
 $ibo-scrollbar--scrollbar-thumb-background-color: $ibo-color-grey-900 !default;
@@ -176,7 +175,6 @@ $ibo-input-select--action-button--color: $ibo-input-select-wrapper--after--color
 $ibo-input-select-selectize--item--active--text-color: $ibo-color-grey-100 !default;
 $ibo-input-select-selectize--item--active--background-color: $ibo-color-grey-500 !default;
 $ibo-input-select--autocomplete-item-image--background-color: $ibo-color-grey-800 !default;
-$ibo-input-date--button--color: $ibo-input-select--action-button--color;
 $ibo-vendors-selectize-input--color: $ibo-body-text-color !default;
 $ibo-vendors-selectize-input--background-color: $ibo-input--background-color !default;
 $ibo-vendors-selectize--input--border-color: $ibo-input--border-color !default;
@@ -224,11 +222,6 @@ $ibo-vendors-datatables--row-highlight--colors:('red': ($ibo-color-red-700),'dan
 
 $ibo-vendors-jqueryui--ui-dialog--background-color: $ibo-color-grey-800 !default;
 $ibo-vendors-jqueryui--ui-dialog-titlebar--border-bottom: solid 1px $ibo-color-grey-500 !default;
-$ibo-vendors-jqueryui--ui-datepicker--background-color: $ibo-color-grey-700 !default;
-$ibo-vendors-jqueryui--ui-datepicker-days--color: $ibo-color-primary-200 !default;
-$ibo-vendors-jqueryui--ui-datepicker-days--highlight--background-color: $ibo-color-primary-800 !default;
-$ibo-vendors-jqueryui--ui-datepicker-days--hover--background-color: $ibo-color-primary-500 !default;
-$ibo-vendors-jqueryui--ui-datepicker-days--active--background-color: $ibo-color-primary-700 !default;
 
 
 

js/forms-json-utils.js

@@ -301,112 +301,88 @@ function ValidateField(sFieldId, sPattern, bMandatory, sFormId, nullValue, origi
 	return true; // Do not stop propagation ??
 }
 
-function EvaluateCKEditorValidation(oOptions)
+function ValidateCKEditField(sFieldId, sPattern, bMandatory, sFormId, nullValue, originalValue)
 {
-	let oField = $('#'+oOptions.sFieldId);
+	let oField = $('#'+sFieldId);
 	if (oField.length === 0) {
 		return false;
 	}
 
-	let oCKEditor = CombodoCKEditorHandler.GetInstanceSynchronous('#'+oOptions.sFieldId);
-	let bValid = true;
-	let sExplain = '';
-	let sTextContent;
-	let sTextOriginalContents;
+	let oCKEditor = CombodoCKEditorHandler.GetInstanceSynchronous('#'+sFieldId);
 
+	var bValid;
+	var sExplain = '';
 	if (oField.prop('disabled')) {
 		bValid = true; // disabled fields are not checked
 	} else {
 		// If the CKEditor is not yet loaded, we need to wait for it to be ready
 		// but as we need this function to be synchronous, we need to call it again when the CKEditor is ready
 		if (oCKEditor === undefined){
-			CombodoCKEditorHandler.GetInstance('#'+oOptions.sFieldId).then((oCKEditor) => {
-				oOptions.onRetry();
+			CombodoCKEditorHandler.GetInstance('#'+sFieldId).then((oCKEditor) => {
+				ValidateCKEditField(sFieldId, sPattern, bMandatory, sFormId, nullValue, originalValue);
 			});
-			return false;
+			return;
 		}
-
+		let sTextContent;
 		let sFormattedContent = oCKEditor.getData();
+
+		// Get the contents without the tags
+		// Check if we have a formatted content that is HTML, otherwise we just have plain text, and we can use it directly
 		sTextContent = $(sFormattedContent).length > 0 ? $(sFormattedContent).text() : sFormattedContent;
 
-		if (sTextContent === '')
-		{
+		if (sTextContent === '') {
 			// No plain text, maybe there is just an image
-			let oImg = $(sFormattedContent).find('img');
-			if (oImg.length !== 0)
-			{
+			let oImg = $(sFormattedContent).find("img");
+			if (oImg.length !== 0) {
 				sTextContent = 'image';
 			}
 		}
 
-		let oFormattedOriginalContents = (oOptions.sOriginalValue !== undefined) ? $('<div></div>').html(oOptions.sOriginalValue) : undefined;
-		sTextOriginalContents = (oFormattedOriginalContents !== undefined) ? oFormattedOriginalContents.text() : undefined;
+		// Get the original value without the tags
+		let oFormattedOriginalContents = (originalValue !== undefined) ? $('<div></div>').html(originalValue) : undefined;
+		let sTextOriginalContents = (oFormattedOriginalContents !== undefined) ? oFormattedOriginalContents.text() : undefined;
 
-		if (oOptions.validate !== undefined) {
-			let oValidation = oOptions.validate(sTextContent, sTextOriginalContents);
-			bValid = oValidation.bValid;
-			sExplain = oValidation.sExplain;
+		if (bMandatory && (sTextContent === nullValue)) {
+			bValid = false;
+			sExplain = Dict.S('UI:ValueMustBeSet');
+		} else if ((sTextOriginalContents !== undefined) && (sTextContent === sTextOriginalContents)) {
+			bValid = false;
+			if (sTextOriginalContents === nullValue) {
+				sExplain = Dict.S('UI:ValueMustBeSet');
+			} else {
+				// Note: value change check is not working well yet as the HTML to Text conversion is not exactly the same when done from the PHP value or the CKEditor value.
+				sExplain = Dict.S('UI:ValueMustBeChanged');
+			}
+		} else {
+			bValid = true;
 		}
-
-		// Put an event to check the field when the content changes, remove the event right after as we'll call this same function again, and we don't want to call the event more than once (especially not ^2 times on each call)
+		
+		// Put and event to check the field when the content changes, remove the event right after as we'll call this same function again, and we don't want to call the event more than once (especially not ^2 times on each call)
 		oCKEditor.model.document.once('change:data', (event) => {
-			oOptions.onChange();
+			ValidateCKEditField(sFieldId, sPattern, bMandatory, sFormId, nullValue, originalValue);
 		});
 	}
-
-	ReportFieldValidationStatus(oOptions.sFieldId, oOptions.sFormId, bValid, sExplain);
+	
+	ReportFieldValidationStatus(sFieldId, sFormId, bValid, sExplain);
 	return bValid;
 }
 
-function ValidateCKEditField(sFieldId, sPattern, bMandatory, sFormId, nullValue, originalValue)
-{
-	return EvaluateCKEditorValidation({
-		sFieldId: sFieldId,
-		sFormId: sFormId,
-        sOriginalValue: originalValue,
-		onRetry: function() {
-			ValidateCKEditField(sFieldId, sPattern, bMandatory, sFormId, nullValue, originalValue);
-		},
-		onChange: function() {
-			ValidateCKEditField(sFieldId, sPattern, bMandatory, sFormId, nullValue, originalValue);
-		},
-		validate: function(sTextContent, sTextOriginalContents) {
-			var bValid;
-			var sExplain = '';
-			if (bMandatory && (sTextContent === nullValue)) {
-				bValid = false;
-				sExplain = Dict.S('UI:ValueMustBeSet');
-			} else if ((sTextOriginalContents !== undefined) && (sTextContent === sTextOriginalContents)) {
-				bValid = false;
-				if (sTextOriginalContents === nullValue) {
-					sExplain = Dict.S('UI:ValueMustBeSet');
-				} else {
-					// Note: value change check is not working well yet as the HTML to Text conversion is not exactly the same when done from the PHP value or the CKEditor value.
-					sExplain = Dict.S('UI:ValueMustBeChanged');
-				}
-			} else {
-				bValid = true;
-			}
-			return {bValid: bValid, sExplain: sExplain};
-		}
-	});
-}
 function ResetPwd(id)
 {
-    // Reset the values of the password fields
-    $('#'+id).val('*****');
-    $('#'+id+'_confirm').val('*****');
-    // And reset the flag, to tell it that the password remains unchanged
-    $('#'+id+'_changed').val(0);
-    // Visual feedback, None when it's Ok
-    $('#v_'+id).html('');
+	// Reset the values of the password fields
+	$('#'+id).val('*****');
+	$('#'+id+'_confirm').val('*****');
+	// And reset the flag, to tell it that the password remains unchanged
+	$('#'+id+'_changed').val(0);
+	// Visual feedback, None when it's Ok
+	$('#v_'+id).html('');
 }
 
 // Called whenever the content of a one way encrypted password changes
 function PasswordFieldChanged(id)
 {
-    // Set the flag, to tell that the password changed
-    $('#'+id+'_changed').val(1);
+	// Set the flag, to tell that the password changed
+	$('#'+id+'_changed').val(1);
 }
 
 // Special validation function for one way encrypted password fields
@@ -439,48 +415,37 @@ function ValidatePasswordField(id, sFormId)
 // to determine if the field is empty or not
 function ValidateCaseLogField(sFieldId, bMandatory, sFormId, nullValue, originalValue)
 {
-	return EvaluateCKEditorValidation({
-		sFieldId: sFieldId,
-		sFormId: sFormId,
-		sOriginalValue: originalValue,
-		onRetry: function() {
-			ValidateCaseLogField(sFieldId, bMandatory, sFormId, nullValue, originalValue);
-		},
-		onChange: function() {
-			ValidateCaseLogField(sFieldId, bMandatory, sFormId, nullValue, originalValue);
-		},
-		validate: function(sTextContent, sTextOriginalContents) {
-			var bValid;
-			var sExplain = '';
-			// CaseLog is special: history count matters when deciding if the field is empty
-			var count = $('#'+sFieldId+'_count').val();
+	var bValid = true;
+	var sExplain = '';
+	var sTextContent;
+	
+	if ($('#'+sFieldId).prop('disabled'))
+	{
+		bValid = true; // disabled fields are not checked
+	}
+	else
+	{
+		// Get the contents (with tags)
+		// Note: For CaseLog we can't retrieve the formatted contents from CKEditor (unlike in ValidateCKEditorField() method) because of the place holder.
+		sTextContent = $('#' + sFieldId).val();
+		var count = $('#'+sFieldId+'_count').val();
 
-			if (bMandatory && (count == 0) && (sTextContent === nullValue))
-			{
-				// No previous entry and no content typed
-				bValid = false;
-				sExplain = Dict.S('UI:ValueMustBeSet');
-			}
-			else if ((sTextOriginalContents !== undefined) && (sTextContent === sTextOriginalContents))
-			{
-				bValid = false;
-				if (sTextOriginalContents === nullValue)
-				{
-					sExplain = Dict.S('UI:ValueMustBeSet');
-				}
-				else
-				{
-					// Note: value change check is not working well yet as the HTML to Text conversion is not exactly the same when done from the PHP value or the CKEditor value.
-					sExplain = Dict.S('UI:ValueMustBeChanged');
-				}
-			}
-			else
-			{
-				bValid = true;
-			}
-			return {bValid: bValid, sExplain: sExplain};
+		if (bMandatory && (count == 0) && (sTextContent == nullValue))
+		{
+			// No previous entry and no content typed
+			bValid = false;
+			sExplain = Dict.S('UI:ValueMustBeSet');
 		}
-	});
+		else if ((originalValue != undefined) && (sTextContent == originalValue))
+		{
+			bValid = false;
+			sExplain = Dict.S('UI:ValueMustBeChanged');
+		}
+	}
+	ReportFieldValidationStatus(sFieldId, sFormId, bValid, '' /* sExplain */);
+
+	// We need to check periodically as CKEditor doesn't trigger our events. More details in UIHTMLEditorWidget::Display() @ line 92
+	setTimeout(function(){ValidateCaseLogField(sFieldId, bMandatory, sFormId, nullValue, originalValue);}, 500);
 }
 
 // Validate the inputs depending on the current setting

sources/Application/Helper/SearchHelper.php

@@ -1,5 +1,4 @@
 <?php
-
 /*
  * @copyright   Copyright (C) 2010-2026 Combodo SAS
  * @license     http://opensource.org/licenses/AGPL-3.0

sources/Application/UI/Base/Component/DataTable/DataTableSettings.php

@@ -8,8 +8,13 @@ use AttributeFriendlyName;
 use AttributeLinkedSet;
 use cmdbAbstract;
 use cmdbAbstractObject;
+use CoreException;
 use Dict;
+use Exception;
+use IssueLog;
+use LogChannels;
 use Metamodel;
+use utils;
 
 /**
  * Class DataTableSettings
@@ -130,7 +135,10 @@ class DataTableSettings
 	 */
 	public function unserialize($sData)
 	{
-		$aData = unserialize($sData);
+		$aData = utils::Unserialize($sData, ['allowed_classes' => false]);
+		if (!is_array($aData)) {
+			throw new CoreException('Wrong data table settings format, expected an array', ['datatable_settings_data' => $aData]);
+		}
 		$this->iDefaultPageSize = $aData['iDefaultPageSize'];
 		$this->aColumns = $aData['aColumns'];
 		foreach ($this->aClassAliases as $sAlias => $sClass) {
@@ -269,7 +277,19 @@ class DataTableSettings
 				return null;
 			}
 		}
-		$oSettings->unserialize($pref);
+
+		try {
+			$oSettings->unserialize($pref);
+		} catch (Exception $e) {
+			IssueLog::Warning("User table settings corrupted, back to the default values provided by the data model", LogChannels::CONSOLE, [
+				'table_id' => $sTableId,
+				'root_cause' => $e->getMessage(),
+			]);
+			// unset the preference
+			appUserPreferences::UnsetPref($oSettings->GetPrefsKey($sTableId));
+			// use the default values provided by the data model
+			return null;
+		}
 
 		return $oSettings;
 	}

tests/php-unit-tests/unitary-tests/application/Helper/SearchHelperTest.php

@@ -1,5 +1,4 @@
 <?php
-
 /*
  * @copyright   Copyright (C) 2010-2026 Combodo SAS
  * @license     http://opensource.org/licenses/AGPL-3.0

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

@@ -1,95 +0,0 @@
-<?php
-
-/**
- * Copyright (C) 2010-2024 Combodo SAS
- *
- * This file is part of iTop.
- *
- * iTop is free software; you can redistribute it and/or modify
- * it under the terms of the GNU Affero General Public License as published by
- * the Free Software Foundation, either version 3 of the License, or
- * (at your option) any later version.
- *
- * iTop is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
- * GNU Affero General Public License for more details.
- *
- * You should have received a copy of the GNU Affero General Public License
- * along with iTop. If not, see <http://www.gnu.org/licenses/>
- */
-
-namespace Combodo\iTop\Test\UnitTest\Application;
-
-use Combodo\iTop\Test\UnitTest\ItopDataTestCase;
-use utils;
-
-class LoginExternalTest extends ItopDataTestCase
-{
-	private $oConfig;
-	private $sOriginalExtAuthVariable;
-
-	protected function setUp(): void
-	{
-		parent::setUp();
-		require_once APPROOT.'application/loginexternal.class.inc.php';
-		$this->oConfig = utils::GetConfig();
-		$this->sOriginalExtAuthVariable = $this->oConfig->Get('ext_auth_variable');
-	}
-
-	protected function tearDown(): void
-	{
-		$this->oConfig->SetExternalAuthenticationVariable($this->sOriginalExtAuthVariable);
-		parent::tearDown();
-	}
-
-	private function CallGetAuthUser()
-	{
-		$oLoginExternal = new \LoginExternal();
-		$oMethod = new \ReflectionMethod(\LoginExternal::class, 'GetAuthUser');
-		$oMethod->setAccessible(true);
-		return $oMethod->invoke($oLoginExternal);
-	}
-
-	public function testGetAuthUserFromServerVariable()
-	{
-		$_SERVER['REMOTE_USER'] = 'alice';
-		$this->oConfig->SetExternalAuthenticationVariable('$_SERVER[\'REMOTE_USER\']');
-
-		$this->assertSame('alice', $this->CallGetAuthUser());
-	}
-
-	public function testGetAuthUserFromCookie()
-	{
-		$_COOKIE['auth_user'] = 'bob';
-		$this->oConfig->SetExternalAuthenticationVariable('$_COOKIE[\'auth_user\']');
-
-		$this->assertSame('bob', $this->CallGetAuthUser());
-	}
-
-	public function testGetAuthUserFromRequest()
-	{
-		$_REQUEST['auth_user'] = 'carol';
-		$this->oConfig->SetExternalAuthenticationVariable('$_REQUEST[\'auth_user\']');
-
-		$this->assertSame('carol', $this->CallGetAuthUser());
-	}
-
-	public function testInvalidExpressionReturnsFalse()
-	{
-		$this->oConfig->SetExternalAuthenticationVariable('$_SERVER[\'HTTP_X_CMD\']) ? print(\'x\') : false; //');
-
-		$this->assertFalse($this->CallGetAuthUser());
-	}
-
-	public function testGetAuthUserFromHeaderWithoutAllowlist()
-	{
-		if (!function_exists('getallheaders')) {
-			$this->markTestSkipped('getallheaders() not available');
-		}
-		$_SERVER['HTTP_X_REMOTE_USER'] = 'CN=header-test';
-		$this->oConfig->SetExternalAuthenticationVariable('getallheaders()[\'X-Remote-User\']');
-
-		$this->assertSame('CN=header-test', $this->CallGetAuthUser());
-	}
-}