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
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: 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

Pull Requests

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: Thu Nov 21 15:01:30 2024 UTC