diff --git a/application/menunode.class.inc.php b/application/menunode.class.inc.php index 0071b5532..bbd1341c9 100644 --- a/application/menunode.class.inc.php +++ b/application/menunode.class.inc.php @@ -1546,12 +1546,21 @@ class ShortcutMenuNode extends MenuNode public function GetHyperlink($aExtraParams) { $sContext = $this->oShortcut->Get('context'); - $aContext = unserialize($sContext); - if (isset($aContext['menu'])) { - unset($aContext['menu']); - } - foreach ($aContext as $sArgName => $sArgValue) { - $aExtraParams[$sArgName] = $sArgValue; + try { + $aContext = utils::Unserialize($sContext); + if (isset($aContext['menu'])) { + unset($aContext['menu']); + } + foreach ($aContext as $sArgName => $sArgValue) { + $aExtraParams[$sArgName] = $sArgValue; + } + } 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(); } return parent::GetHyperlink($aExtraParams); } diff --git a/application/utils.inc.php b/application/utils.inc.php index 3e0e6ddb3..84ee1b28f 100644 --- a/application/utils.inc.php +++ b/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 string $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(string $data, array $aOptions = ['allowed_classes' => false], 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); + } + } + } } diff --git a/core/attributedef.class.inc.php b/core/attributedef.class.inc.php index a7554f29c..afc52329b 100644 --- a/core/attributedef.class.inc.php +++ b/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); diff --git a/sources/Application/UI/Base/Component/DataTable/DataTableSettings.php b/sources/Application/UI/Base/Component/DataTable/DataTableSettings.php index 2a4496b11..03ceb31f1 100644 --- a/sources/Application/UI/Base/Component/DataTable/DataTableSettings.php +++ b/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); + 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; } diff --git a/tests/php-unit-tests/unitary-tests/application/utilsTest.php b/tests/php-unit-tests/unitary-tests/application/utilsTest.php index c0ca39975..5d357e846 100644 --- a/tests/php-unit-tests/unitary-tests/application/utilsTest.php +++ b/tests/php-unit-tests/unitary-tests/application/utilsTest.php @@ -23,6 +23,7 @@ namespace Combodo\iTop\Test\UnitTest\Application; use Combodo\iTop\Test\UnitTest\ItopTestCase; +use CoreException; use ormDocument; use utils; @@ -1043,4 +1044,21 @@ INI; unlink($sTmpFileOutsideItop); } + + public function testUnserialize() + { + // data to unserialize containing an object + $sData = 'a:2:{s:6:"string";s:9:"My string";s:6:"object";O:8:"DateTime":3:{s:4:"date";s:26:"2026-04-13 09:09:23.033175";s:13:"timezone_type";i:3;s:8:"timezone";s:16:"Europe/Amsterdam";}}'; + + // allow the DateTime object (no exception triggered) + utils::Unserialize($sData, ['allowed_classes' => ['DateTime']]); + + // flag to avoid throwing an exception + utils::Unserialize($sData, ['allowed_classes' => false], false); + + // flag to require throwing an exception + $this->expectException(CoreException::class); + utils::Unserialize($sData); + + } }