Bug #69732 [Opn]: can induce segmentation fault with basic php code

Edit report at https://bugs.php.net/bug.php?id=69732&edit=1 ID: 69732 Updated by: laruence@php.net Reported by: will dot o dot west at gmail dot com Summary: can induce segmentation fault with basic php code Status: Open Type: Bug Package: Reproducible crash Operating System: OSX / Ubuntu Trusty PHP Version: 5.5.25 -Assigned To: +Assigned To: dmitry Block user comment: N Private report: N New Comment: hmm, it's not a stack overflow segfault.. it's a double free, freeing the op1 of ZEND_ASSIGN. a simple fix could be(but it break some tests, since they are not sharing one zval anymore): $ git diff diff --git a/Zend/zend_execute.c b/Zend/zend_execute.c index ac00c28..0eff680 100644 --- a/Zend/zend_execute.c +++ b/Zend/zend_execute.c @@ -896,13 +896,11 @@ static inline zval* zend_assign_to_variable(zval **variable_ptr_ptr, zval *value if (UNEXPECTED(variable_ptr == value)) { return variable_ptr; } else if (EXPECTED(!PZVAL_IS_REF(value))) { - Z_ADDREF_P(value); - *variable_ptr_ptr = value; - ZEND_ASSERT(variable_ptr != &EG(uninitialized_zval)); - GC_REMOVE_ZVAL_FROM_BUFFER(variable_ptr); - zval_dtor(variable_ptr); - efree(variable_ptr); - return value; + ZVAL_COPY_VALUE(&garbage, variable_ptr); + ZVAL_COPY_VALUE(variable_ptr, value); + zendi_zval_copy_ctor(*variable_ptr); + _zval_dtor_func(&garbage ZEND_FILE_LINE_CC); + return variable_ptr; } else { goto copy_value; } Previous Comments: ------------------------------------------------------------------------ [2015-05-30 01:27:06] requinix@php.net The segfault is because of infinite recursion: if ->foo does not exist then __get(foo) will be called, which tries to do ->foo, which does not exist so __get(foo) will be called... until PHP dies. __get() should never blindly do a ->$name for this reason. If anything it should manually test for the presence of the property, like with get_object_vars(), before attempting to return it. If there is a bug here it's because $wpq->interesting =& ret_assoc() does not behave like a regular assignment (given that ret_assoc() isn't returning by-reference), creating the property on $wpq. If you dump $wpq after this line you'll see it doesn't have a $interesting property - just $unreferenced. Making ret_assoc() return by-ref results in a fatal error "Cannot assign by reference to overloaded object". I too don't think that should be a problem if __set is not implemented and assignment should behave normally (ie, creating the property). ------------------------------------------------------------------------ [2015-05-29 16:16:40] will dot o dot west at gmail dot com Description: ------------ The following snippet is a minimal reproduction of an segv found in a WordPress site, wherein a theme was stashing data in an undeclared field of the WP_Query object, which has a __get but no __set. Its not clear whether this worked as intended in any prior versions of php. Test script: --------------- <?php class wpq { private $unreferenced; public function __get($name) { return $this->$name; } } function ret_assoc() { return array('foo' => 'bar'); } $wpq = new wpq; $wpq->interesting =& ret_assoc(); $x = $wpq->interesting; printf("%s\n", $x); Expected result: ---------------- likely printing an empty string Actual result: -------------- Segmentation fault: 11 ------------------------------------------------------------------------ -- Edit this bug report at https://bugs.php.net/bug.php?id=69732&edit=1