Skip to content

Commit

Permalink
Fixed bug #75573 (Segmentation fault in 7.1.12 and 7.0.26)
Browse files Browse the repository at this point in the history
(cherry picked from commit 3b9ba7b)
  • Loading branch information
laruence authored and weltling committed Dec 4, 2017
1 parent d6d4f2a commit d4dee4a
Show file tree
Hide file tree
Showing 2 changed files with 65 additions and 1 deletion.
64 changes: 64 additions & 0 deletions Zend/tests/bug75573.phpt
Original file line number Diff line number Diff line change
@@ -0,0 +1,64 @@
--TEST--
Bug #75573 (Segmentation fault in 7.1.12 and 7.0.26)
--FILE--
<?php

class A
{
var $_stdObject;
function initialize($properties = FALSE) {
$this->_stdObject = $properties ? (object) $properties : new stdClass();
parent::initialize();
}
function &__get($property)
{
if (isset($this->_stdObject->{$property})) {
$retval =& $this->_stdObject->{$property};
return $retval;
} else {
return NULL;
}
}
function &__set($property, $value)
{
return $this->_stdObject->{$property} = $value;
}
function __isset($property_name)
{
return isset($this->_stdObject->{$property_name});
}
}

class B extends A
{
function initialize($properties = array())
{
parent::initialize($properties);
}
function &__get($property)
{
if (isset($this->settings) && isset($this->settings[$property])) {
$retval =& $this->settings[$property];
return $retval;
} else {
return parent::__get($property);
}
}
}

$b = new B();
$b->settings = [ "foo" => "bar", "name" => "abc" ];
var_dump($b->name);
var_dump($b->settings);
?>
--EXPECTF--
Warning: Creating default object from empty value in %sbug75573.php on line %d

Notice: Only variable references should be returned by reference in %sbug75573.php on line %d
string(3) "abc"
array(2) {
["foo"]=>
string(3) "bar"
["name"]=>
string(3) "abc"
}
2 changes: 1 addition & 1 deletion Zend/zend_object_handlers.c
Original file line number Diff line number Diff line change
Expand Up @@ -602,13 +602,13 @@ zval *zend_std_read_property(zval *object, zval *member, int type, void **cache_
zval_ptr_dtor(&tmp_object);
goto exit;
} else {
zval_ptr_dtor(&tmp_object);
if (Z_STRVAL_P(member)[0] == '\0') {
if (Z_STRLEN_P(member) == 0) {
zend_throw_error(NULL, "Cannot access empty property");
retval = &EG(uninitialized_zval);
goto exit;
} else {
zval_ptr_dtor(&tmp_object);

This comment has been minimized.

Copy link
@nikic

nikic Dec 4, 2017

Member

Shouldn't this dtor be in both branches (or directly inside the if (Z_STRVAL_P(member)[0] == '\0'))?

This comment has been minimized.

Copy link
@weltling

weltling Dec 4, 2017

Contributor

It seemed safer to me to mirror the exact condition Z_STRVAL_P(member)[0] == '\0' && Z_STRLEN_P(member) != 0 from the original patch. As if Z_STRVAL_P(member)[0] == '\0' && Z_STRLEN_P(member) == 0 - the passed member zend_string must have something wrong or it could be zend_empty_string. Now thinking more - if that's not zend_empty_string, it should be ok to destruct. Testing a followup patch.

Thanks.

This comment has been minimized.

Copy link
@weltling

weltling Dec 4, 2017

Contributor

Well, CG(empty_string) is interned after all, so it's ok to put the dtor as you say, i think. Will put the after testing.

Thanks.

zend_throw_error(NULL, "Cannot access property started with '\\0'");
retval = &EG(uninitialized_zval);
goto exit;
Expand Down

0 comments on commit d4dee4a

Please sign in to comment.