Skip to content

Commit 6381a16

Browse files
nikicnikic
committed
Avoid use after free in internal prop type verification
This issue only applies to debug builds: read_property can free the object, but we'd try to check the object handlers afterwards. Rewrite the check in a way that only accessed the object before the read_property call. Fixes oss-fuzz #38297.
1 parent b514e55 commit 6381a16

File tree

4 files changed

+158
-35
lines changed

4 files changed

+158
-35
lines changed

‎Zend/tests/magic_get_destroy_object.phpt

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
--TEST--
2+
Destroy object in magic __get()
3+
--FILE--
4+
<?php
5+
class Test {
6+
function __get($name) {
7+
$GLOBALS["x"] = null;
8+
}
9+
}
10+
$x = new Test;
11+
var_dump($x->prop);
12+
13+
?>
14+
--EXPECT--
15+
NULL

‎Zend/zend_execute.c

Lines changed: 0 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1194,15 +1194,6 @@ ZEND_API ZEND_COLD void zend_internal_call_arginfo_violation(zend_function *fbc)
11941194
ZSTR_VAL(fbc->common.function_name));
11951195
}
11961196

1197-
static void zend_verify_internal_read_property_type(zend_object *obj, zend_string *name, zval *val)
1198-
{
1199-
zend_property_info *prop_info =
1200-
zend_get_property_info(obj->ce, name, /* silent */ true);
1201-
if (prop_info && prop_info != ZEND_WRONG_PROPERTY_INFO && ZEND_TYPE_IS_SET(prop_info->type)) {
1202-
zend_verify_property_type(prop_info, val, /* strict */ true);
1203-
}
1204-
}
1205-
12061197
#ifndef ZEND_VERIFY_FUNC_INFO
12071198
# define ZEND_VERIFY_FUNC_INFO 0
12081199
#endif

‎Zend/zend_vm_def.h

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2104,10 +2104,19 @@ ZEND_VM_C_LABEL(fetch_obj_r_fast_copy):
21042104
}
21052105
}
21062106

2107+
#if ZEND_DEBUG
2108+
/* For non-standard object handlers, verify a declared property type in debug builds.
2109+
* Fetch prop_info before calling read_property(), as it may deallocate the object. */
2110+
zend_property_info *prop_info = NULL;
2111+
if (zobj->handlers->read_property != zend_std_read_property) {
2112+
prop_info = zend_get_property_info(zobj->ce, name, /* silent */ true);
2113+
}
2114+
#endif
21072115
retval = zobj->handlers->read_property(zobj, name, BP_VAR_R, cache_slot, EX_VAR(opline->result.var));
21082116
#if ZEND_DEBUG
2109-
if (!EG(exception) && zobj->handlers->read_property != zend_std_read_property) {
2110-
zend_verify_internal_read_property_type(zobj, name, retval);
2117+
if (!EG(exception) && prop_info && prop_info != ZEND_WRONG_PROPERTY_INFO
2118+
&& ZEND_TYPE_IS_SET(prop_info->type)) {
2119+
zend_verify_property_type(prop_info, retval, /* strict */ true);
21112120
}
21122121
#endif
21132122

‎Zend/zend_vm_execute.h

Lines changed: 132 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -6288,10 +6288,19 @@ static ZEND_VM_COLD ZEND_OPCODE_HANDLER_RET ZEND_FASTCALL ZEND_FETCH_OBJ_R_SPEC_
62886288
}
62896289
}
62906290

