From d617101c64edc380b597aa8d086ea37f147b54f0 Mon Sep 17 00:00:00 2001 From: Ilia Alshanetsky Date: Sat, 11 Jul 2026 08:28:59 -0400 Subject: [PATCH] Add destructor-hazard sections to defer reentrant destructors An internal C operation that drops a refcount while holding a raw pointer into a structure a synchronous __destruct could free is a use-after-free. Add ZEND_DTOR_HAZARD_BEGIN/END sections and a deferral gate in zend_objects_store_del: while a section is open, an object that reaches refcount zero and has a destructor is pinned with GC_ADDREF and its destructor deferred until the section closes. gc_possible_root_when_full skips synchronous collection while a section is open. Apply the sections to concat (GH-20477), zend_hash_clean (GH-22061), and unserialize teardown (GH-21824). Fixes GH-20477 Fixes GH-22061 Fixes GH-21824 --- NEWS | 6 ++++ Zend/tests/gh19999.phpt | 16 +++++++++ Zend/zend_execute_API.c | 15 +++++++++ Zend/zend_gc.c | 2 +- Zend/zend_globals.h | 6 ++++ Zend/zend_hash.c | 8 +++++ Zend/zend_objects_API.c | 41 +++++++++++++++++++++-- Zend/zend_objects_API.h | 14 ++++++++ Zend/zend_operators.c | 12 +++++++ ext/session/tests/gh22061.phpt | 27 +++++++++++++++ ext/standard/tests/serialize/gh21824.phpt | 25 ++++++++++++++ ext/standard/tests/strings/gh20477.phpt | 17 ++++++++++ ext/standard/var_unserializer.re | 2 ++ 13 files changed, 188 insertions(+), 3 deletions(-) create mode 100644 Zend/tests/gh19999.phpt create mode 100644 ext/session/tests/gh22061.phpt create mode 100644 ext/standard/tests/serialize/gh21824.phpt create mode 100644 ext/standard/tests/strings/gh20477.phpt diff --git a/NEWS b/NEWS index f40c6dfd8da6..c746620c9101 100644 --- a/NEWS +++ b/NEWS @@ -9,6 +9,10 @@ PHP NEWS . Fixed bug GH-15672 and GH-15911 (Stack overflow when an internal function recurses through zend_call_function, such as a self-attached SPL iterator). (iliaal) + . Fixed bug GH-20477 (Use-after-free in string concatenation reachable from + a destructor). (iliaal) + . Fixed bug GH-22061 (Double free via re-entrant destructor in + zend_hash_clean()). (iliaal) - Calendar: . Fixed bug GH-22602 (gregoriantojd() and juliantojd() integer overflow with @@ -80,6 +84,8 @@ PHP NEWS do_request() parameters). (David Carlier) - Standard: + . Fixed bug GH-21824 (Use-after-free in unserialize() teardown reachable from + a destructor). (iliaal) . Fixed sleep() and usleep() to reject values that overflow the underlying unsigned int timeout. (Weilin Du) . Fixed bug GH-22671 (assert.bail aborts the process when the assert callback diff --git a/Zend/tests/gh19999.phpt b/Zend/tests/gh19999.phpt new file mode 100644 index 000000000000..ca87e6df5b3f --- /dev/null +++ b/Zend/tests/gh19999.phpt @@ -0,0 +1,16 @@ +--TEST-- +GH-19999 (GC refcount assertion via object destruction during concat assignment) +--FILE-- + +--EXPECTF-- +Warning: Array to string conversion in %s on line %d +NULL diff --git a/Zend/zend_execute_API.c b/Zend/zend_execute_API.c index cb48e6234371..66e963b53a3d 100644 --- a/Zend/zend_execute_API.c +++ b/Zend/zend_execute_API.c @@ -170,6 +170,11 @@ void init_executor(void) /* {{{ */ zend_objects_store_init(&EG(objects_store), 1024); zend_lazy_objects_init(&EG(lazy_objects_store)); + EG(dtor_hazard_depth) = 0; + EG(deferred_dtors_count) = 0; + EG(deferred_dtors_capacity) = 0; + EG(deferred_dtors) = NULL; + EG(full_tables_cleanup) = 0; ZEND_ATOMIC_BOOL_INIT(&EG(vm_interrupt), false); ZEND_ATOMIC_BOOL_INIT(&EG(timed_out), false); @@ -449,6 +454,16 @@ void shutdown_executor(void) /* {{{ */ zend_stream_shutdown(); } zend_end_try(); + EG(dtor_hazard_depth) = 0; + if (EG(deferred_dtors_count)) { + zend_flush_deferred_destructors(); + } + if (EG(deferred_dtors)) { + efree(EG(deferred_dtors)); + EG(deferred_dtors) = NULL; + EG(deferred_dtors_capacity) = 0; + } + zend_shutdown_executor_values(fast_shutdown); zend_weakrefs_shutdown(); diff --git a/Zend/zend_gc.c b/Zend/zend_gc.c index 5de2b69bf568..21e3a18f0cdc 100644 --- a/Zend/zend_gc.c +++ b/Zend/zend_gc.c @@ -696,7 +696,7 @@ static zend_never_inline void ZEND_FASTCALL gc_possible_root_when_full(zend_refc ZEND_ASSERT(GC_TYPE(ref) == IS_ARRAY || GC_TYPE(ref) == IS_OBJECT); ZEND_ASSERT(GC_INFO(ref) == 0); - if (GC_G(gc_enabled) && !GC_G(gc_active)) { + if (GC_G(gc_enabled) && !GC_G(gc_active) && EG(dtor_hazard_depth) == 0) { GC_ADDREF(ref); gc_adjust_threshold(gc_collect_cycles()); if (UNEXPECTED(GC_DELREF(ref) == 0)) { diff --git a/Zend/zend_globals.h b/Zend/zend_globals.h index 61499c0cc23d..3c63d6789b29 100644 --- a/Zend/zend_globals.h +++ b/Zend/zend_globals.h @@ -261,6 +261,12 @@ struct _zend_executor_globals { zend_objects_store objects_store; zend_lazy_objects_store lazy_objects_store; + + uint32_t dtor_hazard_depth; + uint32_t deferred_dtors_count; + uint32_t deferred_dtors_capacity; + zend_object **deferred_dtors; + zend_object *exception; const zend_op *opline_before_exception; zend_op exception_op[3]; diff --git a/Zend/zend_hash.c b/Zend/zend_hash.c index acc342bc267d..c738a216bb96 100644 --- a/Zend/zend_hash.c +++ b/Zend/zend_hash.c @@ -1886,7 +1886,12 @@ ZEND_API void ZEND_FASTCALL zend_hash_clean(HashTable *ht) IS_CONSISTENT(ht); HT_ASSERT_RC1(ht); + bool dtor_deferred = false; if (ht->nNumUsed) { + if (ht->pDestructor) { + ZEND_DTOR_HAZARD_BEGIN(); + dtor_deferred = true; + } if (HT_IS_PACKED(ht)) { zval *zv = ht->arPacked; zval *end = zv + ht->nNumUsed; @@ -1956,6 +1961,9 @@ ZEND_API void ZEND_FASTCALL zend_hash_clean(HashTable *ht) ht->nNumOfElements = 0; ht->nNextFreeElement = ZEND_LONG_MIN; ht->nInternalPointer = 0; + if (dtor_deferred) { + ZEND_DTOR_HAZARD_END(); + } } ZEND_API void ZEND_FASTCALL zend_symtable_clean(HashTable *ht) diff --git a/Zend/zend_objects_API.c b/Zend/zend_objects_API.c index 537cad8a3644..49acb2b139ad 100644 --- a/Zend/zend_objects_API.c +++ b/Zend/zend_objects_API.c @@ -156,6 +156,35 @@ ZEND_API void ZEND_FASTCALL zend_objects_store_put(zend_object *object) EG(objects_store).object_buckets[handle] = object; } +ZEND_API void ZEND_FASTCALL zend_defer_destructor(zend_object *object) /* {{{ */ +{ + if (UNEXPECTED(EG(deferred_dtors_count) == EG(deferred_dtors_capacity))) { + uint32_t new_capacity = EG(deferred_dtors_capacity) ? EG(deferred_dtors_capacity) * 2 : 16; + EG(deferred_dtors) = erealloc(EG(deferred_dtors), new_capacity * sizeof(zend_object*)); + EG(deferred_dtors_capacity) = new_capacity; + } + EG(deferred_dtors)[EG(deferred_dtors_count)++] = object; +} +/* }}} */ + +ZEND_API void zend_flush_deferred_destructors(void) /* {{{ */ +{ + while (EG(deferred_dtors_count)) { + zend_object **batch = EG(deferred_dtors); + uint32_t count = EG(deferred_dtors_count); + + EG(deferred_dtors) = NULL; + EG(deferred_dtors_count) = 0; + EG(deferred_dtors_capacity) = 0; + + for (uint32_t i = 0; i < count; i++) { + OBJ_RELEASE(batch[i]); + } + efree(batch); + } +} +/* }}} */ + ZEND_API void ZEND_FASTCALL zend_objects_store_del(zend_object *object) /* {{{ */ { ZEND_ASSERT(GC_REFCOUNT(object) == 0); @@ -170,10 +199,18 @@ ZEND_API void ZEND_FASTCALL zend_objects_store_del(zend_object *object) /* {{{ * when the refcount reaches 0 a second time */ if (!(OBJ_FLAGS(object) & IS_OBJ_DESTRUCTOR_CALLED)) { + bool has_dtor = (object->handlers->dtor_obj != zend_objects_destroy_object + || object->ce->destructor); + + if (UNEXPECTED(has_dtor && EG(dtor_hazard_depth) != 0)) { + GC_ADDREF(object); + zend_defer_destructor(object); + return; + } + GC_ADD_FLAGS(object, IS_OBJ_DESTRUCTOR_CALLED); - if (object->handlers->dtor_obj != zend_objects_destroy_object - || object->ce->destructor) { + if (has_dtor) { GC_SET_REFCOUNT(object, 1); object->handlers->dtor_obj(object); GC_DELREF(object); diff --git a/Zend/zend_objects_API.h b/Zend/zend_objects_API.h index 434ac4499e7f..23f34a1146ce 100644 --- a/Zend/zend_objects_API.h +++ b/Zend/zend_objects_API.h @@ -62,6 +62,20 @@ ZEND_API void ZEND_FASTCALL zend_objects_store_destroy(zend_objects_store *objec ZEND_API void ZEND_FASTCALL zend_objects_store_put(zend_object *object); ZEND_API void ZEND_FASTCALL zend_objects_store_del(zend_object *object); +ZEND_API void ZEND_FASTCALL zend_defer_destructor(zend_object *object); +ZEND_API void zend_flush_deferred_destructors(void); + +#define ZEND_DTOR_HAZARD_BEGIN() do { \ + EG(dtor_hazard_depth)++; \ + } while (0) + +#define ZEND_DTOR_HAZARD_END() do { \ + ZEND_ASSERT(EG(dtor_hazard_depth) > 0); \ + if (--EG(dtor_hazard_depth) == 0 && EG(deferred_dtors_count)) { \ + zend_flush_deferred_destructors(); \ + } \ + } while (0) + /* Called when the ctor was terminated by an exception */ static zend_always_inline void zend_object_store_ctor_failed(zend_object *obj) { diff --git a/Zend/zend_operators.c b/Zend/zend_operators.c index ab8f2c2b54f8..28c2ad70b0ea 100644 --- a/Zend/zend_operators.c +++ b/Zend/zend_operators.c @@ -2075,9 +2075,12 @@ ZEND_API zend_result ZEND_FASTCALL concat_function(zval *result, zval *op1, zval } while (0); has_op2_string:; + bool inner_dtor_deferred = false; if (UNEXPECTED(ZSTR_LEN(op1_string) == 0)) { if (EXPECTED(result != op2 || Z_TYPE_P(result) != IS_STRING)) { if (result == orig_op1) { + ZEND_DTOR_HAZARD_BEGIN(); + inner_dtor_deferred = true; i_zval_ptr_dtor(result); } if (free_op2_string) { @@ -2091,6 +2094,8 @@ has_op2_string:; } else if (UNEXPECTED(ZSTR_LEN(op2_string) == 0)) { if (EXPECTED(result != op1 || Z_TYPE_P(result) != IS_STRING)) { if (result == orig_op1) { + ZEND_DTOR_HAZARD_BEGIN(); + inner_dtor_deferred = true; i_zval_ptr_dtor(result); } if (free_op1_string) { @@ -2121,6 +2126,8 @@ has_op2_string:; if (result == op1) { /* Destroy the old result first to drop the refcount, such that $x .= ...; may happen in-place. */ if (free_op1_string) { + ZEND_DTOR_HAZARD_BEGIN(); + inner_dtor_deferred = true; /* op1_string will be used as the result, so we should not free it */ i_zval_ptr_dtor(result); /* Set it to NULL in case that the extension will throw an out-of-memory error. @@ -2142,6 +2149,8 @@ has_op2_string:; result_str = zend_string_alloc(result_len, 0); memcpy(ZSTR_VAL(result_str), ZSTR_VAL(op1_string), op1_len); if (result == orig_op1) { + ZEND_DTOR_HAZARD_BEGIN(); + inner_dtor_deferred = true; i_zval_ptr_dtor(result); } } @@ -2151,6 +2160,9 @@ has_op2_string:; memcpy(ZSTR_VAL(result_str) + op1_len, ZSTR_VAL(op2_string), op2_len); ZSTR_VAL(result_str)[result_len] = '\0'; } + if (inner_dtor_deferred) { + ZEND_DTOR_HAZARD_END(); + } if (free_op1_string) zend_string_release_ex(op1_string, false); if (free_op2_string) zend_string_release_ex(op2_string, false); diff --git a/ext/session/tests/gh22061.phpt b/ext/session/tests/gh22061.phpt new file mode 100644 index 000000000000..507845c76a10 --- /dev/null +++ b/ext/session/tests/gh22061.phpt @@ -0,0 +1,27 @@ +--TEST-- +GH-22061 (Double free via re-entrant destructor in zend_hash_clean via session_unset) +--EXTENSIONS-- +session +--SKIPIF-- + +--INI-- +session.use_cookies=0 +session.cache_limiter= +session.save_handler=files +--FILE-- + +--EXPECT-- +done diff --git a/ext/standard/tests/serialize/gh21824.phpt b/ext/standard/tests/serialize/gh21824.phpt new file mode 100644 index 000000000000..593ef8571301 --- /dev/null +++ b/ext/standard/tests/serialize/gh21824.phpt @@ -0,0 +1,25 @@ +--TEST-- +GH-21824 (Crash in unserialize() when a destructor reentrantly reuses the context) +--FILE-- + +--EXPECT-- +bool(true) +int(1018) +done diff --git a/ext/standard/tests/strings/gh20477.phpt b/ext/standard/tests/strings/gh20477.phpt new file mode 100644 index 000000000000..3631b4bc16f9 --- /dev/null +++ b/ext/standard/tests/strings/gh20477.phpt @@ -0,0 +1,17 @@ +--TEST-- +GH-20477 (Use-after-free in concat when a destructor reads the concatenated value) +--FILE-- + new Foo]; +$a .= 'x'; +var_dump($a); +?> +--EXPECTF-- +Warning: Array to string conversion in %s on line %d +string(6) "Arrayx" diff --git a/ext/standard/var_unserializer.re b/ext/standard/var_unserializer.re index 4a9b278c116c..9e1f14847d67 100644 --- a/ext/standard/var_unserializer.re +++ b/ext/standard/var_unserializer.re @@ -81,6 +81,7 @@ PHPAPI php_unserialize_data_t php_var_unserialize_init(void) { PHPAPI void php_var_unserialize_destroy(php_unserialize_data_t d) { /* fprintf(stderr, "UNSERIALIZE_DESTROY == lock: %u, level: %u\n", BG(serialize_lock), BG(unserialize).level); */ + ZEND_DTOR_HAZARD_BEGIN(); if (BG(serialize_lock) || BG(unserialize).level == 1) { var_destroy(&d); efree(d); @@ -88,6 +89,7 @@ PHPAPI void php_var_unserialize_destroy(php_unserialize_data_t d) { if (!BG(serialize_lock) && !--BG(unserialize).level) { BG(unserialize).data = NULL; } + ZEND_DTOR_HAZARD_END(); } PHPAPI HashTable *php_var_unserialize_get_allowed_classes(php_unserialize_data_t d) {