php.net |  support |  documentation |  report a bug |  advanced search |  search howto |  statistics |  random bug |  login
Bug #64146 serialize incorrectly saving objects when they are cloned
Submitted: 2013-02-04 21:11 UTC Modified: 2014-10-08 19:40 UTC
Votes:77
Avg. Score:1.3 ± 0.9
Reproduced:3 of 8 (37.5%)
Same Version:2 (66.7%)
Same OS:2 (66.7%)
From: slusarz at curecanti dot org Assigned: mike
Status: Closed Package: Variables related
PHP Version: 5.4.11 OS: Linux
Private report: No CVE-ID:
 [2013-02-04 21:11 UTC] slusarz at curecanti dot org
Description:
------------
When using clone in a Serializable serialize() method, the first object is correctly cloned/saved.  However, all subsequent calls to serialize() in the same script access will have incorrect serialized representations of the cloned object.

Using var_dump, it appears that clone is reusing the same object ID when the object is cloned.

I can verify that removing the 'clone' keyword in the test script causes the script to run correctly.

Test script:
---------------
https://gist.github.com/4709613

Expected result:
----------------
See gist for expected value.

For the record, the serialized value of the A object is as follows:

O:1:"A":1:{s:1:"a";a:2:{i:0;C:1:"B":24:{O:1:"C":1:{s:1:"c";i:1;}}i:1;C:1:"B":4:{r:4;}}}

Actual result:
--------------
See gist for actual value.

FWIW, running with --enable-debug produces this message:

[Mon Feb  4 14:11:17 2013]  Script:  '/tmp/test.php'
/disk2/src/php-5.4.11/Zend/zend_vm_execute.h(624) :  Freeing 0x7FD6C9C94D78 (32 bytes), script=/tmp/test.php
=== Total 1 memory leaks detected ===

Patches

zend_objects_get_address (last revision 2013-02-07 22:57 UTC) by mike@php.net)

Add a Patch

Pull Requests

Add a Pull Request

History

AllCommentsChangesGit/SVN commitsRelated reports
 [2013-02-07 16:05 UTC] laruence@php.net
-Status: Open +Status: Feedback
 [2013-02-07 16:05 UTC] laruence@php.net
the link says: 4709613 hasn't created any public gists yet.
 [2013-02-07 17:18 UTC] slusarz at curecanti dot org
-Status: Feedback +Status: Open
 [2013-02-07 17:18 UTC] slusarz at curecanti dot org
Strange... Since that is the link github provides when you click on "share".

Anyway, try this instead:

https://gist.github.com/slusarz/4709613
 [2013-02-07 19:37 UTC] nikic@php.net
@slusarz: Github changed the Gist URLs to include the owner's name too. I guess they didn't consider users having number-names while doing that ^^
 [2013-02-07 21:35 UTC] mike@php.net
Obviously a flaw in the way an object is identified in the engine/in the 
serializer.

Since the cloned objects only live for the time of serialization, both have the 
same object handle and will be identified as the *same*. 

Not sure why unserialize barfs on it, though.
 [2013-02-07 21:35 UTC] mike@php.net
-Status: Open +Status: Verified
 [2013-02-07 22:57 UTC] mike@php.net
The following patch has been added/updated:

Patch Name: zend_objects_get_address
Revision:   1360277847
URL:        https://bugs.php.net/patch-display.php?bug=64146&patch=zend_objects_get_address&revision=1360277847
 [2013-02-07 22:58 UTC] mike@php.net
Using zend_objects_get_address() instead of the object handle fixes; but triggers 
bug #62836
 [2013-10-04 14:18 UTC] 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-10-04 14:18 UTC] mike@php.net
-Status: Verified +Status: Closed
 [2014-01-10 08:33 UTC] 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
$ 
===
 [2014-01-15 19:35 UTC] mike@php.net
-Status: Closed +Status: Assigned -Assigned To: +Assigned To: mike
 [2014-09-09 08:01 UTC] turneliusz at gmail dot com
Fixed in 5.5.5-5.7 http://3v4l.org/DR8T3
 [2014-09-28 22:36 UTC] 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-28 22:37 UTC] stas@php.net
-Status: Assigned +Status: Analyzed
 [2014-10-07 23:17 UTC] stas@php.net
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)
 [2014-10-07 23:17 UTC] stas@php.net
-Status: Analyzed +Status: Closed
 [2014-10-07 23:28 UTC] stas@php.net
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)
 [2014-10-08 19:40 UTC] slusarz at curecanti dot org
Those most recent commit links seem broken.

For future reference, this appears to be the patch that fixes:

http://git.php.net/?p=php-src.git;a=commit;h=8973390541faaadfdfc0f838421f037060188e5e
 
PHP Copyright © 2001-2015 The PHP Group
All rights reserved.
Last updated: Mon Jun 15 18:01:54 2015 UTC