6291+
#if ZEND_DEBUG
6292+
/* For non-standard object handlers, verify a declared property type in debug builds.
6293+
* Fetch prop_info before calling read_property(), as it may deallocate the object. */
6294+
zend_property_info *prop_info = NULL;
6295+
if (zobj->handlers->read_property != zend_std_read_property) {
6296+
prop_info = zend_get_property_info(zobj->ce, name, /* silent */ true);
6297+
}
6298+
#endif
62916299
retval = zobj->handlers->read_property(zobj, name, BP_VAR_R, cache_slot, EX_VAR(opline->result.var));
62926300
#if ZEND_DEBUG
6293-
if (!EG(exception) && zobj->handlers->read_property != zend_std_read_property) {
6294-
zend_verify_internal_read_property_type(zobj, name, retval);
6301+
if (!EG(exception) && prop_info && prop_info != ZEND_WRONG_PROPERTY_INFO
6302+
&& ZEND_TYPE_IS_SET(prop_info->type)) {
6303+
zend_verify_property_type(prop_info, retval, /* strict */ true);
62956304
}
62966305
#endif
62976306

@@ -8608,10 +8617,19 @@ static ZEND_VM_COLD ZEND_OPCODE_HANDLER_RET ZEND_FASTCALL ZEND_FETCH_OBJ_R_SPEC_
86088617
}
86098618
}
86108619

8620+
#if ZEND_DEBUG
8621+
/* For non-standard object handlers, verify a declared property type in debug builds.
8622+
* Fetch prop_info before calling read_property(), as it may deallocate the object. */
8623+
zend_property_info *prop_info = NULL;
8624+
if (zobj->handlers->read_property != zend_std_read_property) {
8625+
prop_info = zend_get_property_info(zobj->ce, name, /* silent */ true);
8626+
}
8627+
#endif
86118628
retval = zobj->handlers->read_property(zobj, name, BP_VAR_R, cache_slot, EX_VAR(opline->result.var));
86128629
#if ZEND_DEBUG
8613-
if (!EG(exception) && zobj->handlers->read_property != zend_std_read_property) {
8614-
zend_verify_internal_read_property_type(zobj, name, retval);
8630+
if (!EG(exception) && prop_info && prop_info != ZEND_WRONG_PROPERTY_INFO
8631+
&& ZEND_TYPE_IS_SET(prop_info->type)) {
8632+
zend_verify_property_type(prop_info, retval, /* strict */ true);
86158633
}
86168634
#endif
86178635

@@ -10963,10 +10981,19 @@ static ZEND_VM_COLD ZEND_OPCODE_HANDLER_RET ZEND_FASTCALL ZEND_FETCH_OBJ_R_SPEC_
1096310981
}
1096410982
}
1096510983

10984+
#if ZEND_DEBUG
10985+
/* For non-standard object handlers, verify a declared property type in debug builds.
10986+
* Fetch prop_info before calling read_property(), as it may deallocate the object. */
10987+
zend_property_info *prop_info = NULL;
10988+
if (zobj->handlers->read_property != zend_std_read_property) {
10989+
prop_info = zend_get_property_info(zobj->ce, name, /* silent */ true);
10990+
}
10991+
#endif
1096610992
retval = zobj->handlers->read_property(zobj, name, BP_VAR_R, cache_slot, EX_VAR(opline->result.var));
1096710993
#if ZEND_DEBUG
10968-
if (!EG(exception) && zobj->handlers->read_property != zend_std_read_property) {
10969-
zend_verify_internal_read_property_type(zobj, name, retval);
10994+
if (!EG(exception) && prop_info && prop_info != ZEND_WRONG_PROPERTY_INFO
10995+
&& ZEND_TYPE_IS_SET(prop_info->type)) {
10996+
zend_verify_property_type(prop_info, retval, /* strict */ true);
1097010997
}
1097110998
#endif
1097210999

@@ -15386,10 +15413,19 @@ static ZEND_OPCODE_HANDLER_RET ZEND_FASTCALL ZEND_FETCH_OBJ_R_SPEC_TMPVAR_CONST_
1538615413
}
1538715414
}
1538815415

