Bug #64146 [Ana->Csd]: serialize incorrectly saving objects when they are cloned

Edit report at https://bugs.php.net/bug.php?id=64146&edit=1 ID: 64146 Updated by: stas@php.net Reported by: slusarz at curecanti dot org Summary: serialize incorrectly saving objects when they are cloned -Status: Analyzed +Status: Closed Type: Bug Package: Variables related Operating System: Linux PHP Version: 5.4.11 Assigned To: mike Block user comment: N Private report: N New Comment: Automatic comment on behalf of mike Revision: http://git.php.net/?p=php-src-security.git;a=commit;h=8973390541faaadfdfc0f838421f037060188e5e Log: fix bug #64146 (serialize incorrectly saving objects when they are cloned) Previous Comments: ------------------------------------------------------------------------ [2014-09-28 22:36:54] stas@php.net Still happens for me in latest 5.5 on 32-bit. I think the problem is as follows: When serializing the value given by clone in the first B object, it is remembered in the var_hash. However, it is then immediately destroyed. When it comes to serialize the other value in the second B object, the new clone is created. By chance, it may happen that this clone is located in the same memory address and has the same object ID as the previous clone. Thus, since the common hash is used, for the system it is indistinguishable from the clone created when serializing the previous B object. Thus, it is recorded as reference (r). This is wrong (since it essentially says both B objects refer to the same object, while they are not) but this is only half of the bug. unserialize() fails for a different reason. The reason is that when trying to parse r:4; it should replace current return value with pointer to the element 4 (which is previous B object) but it can not since it is given only return_value and not return_value_ptr. I.e., it does the replacement but this replacement does nothing, since unserialize() expects by-value return. It can be fixed, but the result will still not be right - in that case, the return would look as if both B classes refer to the same value. I'm not sure how to fix it properly, as the engine in this case has no good way to distinguish between the two clones short of retaining each object it serializes, which may make serialization significantly more expensive. ------------------------------------------------------------------------ [2014-09-09 08:01:53] turneliusz at gmail dot com Fixed in 5.5.5-5.7 http://3v4l.org/DR8T3 ------------------------------------------------------------------------ [2014-01-10 08:33:44] gm dot outside+php at gmail dot com PHP 5.5.7 (the latest at this moment) fails its testsuite on a 32-bit architecture on this bug. The reproduction build is very simple: ./configure --disable-all && make && make test . According to Remi Collet this has started with PHP 5.5.5 and affects only 32-bit systems while 64-bit systems pass the test. The latest reply on the PHP development list I found was from Michael Wallner saying that he was going to look into the issue: http://permalink.gmane.org/gmane.comp.php.devel/82473 Well, the issue is still there, so the bug is not properly solved, IMO. Below is the content of ext/standard/tests/serialize/bug64146.diff on my 32-bit system after the failure of the test: === $ cat ext/standard/tests/serialize/bug64146.diff003+ 004+ Notice: Trying to get property of non-object in /usr/src/php-5.5.7/ext/standard/tests/serialize/bug64146.php on line 49 005+ 003- 2 $ === ------------------------------------------------------------------------ [2013-10-04 14:18:01] mike@php.net Automatic comment on behalf of mike Revision: http://git.php.net/?p=php-src.git;a=commit;h=8973390541faaadfdfc0f838421f037060188e5e Log: fix bug #64146 (serialize incorrectly saving objects when they are cloned) ------------------------------------------------------------------------ [2013-02-07 22:58:23] mike@php.net Using zend_objects_get_address() instead of the object handle fixes; but triggers bug #62836 ------------------------------------------------------------------------ The remainder of the comments for this report are too long. To view the rest of the comments, please view the bug report online at https://bugs.php.net/bug.php?id=64146 -- Edit this bug report at https://bugs.php.net/bug.php?id=64146&edit=1