Skip to content

Commit 0ff5f92

Browse files
committed
Hold onto the container after a get_property_ptr_ptr() call
Effects performed after a get_property_ptr_ptr() call may make the property pointer invalid by freeing or reallocating its container. The container might be the object itself, the properties ht, a proxied object, or anything else (for internal classes). Here we change the get_property_ptr_ptr handler so it exposes the actual container. The caller can then increase its refcount if any operation performed before the last access to the property could render the property invalid. The get_property_ptr_ptr() implementation has the responsibility of ensuring that while the container's refcount is incremented, the property pointer remains valid.
1 parent 26c96d3 commit 0ff5f92

File tree

22 files changed

+351
-97
lines changed

22 files changed

+351
-97
lines changed

Zend/tests/gh15938-001.phpt

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,32 @@
1+
--TEST--
2+
GH-15938 001: ASSIGN_OBJ_OP: Dynamic property may be undef by __toString(), leaking the new value
3+
--ENV--
4+
LEN=10
5+
--FILE--
6+
<?php
7+
8+
#[AllowDynamicProperties]
9+
class C {
10+
public $a;
11+
}
12+
13+
$obj = new C();
14+
$obj->b = str_repeat('c', (int)getenv('LEN'));
15+
$obj->b .= new class {
16+
function __toString() {
17+
global $obj;
18+
unset($obj->b);
19+
return str_repeat('d', (int)getenv('LEN'));
20+
}
21+
};
22+
23+
var_dump($obj);
24+
25+
?>
26+
==DONE==
27+
--EXPECTF--
28+
object(C)#%d (1) {
29+
["a"]=>
30+
NULL
31+
}
32+
==DONE==

Zend/tests/gh15938-002.phpt

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,51 @@
1+
--TEST--
2+
GH-15938 002: ASSIGN_OBJ_OP: Properties ht may be resized by __toString(), write happens on freed buckets
3+
--FILE--
4+
<?php
5+
6+
#[AllowDynamicProperties]
7+
class C {
8+
public $a;
9+
}
10+
11+
$obj = new C();
12+
$obj->b = '';
13+
14+
$obj->b .= new class {
15+
function __toString() {
16+
global $obj;
17+
for ($i = 0; $i < 8; $i++) {
18+
$obj->{$i} = 0;
19+
}
20+
return 'str';
21+
}
22+
};
23+
24+
var_dump($obj);
25+
26+
?>
27+
==DONE==
28+
--EXPECTF--
29+
object(C)#%d (10) {
30+
["a"]=>
31+
NULL
32+
["b"]=>
33+
string(0) ""
34+
["0"]=>
35+
int(0)
36+
["1"]=>
37+
int(0)
38+
["2"]=>
39+
int(0)
40+
["3"]=>
41+
int(0)
42+
["4"]=>
43+
int(0)
44+
["5"]=>
45+
int(0)
46+
["6"]=>
47+
int(0)
48+
["7"]=>
49+
int(0)
50+
}
51+
==DONE==

Zend/tests/gh15938-003.phpt

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
--TEST--
2+
GH-15938 003: ASSIGN_OBJ_OP: Object may be freed by __toString(), write happens on freed object
3+
--FILE--
4+
<?php
5+
6+
class C {
7+
public $a;
8+
}
9+
10+
$obj = new C();
11+
$obj->a = '';
12+
$obj->a .= new class {
13+
function __toString() {
14+
global $obj;
15+
$obj = null;
16+
return 'str';
17+
}
18+
};
19+
20+
var_dump($obj);
21+
22+
?>
23+
==DONE==
24+
--EXPECT--
25+
NULL
26+
==DONE==

Zend/tests/gh15938-004.phpt

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
--TEST--
2+
GH-15938 001: ASSIGN_OBJ_OP: Property ht may be freed by resetAsLazy*(), write happens on freed ht
3+
--FILE--
4+
<?php
5+
6+
#[AllowDynamicProperties]
7+
class C {
8+
public $a;
9+
}
10+
11+
$obj = new C();
12+
$obj->b = str_repeat('a', 10);
13+
$obj->b .= new class {
14+
function __toString() {
15+
global $obj;
16+
$rc = new ReflectionClass($obj::class);
17+
$rc->resetAsLazyGhost($obj, function () {});
18+
return 'str';
19+
}
20+
};
21+
22+
var_dump($obj);
23+
24+
?>
25+
==DONE==
26+
--EXPECTF--
27+
lazy ghost object(C)#%d (0) {
28+
}
29+
==DONE==