15416+
#if ZEND_DEBUG
15417+
/* For non-standard object handlers, verify a declared property type in debug builds.
15418+
* Fetch prop_info before calling read_property(), as it may deallocate the object. */
15419+
zend_property_info *prop_info = NULL;
15420+
if (zobj->handlers->read_property != zend_std_read_property) {
15421+
prop_info = zend_get_property_info(zobj->ce, name, /* silent */ true);
15422+
}
15423+
#endif
1538915424
retval = zobj->handlers->read_property(zobj, name, BP_VAR_R, cache_slot, EX_VAR(opline->result.var));
1539015425
#if ZEND_DEBUG
15391-
if (!EG(exception) && zobj->handlers->read_property != zend_std_read_property) {
15392-
zend_verify_internal_read_property_type(zobj, name, retval);
15426+
if (!EG(exception) && prop_info && prop_info != ZEND_WRONG_PROPERTY_INFO
15427+
&& ZEND_TYPE_IS_SET(prop_info->type)) {
15428+
zend_verify_property_type(prop_info, retval, /* strict */ true);
1539315429
}
1539415430
#endif
1539515431

@@ -16809,10 +16845,19 @@ static ZEND_OPCODE_HANDLER_RET ZEND_FASTCALL ZEND_FETCH_OBJ_R_SPEC_TMPVAR_TMPVAR
1680916845
}
1681016846
}
1681116847

16848+
#if ZEND_DEBUG
16849+
/* For non-standard object handlers, verify a declared property type in debug builds.
16850+
* Fetch prop_info before calling read_property(), as it may deallocate the object. */
16851+
zend_property_info *prop_info = NULL;
16852+
if (zobj->handlers->read_property != zend_std_read_property) {
16853+
prop_info = zend_get_property_info(zobj->ce, name, /* silent */ true);
16854+
}
16855+
#endif
1681216856
retval = zobj->handlers->read_property(zobj, name, BP_VAR_R, cache_slot, EX_VAR(opline->result.var));
1681316857
#if ZEND_DEBUG
16814-
if (!EG(exception) && zobj->handlers->read_property != zend_std_read_property) {
16815-
zend_verify_internal_read_property_type(zobj, name, retval);
16858+
if (!EG(exception) && prop_info && prop_info != ZEND_WRONG_PROPERTY_INFO
16859+
&& ZEND_TYPE_IS_SET(prop_info->type)) {
16860+
zend_verify_property_type(prop_info, retval, /* strict */ true);
1681616861
}
1681716862
#endif
1681816863

@@ -18124,10 +18169,19 @@ static ZEND_OPCODE_HANDLER_RET ZEND_FASTCALL ZEND_FETCH_OBJ_R_SPEC_TMPVAR_CV_HAN
1812418169
}
1812518170
}
1812618171

18172+
#if ZEND_DEBUG
18173+
/* For non-standard object handlers, verify a declared property type in debug builds.
18174+
* Fetch prop_info before calling read_property(), as it may deallocate the object. */
18175+
zend_property_info *prop_info = NULL;
18176+
if (zobj->handlers->read_property != zend_std_read_property) {
18177+
prop_info = zend_get_property_info(zobj->ce, name, /* silent */ true);
18178+
}
18179+
#endif
1812718180
retval = zobj->handlers->read_property(zobj, name, BP_VAR_R, cache_slot, EX_VAR(opline->result.var));
1812818181
#if ZEND_DEBUG
18129-
if (!EG(exception) && zobj->handlers->read_property != zend_std_read_property) {
18130-
zend_verify_internal_read_property_type(zobj, name, retval);
18182+
if (!EG(exception) && prop_info && prop_info != ZEND_WRONG_PROPERTY_INFO
18183+
&& ZEND_TYPE_IS_SET(prop_info->type)) {
18184+
zend_verify_property_type(prop_info, retval, /* strict */ true);
1813118185
}
1813218186
#endif
1813318187

@@ -31428,10 +31482,19 @@ static zend_always_inline ZEND_OPCODE_HANDLER_RET ZEND_FASTCALL ZEND_FETCH_OBJ_R
3142831482
}
3142931483
}
3143031484

