Re: com php-src: Fixed bug #67072 Echoing unserialized "SplFileObject" crash: NEWS ext/standard/tests/serialize/005.phpt
ext/standard/tests/serialize/bug67072.phpt ext/standard/var_unserializer.c ext/standard/var_unserializer.re

On Thu, Apr 17, 2014 at 10:48 AM, Anatol Belski <ab@php.net> wrote: > Commit: 5328d4289946e260232f3195ba2e0f0eb173d5ef > Author: Anatol Belski <ab@php.net> Thu, 17 Apr 2014 10:48:14 > +0200 > Parents: 7a5f1663c6775bbdaf870e4c71ef8813d5d13179 > Branches: PHP-5.4 PHP-5.5 PHP-5.6 master > > Link: > > http://git.php.net/?p=php-src.git;a=commitdiff;h=5328d4289946e260232f3195ba2e0f0eb173d5ef > > Log: > Fixed bug #67072 Echoing unserialized "SplFileObject" crash > > The actual issue lays in the unserializer code which doesn't honor > the unserialize callback. By contrast, the serialize callback is > respected. This leads to the situation that even if a class has > disabled the serialization explicitly, user could still construct > a vulnerable string which would result bad things when trying > to unserialize. > > This conserns also the classes implementing Serializable as well > as some core classes disabling serialize/unserialize callbacks > explicitly (PDO, SimpleXML, SplFileInfo and co). As of now, the > flow is first to call the unserialize callback (if available), > then call __wakeup. If the unserialize callback returns with no > success, no object is instantiated. This makes the scheme used > by internal classes effective, to disable unserialize just assign > zend_class_unserialize_deny as callback. > > Bugs: > https://bugs.php.net/67072 > This was likely by design. The unserialize callback is supposed to only be invoked for the C type, whereas wakeup is used only for the O type. That's why disallowing serialization was always done with unserialize_deny for C and an erroring __wakeup for O. Now the O type will try to invoke the C unserialization handler as well - which doesn't make a lot of sense, because both use entirely different formats (O is basically an array, C is custom data format). In particular, the parse_iv2 value has a different meaning for O (number of elements) and C (length of string). As such the new code introduces a potential buffer overread vulnerability, as we can now specify a parse_iv2 value that is larger than the input string. To see that this is plausible, try something simple like unserialize('O:11:"ArrayObject":100:{}'). This will give you the following exception, which contains trailing data after the unserialized string: Fatal error: Uncaught exception 'UnexpectedValueException' with message 'Error at offset 0 of 100 bytes' in /home/nikic/dev/php-5.6/t09.php:3 Stack trace: #0 [internal function]: ArrayObject->unserialize('}\x00\xF6\x9B\xC6n\xC6nQ\xBA\xC7\xE59\x10\x00...') #1 /home/nikic/dev/php-5.6/t09.php(3): unserialize('O:11:"ArrayObje...') #2 {main} thrown in /home/nikic/dev/php-5.6/t09.php on line 3 Nikita