Skip to content

Commit 65ae09b

Browse files
nielsdosnielsdos
committed
Fix GH-17442: Engine UAF with reference assign and dtor
1 parent 75d7684 commit 65ae09b

File tree

7 files changed

+111
-22
lines changed

7 files changed

+111
-22
lines changed

‎UPGRADING.INTERNALS

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,11 @@ PHP 8.5 INTERNALS UPGRADE NOTES
1414
1. Internal API changes
1515
========================
1616

17+
- Zend
18+
. Added ZEND_SAFE_ASSIGN_VALUE() macro to safely assign a value to a zval.
19+
. Added zval_ptr_safe_dtor() to safely destroy a zval when a destructor
20+
could interfere.
21+
1722
========================
1823
2. Build system changes
1924
========================

‎Zend/tests/weakrefs/gh17442_1.phpt

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
--TEST--
2+
GH-17442 (Engine UAF with reference assign and dtor) - untyped
3+
--CREDITS--
4+
YuanchengJiang
5+
--FILE--
6+
<?php
7+
$map = new WeakMap;
8+
$obj = new stdClass;
9+
$map[$obj] = new class {
10+
function __destruct() {
11+
throw new Exception("Test");
12+
}
13+
};
14+
headers_sent($obj,$generator);
15+
?>
16+
--EXPECTF--
17+
Fatal error: Uncaught Exception: Test in %s:%d
18+
Stack trace:
19+
#0 [internal function]: class@anonymous->__destruct()
20+
#1 %s(%d): headers_sent(NULL, 0)
21+
#2 {main}
22+
thrown in %s on line %d

‎Zend/tests/weakrefs/gh17442_2.phpt

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
1+
--TEST--
2+
GH-17442 (Engine UAF with reference assign and dtor) - typed
3+
--CREDITS--
4+
YuanchengJiang
5+
nielsdos
6+
--FILE--
7+
<?php
8+
$map = new WeakMap;
9+
10+
class Test {
11+
public stdClass|string $obj;
12+
}
13+
14+
$test = new Test;
15+
$test->obj = new stdClass;
16+
17+
$map[$test->obj] = new class {
18+
function __destruct() {
19+
global $test;
20+
var_dump($test->obj);
21+
throw new Exception("Test");
22+
}
23+
};
24+
25+
headers_sent($test->obj);
26+
?>
27+
--EXPECTF--
28+
string(0) ""
29+
30+
Fatal error: Uncaught Exception: Test in %s:%d
31+
Stack trace:
32+
#0 [internal function]: class@anonymous->__destruct()
33+
#1 %s(%d): headers_sent('')
34+
#2 {main}
35+
thrown in %s on line %d

‎Zend/zend_API.c

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4666,8 +4666,7 @@ ZEND_API zend_result zend_try_assign_typed_ref_ex(zend_reference *ref, zval *val
46664666
zval_ptr_dtor(val);
46674667
return FAILURE;
46684668
} else {
4669-
zval_ptr_dtor(&ref->val);
4670-
ZVAL_COPY_VALUE(&ref->val, val);
4669+
ZEND_SAFE_ASSIGN_VALUE(&ref->val, val);
46714670
return SUCCESS;
46724671
}
46734672
}

‎Zend/zend_API.h

Lines changed: 33 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -1097,6 +1097,22 @@ ZEND_API zend_result zend_try_assign_typed_ref_res(zend_reference *ref, zend_res
10971097
ZEND_API zend_result zend_try_assign_typed_ref_zval(zend_reference *ref, zval *zv);
10981098
ZEND_API zend_result zend_try_assign_typed_ref_zval_ex(zend_reference *ref, zval *zv, bool strict);
10991099

1100+
#define ZEND_SAFE_ASSIGN_VALUE(zv, value) do { \
1101+
zval *_zv = zv; \
1102+
zval *_value = value; \
1103+
if (Z_REFCOUNTED_P(_zv)) { \
1104+
zend_refcounted *rc = Z_COUNTED_P(_zv); \
1105+
ZVAL_COPY_VALUE(_zv, _value); \
1106+
if (!GC_DELREF(rc)) { \
1107+
rc_dtor_func(rc); \
1108+
} else { \
1109+
gc_check_possible_root(rc); \
1110+
} \
1111+
} else { \
1112+
ZVAL_COPY_VALUE(_zv, _value); \
1113+
} \
1114+
} while (0)
1115+
11001116
#define _ZEND_TRY_ASSIGN_NULL(zv, is_ref) do { \
11011117
zval *_zv = zv; \
11021118
if (is_ref || UNEXPECTED(Z_ISREF_P(_zv))) { \
@@ -1107,7 +1123,7 @@ ZEND_API zend_result zend_try_assign_typed_ref_zval_ex(zend_reference *ref, zval
11071123
} \
11081124
_zv = &ref->val; \
11091125
} \
1110-
zval_ptr_dtor(_zv); \
1126+
zval_ptr_safe_dtor(_zv); \
11111127
ZVAL_NULL(_zv); \
11121128
} while (0)
11131129