Zend/zend_execute.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3612,7 +3612,7 @@ static zend_always_inline void zend_fetch_property_address(zval *result, zval *c
36123612
} else {
36133613
name = zval_get_tmp_string(prop_ptr, &tmp_name);
36143614
}
3615-
ptr = zobj->handlers->get_property_ptr_ptr(zobj, name, type, cache_slot);
3615+
ptr = zobj->handlers->get_property_ptr_ptr(zobj, name, type, cache_slot, NULL);
36163616
if (NULL == ptr) {
36173617
ptr = zobj->handlers->read_property(zobj, name, type, cache_slot, result);
36183618
if (ptr == result) {

Zend/zend_object_handlers.c

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1378,7 +1378,7 @@ ZEND_API int zend_std_has_dimension(zend_object *object, zval *offset, int check
13781378
}
13791379
/* }}} */
13801380

1381-
ZEND_API zval *zend_std_get_property_ptr_ptr(zend_object *zobj, zend_string *name, int type, void **cache_slot) /* {{{ */
1381+
ZEND_API zval *zend_std_get_property_ptr_ptr(zend_object *zobj, zend_string *name, int type, void **cache_slot, zend_refcounted **container) /* {{{ */
13821382
{
13831383
zval *retval = NULL;
13841384
uintptr_t property_offset;
@@ -1402,7 +1402,7 @@ ZEND_API zval *zend_std_get_property_ptr_ptr(zend_object *zobj, zend_string *nam
14021402
return &EG(error_zval);
14031403
}
14041404

1405-
return zend_std_get_property_ptr_ptr(zobj, name, type, cache_slot);
1405+
return zend_std_get_property_ptr_ptr(zobj, name, type, cache_slot, container);
14061406
}
14071407
if (UNEXPECTED(type == BP_VAR_RW || type == BP_VAR_R)) {
14081408
if (prop_info) {
@@ -1440,6 +1440,9 @@ ZEND_API zval *zend_std_get_property_ptr_ptr(zend_object *zobj, zend_string *nam
14401440
zobj->properties = zend_array_dup(zobj->properties);
14411441
}
14421442
if (EXPECTED((retval = zend_hash_find(zobj->properties, name)) != NULL)) {
1443+
if (container) {
1444+
*container = (zend_refcounted*)zobj->properties;
1445+
}
14431446
return retval;
14441447
}
14451448
}
@@ -1460,7 +1463,7 @@ ZEND_API zval *zend_std_get_property_ptr_ptr(zend_object *zobj, zend_string *nam
14601463
return &EG(error_zval);
14611464
}
14621465

1463-
return zend_std_get_property_ptr_ptr(zobj, name, type, cache_slot);
1466+
return zend_std_get_property_ptr_ptr(zobj, name, type, cache_slot, container);
14641467
}
14651468
if (UNEXPECTED(!zobj->properties)) {
14661469
rebuild_object_properties_internal(zobj);
@@ -1474,6 +1477,9 @@ ZEND_API zval *zend_std_get_property_ptr_ptr(zend_object *zobj, zend_string *nam
14741477
retval = &EG(error_zval);
14751478
}
14761479

1480+
if (container) {
1481+
*container = (zend_refcounted*)zobj;
1482+
}
14771483
return retval;
14781484
}
14791485
/* }}} */

Zend/zend_object_handlers.h

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -102,8 +102,11 @@ typedef void (*zend_object_write_dimension_t)(zend_object *object, zval *offset,
102102
* * &EG(error_zval), if an exception has been thrown.
103103
* * NULL, if acquiring a direct pointer is not possible.
104104
* In this case, the VM will fall back to using read_property and write_property.
105+
* If 'container' is not NULL, it is set to the actual container of the zval
106+
* pointer. The pointer remains valid as long as a reference on the container is
107+
* held by the caller.
105108
*/
106-
typedef zval *(*zend_object_get_property_ptr_ptr_t)(zend_object *object, zend_string *member, int type, void **cache_slot);
109+
typedef zval *(*zend_object_get_property_ptr_ptr_t)(zend_object *object, zend_string *member, int type, void **cache_slot, zend_refcounted **container);
107110

108111
/* Used to check if a property of the object exists */
109112
/* param has_set_exists:
@@ -259,7 +262,7 @@ ZEND_API HashTable *zend_get_properties_no_lazy_init(zend_object *zobj);
259262
ZEND_API HashTable *zend_std_get_gc(zend_object *object, zval **table, int *n);
260263
ZEND_API HashTable *zend_std_get_debug_info(zend_object *object, int *is_temp);
261264
ZEND_API zend_result zend_std_cast_object_tostring(zend_object *object, zval *writeobj, int type);
262-
ZEND_API zval *zend_std_get_property_ptr_ptr(zend_object *object, zend_string *member, int type, void **cache_slot);
265+
ZEND_API zval *zend_std_get_property_ptr_ptr(zend_object *object, zend_string *member, int type, void **cache_slot, zend_refcounted **container);
263266
ZEND_API zval *zend_std_read_property(zend_object *object, zend_string *member, int type, void **cache_slot, zval *rv);
264267
ZEND_API zval *zend_std_write_property(zend_object *object, zend_string *member, zval *value, void **cache_slot);
265268
ZEND_API int zend_std_has_property(zend_object *object, zend_string *member, int has_set_exists, void **cache_slot);

Zend/zend_vm_def.h

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1051,14 +1051,17 @@ ZEND_VM_C_LABEL(assign_op_object):
10511051
}
10521052
}
10531053
cache_slot = (OP2_TYPE == IS_CONST) ? CACHE_ADDR((opline+1)->extended_value) : _cache_slot;
1054-
if (EXPECTED((zptr = zobj->handlers->get_property_ptr_ptr(zobj, name, BP_VAR_RW, cache_slot)) != NULL)) {
1054+
zend_refcounted *container;
1055+
if (EXPECTED((zptr = zobj->handlers->get_property_ptr_ptr(zobj, name, BP_VAR_RW, cache_slot, &container)) != NULL)) {
10551056
if (UNEXPECTED(Z_ISERROR_P(zptr))) {
10561057
if (UNEXPECTED(RETURN_VALUE_USED(opline))) {
10571058
ZVAL_NULL(EX_VAR(opline->result.var));
10581059
}
10591060
} else {
10601061
zend_reference *ref;
10611062

1063+
GC_ADDREF(container);
1064+
10621065
do {
10631066
if (UNEXPECTED(Z_ISREF_P(zptr))) {
10641067
ref = Z_REF_P(zptr);
@@ -1081,6 +1084,8 @@ ZEND_VM_C_LABEL(assign_op_object):
10811084
if (UNEXPECTED(RETURN_VALUE_USED(opline))) {
10821085
ZVAL_COPY(EX_VAR(opline->result.var), zptr);
10831086
}
1087+
1088+
GC_DTOR(container);
10841089
}
10851090
} else {
10861091
zend_assign_op_overloaded_property(zobj, name, cache_slot, value OPLINE_CC EXECUTE_DATA_CC);
@@ -1328,7 +1333,7 @@ ZEND_VM_C_LABEL(pre_incdec_object):
13281333
}
13291334
}
13301335
cache_slot = (OP2_TYPE == IS_CONST) ? CACHE_ADDR(opline->extended_value) : _cache_slot;
1331-
if (EXPECTED((zptr = zobj->handlers->get_property_ptr_ptr(zobj, name, BP_VAR_RW, cache_slot)) != NULL)) {
1336+
if (EXPECTED((zptr = zobj->handlers->get_property_ptr_ptr(zobj, name, BP_VAR_RW, cache_slot, NULL)) != NULL)) {
13321337
if (UNEXPECTED(Z_ISERROR_P(zptr))) {
13331338
if (UNEXPECTED(RETURN_VALUE_USED(opline))) {
13341339
ZVAL_NULL(EX_VAR(opline->result.var));
@@ -1398,7 +1403,7 @@ ZEND_VM_C_LABEL(post_incdec_object):
13981403
}
13991404
}
14001405
cache_slot = (OP2_TYPE == IS_CONST) ? CACHE_ADDR(opline->extended_value) : _cache_slot;
1401-
if (EXPECTED((zptr = zobj->handlers->get_property_ptr_ptr(zobj, name, BP_VAR_RW, cache_slot)) != NULL)) {
1406+
if (EXPECTED((zptr = zobj->handlers->get_property_ptr_ptr(zobj, name, BP_VAR_RW, cache_slot, NULL)) != NULL)) {
14021407
if (UNEXPECTED(Z_ISERROR_P(zptr))) {
14031408
ZVAL_NULL(EX_VAR(opline->result.var));
14041409
} else {

0 commit comments

Comments
 (0)