php.net |  support |  documentation |  report a bug |  advanced search |  search howto |  statistics |  random bug |  login
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 Add Comment Developer Edit
Welcome! If you don't have a Git account, you can't do anything here.
You can add a comment by following this link or if you reported this bug, you can edit this bug over here.
(description)
Block user comment
Status: Assign to:
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)

Add a Patch

Pull Requests

Add a Pull Request

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
 [2018-01-04 23:16 UTC] requinix@php.net
Still present in 7.2.
 [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
 [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: Tue Mar 19 10:01:30 2024 UTC