php.net |  support |  documentation |  report a bug |  advanced search |  search howto |  statistics |  random bug |  login
Bug #65821 By-ref foreach on property access of string offset segfaults
Submitted: 2013-10-02 21:23 UTC Modified: 2013-10-04 10:39 UTC
From: nikic@php.net Assigned: nikic (profile)
Status: Closed Package: Scripting Engine problem
PHP Version: 5.5.4 OS:
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: nikic@php.net
New email:
PHP Version: OS:

 

 [2013-10-02 21:23 UTC] nikic@php.net
Description:
------------
This segfaults:

$str = "foo";
foreach ($str[0]->bar as &$baz) {}

Because http://lxr.php.net/xref/PHP_TRUNK/Zend/zend_vm_def.h#1391 uses var.ptr_ptr without NULL check (FETCH_OBJ_W with ZEND_FETCH_ADD_LOCK).


Patches

Add a Patch

Pull Requests

Add a Pull Request

History

AllCommentsChangesGit/SVN commitsRelated reports
 [2013-10-02 21:27 UTC] nikic@php.net
-Assigned To: +Assigned To: dmitry
 [2013-10-02 21:27 UTC] nikic@php.net
@dmity: Could you please take a look whether the ZEND_FETCH_ADD_LOCK in foreach [1] is still necessary? I tried removing it [2] and I didn't get any test failures in Zend/ or tests/, so maybe this is just a leftover?

 [1]: http://lxr.php.net/xref/PHP_TRUNK/Zend/zend_compile.c#6237
 [2]: https://gist.github.com/nikic/6800754
 [2013-10-04 10:39 UTC] dmitry@php.net
-Assigned To: dmitry +Assigned To: nikic
 [2013-10-04 10:39 UTC] dmitry@php.net
Nikita, I think you are right, and ZEND_FETCH_ADD_LOCK may be removed, because now the situation that it handled resolved by the code at the end of the handler, anyway.

I mean the situation when array might be destroyed right in ZEND_FETCH_OBJ_W handler and EX_T(opline->result.var).var.ptr_ptr would be incorrect. e.g.

<?php
function foo() {
	return array((object)array('x'=>array('a','b','c')));
}

foreach (foo()[0]->x as &$x) {
	echo "$x\n";
}
?>

Now it must be handled by:

	if (OP1_TYPE == IS_VAR && OP1_FREE && READY_TO_DESTROY(free_op1.var)) {
		EXTRACT_ZVAL_PTR(&EX_T(opline->result.var));
	}

So, your patch looks fine (I hope I didn't miss anything important)
Fell free to commit it into PHP-5.5 and above.

Please, also check if we need to set "opline->extended_value = 1" for ZEND_FREE/ZEND_SWITCH_FREE opcodes in generate_free_foreach_copy().
I think we don't need it anymore as well.
 [2013-10-04 11:17 UTC] nikic@php.net
Automatic comment on behalf of nikic
Revision: http://git.php.net/?p=php-src.git;a=commit;h=536260f2c52af7057a657af96d991acf27c0cc86
Log: Fix bug #65821: By-ref foreach on property access of string offset segfaults
 [2013-10-04 11:17 UTC] nikic@php.net
-Status: Assigned +Status: Closed
 [2013-11-17 09:30 UTC] laruence@php.net
Automatic comment on behalf of nikic
Revision: http://git.php.net/?p=php-src.git;a=commit;h=536260f2c52af7057a657af96d991acf27c0cc86
Log: Fix bug #65821: By-ref foreach on property access of string offset segfaults
 
PHP Copyright © 2001-2024 The PHP Group
All rights reserved.
Last updated: Wed Apr 24 09:01:28 2024 UTC