diff --git a/lib/Rule/Addresses.php b/lib/Rule/Addresses.php index 0da155a..fd58334 100644 --- a/lib/Rule/Addresses.php +++ b/lib/Rule/Addresses.php @@ -54,9 +54,9 @@ public function __get($name) { switch ($name) { case 'addresses': - return $this->_addr->bare_addresses; + return $this->_addressList()->bare_addresses; case 'addressList': - return $this->_addr; + return $this->_addressList(); } } @@ -80,6 +80,20 @@ public function __set($name, $data) } } + /** + * Ensure $_addr is a usable list (guards incomplete unserialize). + * + * @return Horde_Mail_Rfc822_List + */ + protected function _addressList() + { + if (!($this->_addr instanceof Horde_Mail_Rfc822_List)) { + $this->_addr = new Horde_Mail_Rfc822_List(); + } + + return $this->_addr; + } + /** * Add addresses to the current address list. * @@ -91,7 +105,7 @@ public function addAddresses($to_add) { global $injector; - $addr = clone $this->_addr; + $addr = clone $this->_addressList(); $addr->add($to_add); $addr->unique(); @@ -126,7 +140,7 @@ protected function _setAddressesException($addr_count, $max) */ public function count(): int { - return count($this->_addr); + return count($this->_addressList()); } } diff --git a/lib/Storage.php b/lib/Storage.php index 8a25ba8..39d6e60 100644 --- a/lib/Storage.php +++ b/lib/Storage.php @@ -37,6 +37,43 @@ abstract class Ingo_Storage implements Countable, IteratorAggregate public const MAX_NONE = 1; public const MAX_OVER = 2; + /** + * Classes allowed when unserializing stored rules (prefs/Mongo). + * + * Must include nested Horde_Mail_Rfc822_* objects held by + * Ingo_Rule_Addresses::$_addr — omitting them yields + * __PHP_Incomplete_Class and TypeError on count(). + * + * @return string[] + */ + public static function unserializeAllowedClasses() + { + return [ + 'Ingo_Rule', + 'Ingo_Rule_Addresses', + 'Ingo_Rule_System_Blacklist', + 'Ingo_Rule_System_Forward', + 'Ingo_Rule_System_Spam', + 'Ingo_Rule_System_Vacation', + 'Ingo_Rule_System_Whitelist', + 'Ingo_Rule_User', + 'Ingo_Rule_User_Discard', + 'Ingo_Rule_User_FlagOnly', + 'Ingo_Rule_User_Keep', + 'Ingo_Rule_User_Move', + 'Ingo_Rule_User_MoveKeep', + 'Ingo_Rule_User_Notify', + 'Ingo_Rule_User_Redirect', + 'Ingo_Rule_User_RedirectKeep', + 'Ingo_Rule_User_Reject', + // Nested inside Ingo_Rule_Addresses::$_addr + 'Horde_Mail_Rfc822_Address', + 'Horde_Mail_Rfc822_Group', + 'Horde_Mail_Rfc822_GroupList', + 'Horde_Mail_Rfc822_List', + ]; + } + /** * Configuration parameters. * diff --git a/lib/Storage/Mongo.php b/lib/Storage/Mongo.php index e910322..0321690 100644 --- a/lib/Storage/Mongo.php +++ b/lib/Storage/Mongo.php @@ -80,25 +80,10 @@ protected function _loadFromBackend() if (isset($res['result'])) { foreach ($res['result'] as $val) { - if ($ob = @unserialize($val[self::DATA], ['allowed_classes' => [ - 'Ingo_Rule', - 'Ingo_Rule_Addresses', - 'Ingo_Rule_System_Blacklist', - 'Ingo_Rule_System_Forward', - 'Ingo_Rule_System_Spam', - 'Ingo_Rule_System_Vacation', - 'Ingo_Rule_System_Whitelist', - 'Ingo_Rule_User', - 'Ingo_Rule_User_Discard', - 'Ingo_Rule_User_FlagOnly', - 'Ingo_Rule_User_Keep', - 'Ingo_Rule_User_Move', - 'Ingo_Rule_User_MoveKeep', - 'Ingo_Rule_User_Notify', - 'Ingo_Rule_User_Redirect', - 'Ingo_Rule_User_RedirectKeep', - 'Ingo_Rule_User_Reject', - ]])) { + if ($ob = @unserialize( + $val[self::DATA], + ['allowed_classes' => self::unserializeAllowedClasses()] + )) { $ob->uid = strval($val[self::MONGO_ID]); $this->_rules[] = $ob; } diff --git a/lib/Storage/Prefs.php b/lib/Storage/Prefs.php index d8c95c4..6982292 100644 --- a/lib/Storage/Prefs.php +++ b/lib/Storage/Prefs.php @@ -29,25 +29,10 @@ class Ingo_Storage_Prefs extends Ingo_Storage */ protected function _loadFromBackend() { - if ($rules = @unserialize($this->_prefs()->getValue('rules'), ['allowed_classes' => [ - 'Ingo_Rule', - 'Ingo_Rule_Addresses', - 'Ingo_Rule_System_Blacklist', - 'Ingo_Rule_System_Forward', - 'Ingo_Rule_System_Spam', - 'Ingo_Rule_System_Vacation', - 'Ingo_Rule_System_Whitelist', - 'Ingo_Rule_User', - 'Ingo_Rule_User_Discard', - 'Ingo_Rule_User_FlagOnly', - 'Ingo_Rule_User_Keep', - 'Ingo_Rule_User_Move', - 'Ingo_Rule_User_MoveKeep', - 'Ingo_Rule_User_Notify', - 'Ingo_Rule_User_Redirect', - 'Ingo_Rule_User_RedirectKeep', - 'Ingo_Rule_User_Reject', - ]])) { + if ($rules = @unserialize( + $this->_prefs()->getValue('rules'), + ['allowed_classes' => self::unserializeAllowedClasses()] + )) { $this->_rules = $rules; } } diff --git a/test/Ingo/Unit/StorageUnserializeTest.php b/test/Ingo/Unit/StorageUnserializeTest.php new file mode 100644 index 0000000..9a576bd --- /dev/null +++ b/test/Ingo/Unit/StorageUnserializeTest.php @@ -0,0 +1,74 @@ + + * @category Horde + * @copyright 2026 The Horde Project + * @license http://www.horde.org/licenses/apache ASL + * @package Ingo + * @subpackage UnitTests + */ + +/** + * Ensure prefs/Mongo unserialize allowlist includes nested mail objects. + * + * Regression for TypeError on count() when Horde_Mail_Rfc822_List became + * __PHP_Incomplete_Class after the ZDI-20-1051 allowed_classes lockdown. + * + * @author Torben Dannhauer + * @category Horde + * @copyright 2026 The Horde Project + * @ignore + * @license http://www.horde.org/licenses/apache ASL + * @package Ingo + * @subpackage UnitTests + * @coversNothing + */ +class Ingo_Unit_StorageUnserializeTest extends Ingo_Unit_TestBase +{ + public function testWhitelistRoundtripPreservesAddresses() + { + $rule = new Ingo_Rule_System_Whitelist(); + $ref = new ReflectionProperty(Ingo_Rule_Addresses::class, '_addr'); + $ref->setAccessible(true); + $ref->setValue( + $rule, + new Horde_Mail_Rfc822_List(['alice@example.com', 'bob@example.com']) + ); + + $serialized = serialize([$rule]); + $rules = unserialize($serialized, [ + 'allowed_classes' => Ingo_Storage::unserializeAllowedClasses(), + ]); + + $this->assertCount(1, $rules); + $this->assertInstanceOf(Ingo_Rule_System_Whitelist::class, $rules[0]); + $this->assertSame(2, count($rules[0])); + $this->assertSame( + ['alice@example.com', 'bob@example.com'], + $rules[0]->addresses + ); + $this->assertInstanceOf( + Horde_Mail_Rfc822_List::class, + $rules[0]->addressList + ); + } + + public function testAllowlistIncludesNestedMailClasses() + { + $allowed = Ingo_Storage::unserializeAllowedClasses(); + + $this->assertContains('Horde_Mail_Rfc822_List', $allowed); + $this->assertContains('Horde_Mail_Rfc822_Address', $allowed); + $this->assertContains('Horde_Mail_Rfc822_Group', $allowed); + $this->assertContains('Horde_Mail_Rfc822_GroupList', $allowed); + $this->assertContains('Ingo_Rule_System_Whitelist', $allowed); + $this->assertContains('Ingo_Rule_System_Blacklist', $allowed); + $this->assertContains('Ingo_Rule_System_Vacation', $allowed); + } +}