31485+
#if ZEND_DEBUG
31486+
/* For non-standard object handlers, verify a declared property type in debug builds.
31487+
* Fetch prop_info before calling read_property(), as it may deallocate the object. */
31488+
zend_property_info *prop_info = NULL;
31489+
if (zobj->handlers->read_property != zend_std_read_property) {
31490+
prop_info = zend_get_property_info(zobj->ce, name, /* silent */ true);
31491+
}
31492+
#endif
3143131493
retval = zobj->handlers->read_property(zobj, name, BP_VAR_R, cache_slot, EX_VAR(opline->result.var));
3143231494
#if ZEND_DEBUG
31433-
if (!EG(exception) && zobj->handlers->read_property != zend_std_read_property) {
31434-
zend_verify_internal_read_property_type(zobj, name, retval);
31495+
if (!EG(exception) && prop_info && prop_info != ZEND_WRONG_PROPERTY_INFO
31496+
&& ZEND_TYPE_IS_SET(prop_info->type)) {
31497+
zend_verify_property_type(prop_info, retval, /* strict */ true);
3143531498
}
3143631499
#endif
3143731500

@@ -33286,10 +33349,19 @@ static ZEND_OPCODE_HANDLER_RET ZEND_FASTCALL ZEND_FETCH_OBJ_R_SPEC_UNUSED_TMPVAR
3328633349
}
3328733350
}
3328833351

33352+
#if ZEND_DEBUG
33353+
/* For non-standard object handlers, verify a declared property type in debug builds.
33354+
* Fetch prop_info before calling read_property(), as it may deallocate the object. */
33355+
zend_property_info *prop_info = NULL;
33356+
if (zobj->handlers->read_property != zend_std_read_property) {
33357+
prop_info = zend_get_property_info(zobj->ce, name, /* silent */ true);
33358+
}
33359+
#endif
3328933360
retval = zobj->handlers->read_property(zobj, name, BP_VAR_R, cache_slot, EX_VAR(opline->result.var));
3329033361
#if ZEND_DEBUG
33291-
if (!EG(exception) && zobj->handlers->read_property != zend_std_read_property) {
33292-
zend_verify_internal_read_property_type(zobj, name, retval);
33362+
if (!EG(exception) && prop_info && prop_info != ZEND_WRONG_PROPERTY_INFO
33363+
&& ZEND_TYPE_IS_SET(prop_info->type)) {
33364+
zend_verify_property_type(prop_info, retval, /* strict */ true);
3329333365
}
3329433366
#endif
3329533367

@@ -35756,10 +35828,19 @@ static ZEND_OPCODE_HANDLER_RET ZEND_FASTCALL ZEND_FETCH_OBJ_R_SPEC_UNUSED_CV_HAN
3575635828
}
3575735829
}
3575835830

35831+
#if ZEND_DEBUG
35832+
/* For non-standard object handlers, verify a declared property type in debug builds.
35833+
* Fetch prop_info before calling read_property(), as it may deallocate the object. */
35834+
zend_property_info *prop_info = NULL;
35835+
if (zobj->handlers->read_property != zend_std_read_property) {
35836+
prop_info = zend_get_property_info(zobj->ce, name, /* silent */ true);
35837+
}
35838+
#endif
3575935839
retval = zobj->handlers->read_property(zobj, name, BP_VAR_R, cache_slot, EX_VAR(opline->result.var));
3576035840
#if ZEND_DEBUG
35761-
if (!EG(exception) && zobj->handlers->read_property != zend_std_read_property) {
35762-
zend_verify_internal_read_property_type(zobj, name, retval);
35841+
if (!EG(exception) && prop_info && prop_info != ZEND_WRONG_PROPERTY_INFO
35842+
&& ZEND_TYPE_IS_SET(prop_info->type)) {
35843+
zend_verify_property_type(prop_info, retval, /* strict */ true);
3576335844
}
3576435845
#endif
3576535846

@@ -39895,10 +39976,19 @@ static zend_always_inline ZEND_OPCODE_HANDLER_RET ZEND_FASTCALL ZEND_FETCH_OBJ_R
3989539976
}
3989639977
}
3989739978