@@ -1129,7 +1145,7 @@ ZEND_API zend_result zend_try_assign_typed_ref_zval_ex(zend_reference *ref, zval
11291145
} \
11301146
_zv = &ref->val; \
11311147
} \
1132-
zval_ptr_dtor(_zv); \
1148+
zval_ptr_safe_dtor(_zv); \
11331149
ZVAL_FALSE(_zv); \
11341150
} while (0)
11351151

@@ -1151,7 +1167,7 @@ ZEND_API zend_result zend_try_assign_typed_ref_zval_ex(zend_reference *ref, zval
11511167
} \
11521168
_zv = &ref->val; \
11531169
} \
1154-
zval_ptr_dtor(_zv); \
1170+
zval_ptr_safe_dtor(_zv); \
11551171
ZVAL_TRUE(_zv); \
11561172
} while (0)
11571173

@@ -1173,7 +1189,7 @@ ZEND_API zend_result zend_try_assign_typed_ref_zval_ex(zend_reference *ref, zval
11731189
} \
11741190
_zv = &ref->val; \
11751191
} \
1176-
zval_ptr_dtor(_zv); \
1192+
zval_ptr_safe_dtor(_zv); \
11771193
ZVAL_BOOL(_zv, bval); \
11781194
} while (0)
11791195

@@ -1195,7 +1211,7 @@ ZEND_API zend_result zend_try_assign_typed_ref_zval_ex(zend_reference *ref, zval
11951211
} \
11961212
_zv = &ref->val; \
11971213
} \
1198-
zval_ptr_dtor(_zv); \
1214+
zval_ptr_safe_dtor(_zv); \
11991215
ZVAL_LONG(_zv, lval); \
12001216
} while (0)
12011217

@@ -1217,7 +1233,7 @@ ZEND_API zend_result zend_try_assign_typed_ref_zval_ex(zend_reference *ref, zval
12171233
} \
12181234
_zv = &ref->val; \
12191235
} \
1220-
zval_ptr_dtor(_zv); \
1236+
zval_ptr_safe_dtor(_zv); \
12211237
ZVAL_DOUBLE(_zv, dval); \
12221238
} while (0)
12231239

@@ -1239,7 +1255,7 @@ ZEND_API zend_result zend_try_assign_typed_ref_zval_ex(zend_reference *ref, zval
12391255
} \
12401256
_zv = &ref->val; \
12411257
} \
1242-
zval_ptr_dtor(_zv); \
1258+
zval_ptr_safe_dtor(_zv); \
12431259
ZVAL_EMPTY_STRING(_zv); \
12441260
} while (0)
12451261

@@ -1261,7 +1277,7 @@ ZEND_API zend_result zend_try_assign_typed_ref_zval_ex(zend_reference *ref, zval
12611277
} \
12621278
_zv = &ref->val; \
12631279
} \
1264-
zval_ptr_dtor(_zv); \
1280+
zval_ptr_safe_dtor(_zv); \
12651281
ZVAL_STR(_zv, str); \
12661282
} while (0)
12671283

@@ -1283,7 +1299,7 @@ ZEND_API zend_result zend_try_assign_typed_ref_zval_ex(zend_reference *ref, zval
12831299
} \
12841300
_zv = &ref->val; \
12851301
} \
1286-
zval_ptr_dtor(_zv); \
1302+
zval_ptr_safe_dtor(_zv); \
12871303
ZVAL_NEW_STR(_zv, str); \
12881304
} while (0)
12891305

@@ -1305,7 +1321,7 @@ ZEND_API zend_result zend_try_assign_typed_ref_zval_ex(zend_reference *ref, zval
13051321
} \
13061322
_zv = &ref->val; \
13071323
} \
1308-
zval_ptr_dtor(_zv); \
1324+
zval_ptr_safe_dtor(_zv); \
13091325
ZVAL_STRING(_zv, string); \
13101326
} while (0)
13111327

