diff --git a/apps/qubit/modules/informationobject/actions/inventoryAction.class.php b/apps/qubit/modules/informationobject/actions/inventoryAction.class.php index 7563784a4a..854c330c67 100644 --- a/apps/qubit/modules/informationobject/actions/inventoryAction.class.php +++ b/apps/qubit/modules/informationobject/actions/inventoryAction.class.php @@ -89,7 +89,7 @@ private static function getLevels() } $setting = QubitSetting::getByName('inventory_levels'); - if (null === $setting || false === $value = unserialize($setting->getValue())) { + if (null === $setting || false === $value = Qubit::safeUnserialize($setting->getValue(), false)) { return; } diff --git a/apps/qubit/modules/settings/actions/inventoryAction.class.php b/apps/qubit/modules/settings/actions/inventoryAction.class.php index 78ba7d2743..1639f13041 100644 --- a/apps/qubit/modules/settings/actions/inventoryAction.class.php +++ b/apps/qubit/modules/settings/actions/inventoryAction.class.php @@ -61,8 +61,9 @@ protected function addField($name) { switch ($name) { case 'levels': - $value = unserialize( - $this->settingLevels->getValue(['sourceCulture' => true]) + $value = Qubit::safeUnserialize( + $this->settingLevels->getValue(['sourceCulture' => true]), + false ); if (false !== $value) { diff --git a/apps/qubit/modules/settings/actions/oaiAction.class.php b/apps/qubit/modules/settings/actions/oaiAction.class.php index 6ffb2c76a8..1872682219 100644 --- a/apps/qubit/modules/settings/actions/oaiAction.class.php +++ b/apps/qubit/modules/settings/actions/oaiAction.class.php @@ -30,7 +30,7 @@ class SettingsOaiAction extends sfAction public function execute($request) { // Redirect to global settings form if the OAI plugin is not enabled - if (!in_array('arOaiPlugin', unserialize(sfConfig::get('app_plugins')))) { + if (!in_array('arOaiPlugin', Qubit::safeUnserialize(sfConfig::get('app_plugins'), []))) { $this->redirect('settings/global'); } diff --git a/lib/Qubit.class.php b/lib/Qubit.class.php index 00ee0c5f32..41091ca864 100644 --- a/lib/Qubit.class.php +++ b/lib/Qubit.class.php @@ -19,6 +19,49 @@ class Qubit { + private const MAX_UNSERIALIZE_INSPECTION_DEPTH = 100; + + /** + * Safely unserialize stored application data without rehydrating objects. + * + * @param mixed $value + * @param null|mixed $default + */ + public static function safeUnserialize($value, $default = null) + { + if (!is_string($value) || '' === $value) { + return $default; + } + + // Application data only uses serialized arrays/scalars; object + // rehydration is intentionally disabled for stored values. + $unserializeWarning = false; + + // Invalid serialized input returns false and raises E_WARNING; valid + // serialized false also returns false, but without raising a warning. + set_error_handler(function () use (&$unserializeWarning) { + $unserializeWarning = true; + + return true; + }, E_WARNING); + + try { + $data = unserialize($value, ['allowed_classes' => false]); + } finally { + restore_error_handler(); + } + + if ($unserializeWarning) { + return $default; + } + + if (self::containsObject($data)) { + return $default; + } + + return $data; + } + public static function pathInfo($url, $request = null) { // Allow callers and tests to supply a request explicitly; otherwise use the current one. @@ -501,4 +544,33 @@ private static function isSameConfiguredOriginUrl(array $urlParts) return $urlPort === $baseUrlPort; } + + /** + * Detect object placeholders that remain after class-disabled unserialize. + * + * PHP unserialize() preserves array references, so reject overly deep + * structures before recursive traversal can loop or exhaust memory. + * + * @param mixed $value + */ + private static function containsObject($value, int $depth = 0) + { + if ($depth > self::MAX_UNSERIALIZE_INSPECTION_DEPTH) { + return true; + } + + if (is_object($value)) { + return true; + } + + if (is_array($value)) { + foreach ($value as $item) { + if (self::containsObject($item, $depth + 1)) { + return true; + } + } + } + + return false; + } } diff --git a/lib/QubitCsvTransform.class.php b/lib/QubitCsvTransform.class.php index 147561337f..6dc283c84f 100644 --- a/lib/QubitCsvTransform.class.php +++ b/lib/QubitCsvTransform.class.php @@ -175,7 +175,7 @@ public function writeMySQLRowsToCsvFilePath($filepath) } // Write CSV row data - $data = unserialize($row['data']); + $data = Qubit::safeUnserialize($row['data'], []); fputcsv($fhOut, $data); diff --git a/lib/QubitXmlImport.class.php b/lib/QubitXmlImport.class.php index 8685ce6ba7..b72865b38c 100644 --- a/lib/QubitXmlImport.class.php +++ b/lib/QubitXmlImport.class.php @@ -208,7 +208,7 @@ public function import($xmlFile, $options = [], $xmlOrigFileName = null) $criteria = new Criteria(); $criteria->add(QubitSetting::NAME, 'plugins'); $setting = QubitSetting::getOne($criteria); - if (null === $setting || !in_array('sfSkosPlugin', unserialize($setting->getValue(['sourceCulture' => true])))) { + if (null === $setting || !in_array('sfSkosPlugin', Qubit::safeUnserialize($setting->getValue(['sourceCulture' => true]), []))) { throw new sfException($this->i18n->__('The SKOS plugin is not enabled')); } $this->rootObject = QubitTaxonomy::getById($options['taxonomy']); diff --git a/lib/filter/QubitSettingsFilter.class.php b/lib/filter/QubitSettingsFilter.class.php index f13d2a25c1..3a785e532a 100644 --- a/lib/filter/QubitSettingsFilter.class.php +++ b/lib/filter/QubitSettingsFilter.class.php @@ -26,8 +26,10 @@ public function execute($filterChain) // Get settings (from cache if exists) if ($cache->has($cacheKey)) { - $settings = unserialize($cache->get($cacheKey)); - } else { + $settings = Qubit::safeUnserialize($cache->get($cacheKey)); + } + + if (!isset($settings)) { $settings = QubitSetting::getSettingsArray(); $cache->set($cacheKey, serialize($settings)); diff --git a/lib/form/SettingsPermissionsForm.class.php b/lib/form/SettingsPermissionsForm.class.php index 2f0964dec4..77b2900270 100644 --- a/lib/form/SettingsPermissionsForm.class.php +++ b/lib/form/SettingsPermissionsForm.class.php @@ -60,7 +60,7 @@ protected function getPermissionsForm() throw new sfException('Setting premisAccessRightValues cannot be found'); } - $premisAccessRightValues = unserialize($premisAccessRightValues->getValue(['sourceCulture' => true])); + $premisAccessRightValues = Qubit::safeUnserialize($premisAccessRightValues->getValue(['sourceCulture' => true]), []); $defaults = QubitSetting::$premisAccessRightValueDefaults; $form = new sfForm(); diff --git a/lib/model/QubitActor.php b/lib/model/QubitActor.php index cf4141d0bd..0fda574bb4 100644 --- a/lib/model/QubitActor.php +++ b/lib/model/QubitActor.php @@ -63,7 +63,7 @@ public function __get($name) } } - if (isset($this->values[$name]) && null !== $value = unserialize($this->values[$name]->__get('value', $options + ['sourceCulture' => true]))) { + if (isset($this->values[$name]) && null !== $value = Qubit::safeUnserialize($this->values[$name]->__get('value', $options + ['sourceCulture' => true]))) { return $value; } diff --git a/lib/model/QubitFunctionObject.php b/lib/model/QubitFunctionObject.php index a3bb3104dc..5a9a2ec502 100644 --- a/lib/model/QubitFunctionObject.php +++ b/lib/model/QubitFunctionObject.php @@ -51,7 +51,7 @@ public function __get($name) } } - if (isset($this->values[$name]) && null !== $value = unserialize($this->values[$name]->__get('value', $options + ['sourceCulture' => true]))) { + if (isset($this->values[$name]) && null !== $value = Qubit::safeUnserialize($this->values[$name]->__get('value', $options + ['sourceCulture' => true]))) { return $value; } diff --git a/lib/model/QubitGrantedRight.php b/lib/model/QubitGrantedRight.php index 86d5b5af4a..679505c40e 100644 --- a/lib/model/QubitGrantedRight.php +++ b/lib/model/QubitGrantedRight.php @@ -126,7 +126,7 @@ public static function getPremisSettings() */ $permissions = QubitSetting::getByName('premisAccessRightValues'); - return [$act->id, unserialize($permissions->getValue(['sourceCulture' => true]))]; + return [$act->id, Qubit::safeUnserialize($permissions->getValue(['sourceCulture' => true]), [])]; } /** diff --git a/lib/model/QubitInformationObject.php b/lib/model/QubitInformationObject.php index 79c33142c5..8875c7fbc7 100644 --- a/lib/model/QubitInformationObject.php +++ b/lib/model/QubitInformationObject.php @@ -105,7 +105,7 @@ public function __get($name) } } - if (isset($this->values[$name]) && null !== $value = unserialize($this->values[$name]->__get('value', $options + ['sourceCulture' => true]))) { + if (isset($this->values[$name]) && null !== $value = Qubit::safeUnserialize($this->values[$name]->__get('value', $options + ['sourceCulture' => true]))) { return $value; } diff --git a/lib/model/QubitMenu.php b/lib/model/QubitMenu.php index 84a3e02851..fa06ee437e 100644 --- a/lib/model/QubitMenu.php +++ b/lib/model/QubitMenu.php @@ -124,7 +124,7 @@ public function isProtected() return false; } - $lockInfo = unserialize(sfConfig::get('app_menu_locking_info', [])); + $lockInfo = Qubit::safeUnserialize(sfConfig::get('app_menu_locking_info', []), []); // If lock info isn't empty and the menu's ID or name indicates it should be locked, then lock it if (count($lockInfo) && (in_array($this->id, $lockInfo['byId']) || in_array($this->name, $lockInfo['byName']))) { diff --git a/lib/task/tools/atomPluginsTask.class.php b/lib/task/tools/atomPluginsTask.class.php index d52086342b..a84a99d0e1 100644 --- a/lib/task/tools/atomPluginsTask.class.php +++ b/lib/task/tools/atomPluginsTask.class.php @@ -35,7 +35,7 @@ public function execute($arguments = [], $options = []) } // Array of plugins - $plugins = array_values(unserialize($setting->getValue(['sourceCulture' => true]))); + $plugins = array_values(Qubit::safeUnserialize($setting->getValue(['sourceCulture' => true]), [])); if (in_array($arguments['action'], ['add', 'delete']) && !isset($arguments['plugin'])) { throw new sfException('Missing plugin name.'); diff --git a/plugins/arElasticSearchPlugin/lib/arElasticSearchPluginUtil.class.php b/plugins/arElasticSearchPlugin/lib/arElasticSearchPluginUtil.class.php index 32452e090e..70da751390 100644 --- a/plugins/arElasticSearchPlugin/lib/arElasticSearchPluginUtil.class.php +++ b/plugins/arElasticSearchPlugin/lib/arElasticSearchPluginUtil.class.php @@ -282,7 +282,7 @@ public static function getPremisData($ioId, $conn) $statement->execute([$ioId]); foreach ($statement->fetchAll(PDO::FETCH_OBJ) as $property) { - $value = unserialize($property->value); + $value = Qubit::safeUnserialize($property->value, []); switch ($property->name) { case 'fitsAudio': diff --git a/plugins/qbAclPlugin/lib/model/QubitAclPermission.php b/plugins/qbAclPlugin/lib/model/QubitAclPermission.php index dbf7a60f27..d5d5664308 100644 --- a/plugins/qbAclPlugin/lib/model/QubitAclPermission.php +++ b/plugins/qbAclPlugin/lib/model/QubitAclPermission.php @@ -75,7 +75,7 @@ public function getConstants($options = []) $value = null; if (null !== $constants = parent::__get('constants', $options)) { - $value = unserialize($constants); + $value = Qubit::safeUnserialize($constants, []); } if (isset($options['name'])) { @@ -94,7 +94,7 @@ public function setConstants($value, $options = []) if (is_array($value)) { $constants = []; if (parent::__isset('constants', $options)) { - $constants = unserialize(parent::__get('constants', $options)); + $constants = Qubit::safeUnserialize(parent::__get('constants', $options), []); } foreach ($value as $key => $val) { @@ -124,7 +124,7 @@ public function evaluateConditional($parameters) return true; } - $constants = unserialize($this->constants); + $constants = Qubit::safeUnserialize($this->constants, []); // Substitute constants if (preg_match_all('/%k\[(\w+)\]/', $conditional, $matches)) { diff --git a/plugins/sfEadPlugin/modules/sfEadPlugin/templates/indexSuccessBody.xml.php b/plugins/sfEadPlugin/modules/sfEadPlugin/templates/indexSuccessBody.xml.php index 0a96d7e985..ff20016b31 100644 --- a/plugins/sfEadPlugin/modules/sfEadPlugin/templates/indexSuccessBody.xml.php +++ b/plugins/sfEadPlugin/modules/sfEadPlugin/templates/indexSuccessBody.xml.php @@ -63,7 +63,7 @@ getPropertyByName('languageOfDescription')->__toString()) && ($authenticated || (1 == sfConfig::get('app_element_visibility_isad_control_languages') && !$findingAid))) { ?> - + diff --git a/plugins/sfPluginAdminPlugin/config/sfPluginAdminPluginConfiguration.class.php b/plugins/sfPluginAdminPlugin/config/sfPluginAdminPluginConfiguration.class.php index ada037cc60..5d34413179 100644 --- a/plugins/sfPluginAdminPlugin/config/sfPluginAdminPluginConfiguration.class.php +++ b/plugins/sfPluginAdminPlugin/config/sfPluginAdminPluginConfiguration.class.php @@ -41,7 +41,7 @@ public function initialize() // http://accesstomemory.org/wiki/index.php?title=Autoload $this->dispatcher->disconnect('autoload.filter_config', [$this->configuration, 'filterAutoloadConfig']); - $pluginNames = unserialize($query[0]->__get('value', ['sourceCulture' => true])); + $pluginNames = Qubit::safeUnserialize($query[0]->__get('value', ['sourceCulture' => true]), []); // if (isset($_GET['t'])) // { diff --git a/plugins/sfPluginAdminPlugin/modules/sfPluginAdminPlugin/actions/pluginsAction.class.php b/plugins/sfPluginAdminPlugin/modules/sfPluginAdminPlugin/actions/pluginsAction.class.php index d8076de086..83e2abcdf4 100644 --- a/plugins/sfPluginAdminPlugin/modules/sfPluginAdminPlugin/actions/pluginsAction.class.php +++ b/plugins/sfPluginAdminPlugin/modules/sfPluginAdminPlugin/actions/pluginsAction.class.php @@ -36,7 +36,7 @@ public function execute($request) if (1 == count($query = QubitSetting::get($criteria))) { $setting = $query[0]; - $this->form->setDefault('enabled', unserialize($setting->getValue(['sourceCulture' => true]))); + $this->form->setDefault('enabled', Qubit::safeUnserialize($setting->getValue(['sourceCulture' => true]), [])); } $configuration = ProjectConfiguration::getActive(); @@ -77,7 +77,7 @@ public function execute($request) $setting->name = 'plugins'; } - $settings = unserialize($setting->getValue(['sourceCulture' => true])); + $settings = Qubit::safeUnserialize($setting->getValue(['sourceCulture' => true]), []); $swordEnabled = in_array('qtSwordPlugin', $settings); diff --git a/plugins/sfPluginAdminPlugin/modules/sfPluginAdminPlugin/actions/themesAction.class.php b/plugins/sfPluginAdminPlugin/modules/sfPluginAdminPlugin/actions/themesAction.class.php index 0f3d0d59c4..b89f78b50d 100644 --- a/plugins/sfPluginAdminPlugin/modules/sfPluginAdminPlugin/actions/themesAction.class.php +++ b/plugins/sfPluginAdminPlugin/modules/sfPluginAdminPlugin/actions/themesAction.class.php @@ -35,7 +35,7 @@ public function execute($request) if (1 == count($query = QubitSetting::get($criteria))) { $setting = $query[0]; - $this->form->setDefault('enabled', unserialize($setting->getValue(['sourceCulture' => true]))); + $this->form->setDefault('enabled', Qubit::safeUnserialize($setting->getValue(['sourceCulture' => true]), [])); } $configuration = ProjectConfiguration::getActive(); @@ -76,7 +76,7 @@ public function execute($request) $setting->name = 'plugins'; } - $settings = unserialize($setting->getValue(['sourceCulture' => true])); + $settings = Qubit::safeUnserialize($setting->getValue(['sourceCulture' => true]), []); foreach (array_keys($this->plugins) as $item) { if (in_array($item, (array) $this->form->getValue('enabled'))) { diff --git a/test/phpunit/lib/QubitTest.php b/test/phpunit/lib/QubitTest.php new file mode 100644 index 0000000000..82f5094964 --- /dev/null +++ b/test/phpunit/lib/QubitTest.php @@ -0,0 +1,47 @@ +assertSame(['a' => 1], Qubit::safeUnserialize(serialize(['a' => 1]))); + $this->assertSame('value', Qubit::safeUnserialize(serialize('value'))); + $this->assertFalse(Qubit::safeUnserialize(serialize(false), true)); + } + + public function testSafeUnserializeReturnsDefaultForInvalidValues() + { + $this->assertSame([], Qubit::safeUnserialize('not serialized', [])); + $this->assertSame([], Qubit::safeUnserialize(null, [])); + $this->assertSame([], Qubit::safeUnserialize('', [])); + } + + public function testSafeUnserializeRejectsObjects() + { + $this->assertSame([], Qubit::safeUnserialize(serialize(new stdClass()), [])); + $this->assertSame([], Qubit::safeUnserialize(serialize(['nested' => new stdClass()]), [])); + } + + public function testSafeUnserializeRejectsRecursiveArrays() + { + $this->assertSame([], Qubit::safeUnserialize('a:1:{i:0;R:1;}', [])); + } + + public function testSafeUnserializeRejectsDeeplyNestedArrays() + { + $value = 'value'; + + for ($i = 0; $i < 101; ++$i) { + $value = [$value]; + } + + $this->assertSame([], Qubit::safeUnserialize(serialize($value), [])); + } +}