39979+
#if ZEND_DEBUG
39980+
/* For non-standard object handlers, verify a declared property type in debug builds.
39981+
* Fetch prop_info before calling read_property(), as it may deallocate the object. */
39982+
zend_property_info *prop_info = NULL;
39983+
if (zobj->handlers->read_property != zend_std_read_property) {
39984+
prop_info = zend_get_property_info(zobj->ce, name, /* silent */ true);
39985+
}
39986+
#endif
3989839987
retval = zobj->handlers->read_property(zobj, name, BP_VAR_R, cache_slot, EX_VAR(opline->result.var));
3989939988
#if ZEND_DEBUG
39900-
if (!EG(exception) && zobj->handlers->read_property != zend_std_read_property) {
39901-
zend_verify_internal_read_property_type(zobj, name, retval);
39989+
if (!EG(exception) && prop_info && prop_info != ZEND_WRONG_PROPERTY_INFO
39990+
&& ZEND_TYPE_IS_SET(prop_info->type)) {
39991+
zend_verify_property_type(prop_info, retval, /* strict */ true);
3990239992
}
3990339993
#endif
3990439994

@@ -43531,10 +43621,19 @@ static ZEND_OPCODE_HANDLER_RET ZEND_FASTCALL ZEND_FETCH_OBJ_R_SPEC_CV_TMPVAR_HAN
4353143621
}
4353243622
}
4353343623

43624+
#if ZEND_DEBUG
43625+
/* For non-standard object handlers, verify a declared property type in debug builds.
43626+
* Fetch prop_info before calling read_property(), as it may deallocate the object. */
43627+
zend_property_info *prop_info = NULL;
43628+
if (zobj->handlers->read_property != zend_std_read_property) {
43629+
prop_info = zend_get_property_info(zobj->ce, name, /* silent */ true);
43630+
}
43631+
#endif
4353443632
retval = zobj->handlers->read_property(zobj, name, BP_VAR_R, cache_slot, EX_VAR(opline->result.var));
4353543633
#if ZEND_DEBUG
43536-
if (!EG(exception) && zobj->handlers->read_property != zend_std_read_property) {
43537-
zend_verify_internal_read_property_type(zobj, name, retval);
43634+
if (!EG(exception) && prop_info && prop_info != ZEND_WRONG_PROPERTY_INFO
43635+
&& ZEND_TYPE_IS_SET(prop_info->type)) {
43636+
zend_verify_property_type(prop_info, retval, /* strict */ true);
4353843637
}
4353943638
#endif
4354043639

@@ -48562,10 +48661,19 @@ static ZEND_OPCODE_HANDLER_RET ZEND_FASTCALL ZEND_FETCH_OBJ_R_SPEC_CV_CV_HANDLER
4856248661
}
4856348662
}
4856448663

48664+
#if ZEND_DEBUG
48665+
/* For non-standard object handlers, verify a declared property type in debug builds.
48666+
* Fetch prop_info before calling read_property(), as it may deallocate the object. */
48667+
zend_property_info *prop_info = NULL;
48668+
if (zobj->handlers->read_property != zend_std_read_property) {
48669+
prop_info = zend_get_property_info(zobj->ce, name, /* silent */ true);
48670+
}
48671+
#endif
4856548672
retval = zobj->handlers->read_property(zobj, name, BP_VAR_R, cache_slot, EX_VAR(opline->result.var));
4856648673
#if ZEND_DEBUG
48567-
if (!EG(exception) && zobj->handlers->read_property != zend_std_read_property) {
48568-
zend_verify_internal_read_property_type(zobj, name, retval);
48674+
if (!EG(exception) && prop_info && prop_info != ZEND_WRONG_PROPERTY_INFO
48675+
&& ZEND_TYPE_IS_SET(prop_info->type)) {
48676+
zend_verify_property_type(prop_info, retval, /* strict */ true);
4856948677
}
4857048678
#endif
4857148679

0 commit comments

Comments
 (0)