@@ -1327,7 +1343,7 @@ ZEND_API zend_result zend_try_assign_typed_ref_zval_ex(zend_reference *ref, zval
13271343
} \
13281344
_zv = &ref->val; \
13291345
} \
1330-
zval_ptr_dtor(_zv); \
1346+
zval_ptr_safe_dtor(_zv); \
13311347
ZVAL_STRINGL(_zv, string, len); \
13321348
} while (0)
13331349

@@ -1349,7 +1365,7 @@ ZEND_API zend_result zend_try_assign_typed_ref_zval_ex(zend_reference *ref, zval
13491365
} \
13501366
_zv = &ref->val; \
13511367
} \
1352-
zval_ptr_dtor(_zv); \
1368+
zval_ptr_safe_dtor(_zv); \
13531369
ZVAL_ARR(_zv, arr); \
13541370
} while (0)
13551371

@@ -1371,7 +1387,7 @@ ZEND_API zend_result zend_try_assign_typed_ref_zval_ex(zend_reference *ref, zval
13711387
} \
13721388
_zv = &ref->val; \
13731389
} \
1374-
zval_ptr_dtor(_zv); \
1390+
zval_ptr_safe_dtor(_zv); \
13751391
ZVAL_RES(_zv, res); \
13761392
} while (0)
13771393

@@ -1393,7 +1409,7 @@ ZEND_API zend_result zend_try_assign_typed_ref_zval_ex(zend_reference *ref, zval
13931409
} \
13941410
_zv = &ref->val; \
13951411
} \
1396-
zval_ptr_dtor(_zv); \
1412+
zval_ptr_safe_dtor(_zv); \
13971413
ZVAL_COPY_VALUE(_zv, other_zv); \
13981414
} while (0)
13991415

@@ -1415,7 +1431,7 @@ ZEND_API zend_result zend_try_assign_typed_ref_zval_ex(zend_reference *ref, zval
14151431
} \
14161432
_zv = &ref->val; \
14171433
} \
1418-
zval_ptr_dtor(_zv); \
1434+
zval_ptr_safe_dtor(_zv); \
14191435
ZVAL_COPY_VALUE(_zv, other_zv); \
14201436
} while (0)
14211437

@@ -1447,7 +1463,7 @@ ZEND_API zend_result zend_try_assign_typed_ref_zval_ex(zend_reference *ref, zval
14471463
} \
14481464
_zv = &ref->val; \
14491465
} \
1450-
zval_ptr_dtor(_zv); \
1466+
zval_ptr_safe_dtor(_zv); \
14511467
ZVAL_COPY_VALUE(_zv, other_zv); \
14521468
} while (0)
14531469

@@ -1485,10 +1501,7 @@ static zend_always_inline zval *zend_try_array_init_size(zval *zv, uint32_t size
14851501
}
14861502
zv = &ref->val;
14871503
}
1488-
zval garbage;
1489-
ZVAL_COPY_VALUE(&garbage, zv);
1490-
ZVAL_NULL(zv);
1491-
zval_ptr_dtor(&garbage);
1504+
zval_ptr_safe_dtor(zv);
14921505
ZVAL_ARR(zv, arr);
14931506
return zv;
14941507
}

‎Zend/zend_variables.c

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -85,6 +85,20 @@ ZEND_API void zval_ptr_dtor(zval *zval_ptr) /* {{{ */
8585
}
8686
/* }}} */
8787

88+
ZEND_API void zval_ptr_safe_dtor(zval *zval_ptr)
89+
{
90+
if (Z_REFCOUNTED_P(zval_ptr)) {
91+
zend_refcounted *ref = Z_COUNTED_P(zval_ptr);
92+
93+
if (GC_DELREF(ref) == 0) {
94+
ZVAL_NULL(zval_ptr);
95+
rc_dtor_func(ref);
96+
} else {
97+
gc_check_possible_root(ref);
98+
}
99+
}
100+
}
101+
88102
ZEND_API void zval_internal_ptr_dtor(zval *zval_ptr) /* {{{ */
89103
{
90104
if (Z_REFCOUNTED_P(zval_ptr)) {

‎Zend/zend_variables.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -78,6 +78,7 @@ static zend_always_inline void zval_ptr_dtor_str(zval *zval_ptr)
7878
}
7979

8080
ZEND_API void zval_ptr_dtor(zval *zval_ptr);
81+
ZEND_API void zval_ptr_safe_dtor(zval *zval_ptr);
8182
ZEND_API void zval_internal_ptr_dtor(zval *zvalue);
8283

8384
/* Kept for compatibility */

0 commit comments

Comments
 (0)