Skip to content

Commit 28ed4e6

Browse files
committed
Merge branch 'PHP-8.3' into PHP-8.4
* PHP-8.3: Prevent operands from being released during comparison
2 parents 138ebf4 + bc4b6ce commit 28ed4e6

File tree

7 files changed

+134
-8
lines changed

7 files changed

+134
-8
lines changed

NEWS

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,8 @@ PHP NEWS
1313
unpacking). (ilutov)
1414
. Fixed OSS-Fuzz #434346548 (Failed assertion with throwing __toString in
1515
binary const expr). (ilutov)
16+
. Fixed bug GH-19305 (Operands may be being released during comparison).
17+
(Arnaud)
1618

1719
- FTP:
1820
. Fix theoretical issues with hrtime() not being available. (nielsdos)

Zend/tests/gh19305-001.phpt

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
--TEST--
2+
GH-19305 001: Operands may be released during comparison
3+
--FILE--
4+
<?php
5+
6+
$a = (object)[
7+
'foo' => 'test',
8+
'bar' => 2,
9+
];
10+
$b = (object)[
11+
'foo' => new class {
12+
public function __toString() {
13+
global $a, $b;
14+
$a = $b = null;
15+
return '';
16+
}
17+
},
18+
'bar' => 2,
19+
];
20+
21+
// Comparison of $a->foo and $b->foo calls __toString(), which releases
22+
// both $a and $b.
23+
var_dump($a > $b);
24+
25+
?>
26+
--EXPECT--
27+
bool(true)

Zend/tests/gh19305-002.phpt

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
--TEST--
2+
GH-19305 002: Operands may be released during comparison
3+
--FILE--
4+
<?php
5+
6+
$a = [
7+
'foo' => 'test',
8+
'bar' => 2,
9+
];
10+
$b = [
11+
'foo' => new class {
12+
public function __toString() {
13+
global $a, $b;
14+
$a = $b = null;
15+
return '';
16+
}
17+
},
18+
'bar' => 2,
19+
];
20+
21+
// Comparison of $a['foo'] and $b['foo'] calls __toString(), which releases
22+
// both $a and $b.
23+
var_dump($a > $b);
24+
25+
?>
26+
--EXPECT--
27+
bool(true)

Zend/tests/gh19305-003.phpt

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
--TEST--
2+
GH-19305 003: Operands may be released during comparison
3+
--SKIPIF--
4+
<?php
5+
if (!method_exists('ReflectionClass', 'newLazyGhost')) {
6+
die('skip No lazy objects');
7+
}
8+
?>
9+
--FILE--
10+
<?php
11+
12+
class C
13+
{
14+
public $s;
15+
}
16+
$r = new ReflectionClass(C::class);
17+
$o = $r->newLazyProxy(function () { return new C; });
18+
19+
// Comparison calls initializers, which releases $o
20+
var_dump($o >
21+
$r->newLazyGhost(function () {
22+
global $o;
23+
$o = null;
24+
}));
25+
26+
?>
27+
--EXPECT--
28+
bool(false)

Zend/zend_object_handlers.c

Lines changed: 25 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -2202,6 +2202,10 @@ ZEND_API int zend_std_compare_objects(zval *o1, zval *o2) /* {{{ */
22022202
}
22032203
Z_PROTECT_RECURSION_P(o1);
22042204

2205+
GC_ADDREF(zobj1);
2206+
GC_ADDREF(zobj2);
2207+
int ret;
2208+
22052209
for (i = 0; i < zobj1->ce->default_properties_count; i++) {
22062210
zval *p1, *p2;
22072211

@@ -2216,31 +2220,45 @@ ZEND_API int zend_std_compare_objects(zval *o1, zval *o2) /* {{{ */
22162220

22172221
if (Z_TYPE_P(p1) != IS_UNDEF) {
22182222
if (Z_TYPE_P(p2) != IS_UNDEF) {
2219-
int ret;
2220-
22212223
ret = zend_compare(p1, p2);
22222224
if (ret != 0) {
22232225
Z_UNPROTECT_RECURSION_P(o1);
2224-
return ret;
2226+
goto done;
22252227
}
22262228
} else {
22272229
Z_UNPROTECT_RECURSION_P(o1);
2228-
return 1;
2230+
ret = 1;
2231+
goto done;
22292232
}
22302233
} else {
22312234
if (Z_TYPE_P(p2) != IS_UNDEF) {
22322235
Z_UNPROTECT_RECURSION_P(o1);
2233-
return 1;
2236+
ret = 1;
2237+
goto done;
22342238
}
22352239
}
22362240
}
22372241

22382242
Z_UNPROTECT_RECURSION_P(o1);
2239-
return 0;
2243+
ret = 0;
2244+
2245+
done:
2246+
OBJ_RELEASE(zobj1);
2247+
OBJ_RELEASE(zobj2);
2248+
2249+
return ret;
22402250
} else {
2241-
return zend_compare_symbol_tables(
2251+
GC_ADDREF(zobj1);
2252+
GC_ADDREF(zobj2);
2253+
2254+
int ret = zend_compare_symbol_tables(
22422255
zend_std_get_properties_ex(zobj1),
22432256
zend_std_get_properties_ex(zobj2));
2257+
2258+
OBJ_RELEASE(zobj1);
2259+
OBJ_RELEASE(zobj2);
2260+
2261+
return ret;
22442262
}
22452263
}
22462264
/* }}} */

Zend/zend_operators.c

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3405,7 +3405,19 @@ static int hash_zval_compare_function(zval *z1, zval *z2) /* {{{ */
34053405

34063406
ZEND_API int ZEND_FASTCALL zend_compare_symbol_tables(HashTable *ht1, HashTable *ht2) /* {{{ */
34073407
{
3408-
return ht1 == ht2 ? 0 : zend_hash_compare(ht1, ht2, (compare_func_t) hash_zval_compare_function, 0);
3408+
if (ht1 == ht2) {
3409+
return 0;
3410+
}
3411+
3412+
GC_TRY_ADDREF(ht1);
3413+
GC_TRY_ADDREF(ht2);
3414+
3415+
int ret = zend_hash_compare(ht1, ht2, (compare_func_t) hash_zval_compare_function, 0);
3416+
3417+
GC_TRY_DTOR_NO_REF(ht1);
3418+
GC_TRY_DTOR_NO_REF(ht2);
3419+
3420+
return ret;
34093421
}
34103422
/* }}} */
34113423

Zend/zend_types.h

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -733,6 +733,18 @@ static zend_always_inline uint8_t zval_get_type(const zval* pz) {
733733
} \
734734
} while (0)
735735

736+
#define GC_TRY_DTOR_NO_REF(p) \
737+
do { \
738+
zend_refcounted_h *_p = &(p)->gc; \
739+
if (!(_p->u.type_info & GC_IMMUTABLE)) { \
740+
if (zend_gc_delref(_p) == 0) { \
741+
rc_dtor_func((zend_refcounted *)_p); \
742+
} else { \
743+
gc_check_possible_root_no_ref((zend_refcounted *)_p); \
744+
} \
745+
} \
746+
} while (0)
747+
736748
#define GC_TYPE_MASK 0x0000000f
737749
#define GC_FLAGS_MASK 0x000003f0
738750
#define GC_INFO_MASK 0xfffffc00

0 commit comments

Comments
 (0)