php.net |  support |  documentation |  report a bug |  advanced search |  search howto |  statistics |  random bug |  login

go to bug id or search bugs for

Bug #64196 Indirect/magic call recursion stack overflow
Submitted: 2013-02-12 14:27 UTC Modified: 2022-12-16 17:24 UTC
Votes:6
Avg. Score:4.5 ± 0.5
Reproduced:5 of 6 (83.3%)
Same Version:2 (40.0%)
Same OS:2 (40.0%)
From: krakjoe@php.net Assigned: lbarnaud (profile)
Status: Closed Package: Reproducible crash
PHP Version: Irrelevant OS: Any
Private report: No CVE-ID: None
View Developer Edit
Welcome back! If you're the original bug submitter, here's where you can edit the bug or add additional notes.
If you forgot your password, you can retrieve your password here.
Password:
Status:
Package:
Bug Type:
Summary:
From: krakjoe@php.net
New email:
PHP Version: OS:

 

 [2013-02-12 14:27 UTC] krakjoe@php.net 
Description:
------------
This patch avoids stack overflows where recursion is present in __clone.

Test script:
---------------
<?php
class Cloneable {
	public function __clone(){
		return clone $this;
	}
}

$c = new Cloneable();
$a = clone $c;
?>

Expected result:
----------------
Stack overflow

Actual result:
--------------
E_ERROR

Patches

__clone-2.patch (last revision 2013-02-12 23:12 UTC by krakjoe@php.net)
__clone.patch (last revision 2013-02-12 14:29 UTC by pthreads at pthreads dot org)

Pull Requests

History

AllCommentsChangesGit/SVN commitsRelated reports
 [2013-02-12 14:45 UTC] nikic@php.net 
zend_object.guards is for property guards. Wouldn't you be clashing with the guard for $__clone here?

Also, I'm not convinced we need to add this check at all. Recursion is a valid means of programming and as long as there is some termination condition everything's okay. Arguably with "clone" recursion makes rather little sense, but as it stands now we are open to recursion everywhere and I don't think we should go down the patch of saying "recursion is okay here, but it's not okay here". I mean, "include" for example can also be used recursively, even though you might argue that that's nearly as useless. Should we be adding checks everywhere, where we think recursion makes too little sense? I don't think so.

The only (calls) that currently use recursion guarding are __get/__set and friends. For those it makes sense because the recursion guarding gives access to the underlying property, so it has some actual function (rather than just forbidding a programming pattern).
 [2013-02-12 20:03 UTC] krakjoe@php.net 
The following patch has been added/updated:

Patch Name: __clone-2.patch
Revision:   1360699422
URL:        https://bugs.php.net/patch-display.php?bug=64196&patch=__clone-2.patch&revision=1360699422
 [2013-02-12 20:05 UTC] krakjoe@php.net 
__clone-2.patch addresses the clash ... and ensures proper functionality in all 
situations, not just basic examples.

we don't seem to be able to agree on whether such checks should be made, but at 
least now there are no clashes and the patch is correct ...
 [2013-02-12 21:39 UTC] nikic@php.net 
Not sure in what way the new patch resolves the clash. Doesn't it just move it from "$foo->__clone" towards "$foo->{'$__clone'}"?
 [2013-02-12 23:12 UTC] krakjoe@php.net 
The following patch has been added/updated:

Patch Name: __clone-2.patch
Revision:   1360710727
URL:        https://bugs.php.net/patch-display.php?bug=64196&patch=__clone-2.patch&revision=1360710727
 [2016-03-15 17:17 UTC] nikic@php.net 
Related To: Bug #71830
 [2016-08-17 20:56 UTC] nikic@php.net 
Related To: Bug #72871
 [2016-08-17 21:12 UTC] brian dot carpenter at gmail dot com 
Related To: Bug #72871
 [2017-08-12 16:50 UTC] nikic@php.net 
Related To: Bug #74948
 [2017-11-10 11:08 UTC] nikic@php.net 
Related To: Bug #75509
 [2018-01-04 23:16 UTC] requinix@php.net 
Related To: Bug #75762
 [2018-01-04 23:16 UTC] requinix@php.net 
Still present in 7.2.
 [2018-11-28 23:08 UTC] nikic@php.net 
Related To: Bug #77216
 [2019-03-20 14:21 UTC] dams@php.net 
Still present in 7.3
 [2019-10-21 07:56 UTC] nikic@php.net
-Summary: __clone recursion stack overflow +Summary: Indirect/magic call recursion stack overflow
 [2019-10-21 07:57 UTC] nikic@php.net 
Related To: Bug #78702
 [2019-10-21 08:00 UTC] nikic@php.net 
Related To: Bug #78704
 [2019-10-21 08:02 UTC] nikic@php.net 
Related To: Bug #78705
 [2019-12-13 09:45 UTC] nikic@php.net 
Related To: Bug #78956
 [2020-02-04 14:06 UTC] nikic@php.net 
Related To: Bug #79207
 [2020-06-01 11:12 UTC] nikic@php.net 
Related To: Bug #79658
 [2020-06-25 08:33 UTC] nikic@php.net 
Related To: Bug #79732
 [2020-07-04 00:42 UTC] requinix@php.net 
Related To: Bug #79780
 [2021-05-20 22:16 UTC] nikic@php.net 
Related To: Bug #81062
 [2021-08-19 11:14 UTC] cmb@php.net 
Related To: Bug #81373
 [2021-10-08 13:51 UTC] cmb@php.net 
Related To: Bug #75899
 [2021-11-18 15:13 UTC] cmb@php.net 
Related To: Bug #81636
 [2021-12-17 18:31 UTC] cmb@php.net 
Related To: Bug #81701
 [2021-12-17 18:41 UTC] cmb@php.net 
Related To: Bug #81700
 [2022-05-13 11:15 UTC] cmb@php.net 
Related To: Bug #81717
 [2022-12-16 17:24 UTC] lbarnaud@php.net
-Status: Open +Status: Closed -Assigned To: +Assigned To: lbarnaud
 [2022-12-16 17:24 UTC] lbarnaud@php.net 
The fix for this bug has been committed.
If you are still experiencing this bug, try to check out latest source from https://github.com/php/php-src and re-test.
Thank you for the report, and for helping us make PHP better.

This should be fixed by https://github.com/php/php-src/pull/9104
 
PHP Copyright © 2001-2024 The PHP Group
All rights reserved. 		Last updated: Thu Nov 21 13:01:29 2024 UTC