Staging: destructor-hazard-section deferral mechanism

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

Zend/tests/gh19999.phpt

@@ -0,0 +1,16 @@
+--TEST--
+GH-19999 (GC refcount assertion via object destruction during concat assignment)
+--FILE--
+<?php
+class Test {
+    public function __destruct() {
+        $GLOBALS['a'] = null;
+    }
+}
+$a = [new Test];
+$a .= $a;
+var_dump($a);
+?>
+--EXPECTF--
+Warning: Array to string conversion in %s on line %d
+NULL

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();

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)) {

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];

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)

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);

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)
 {

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);

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--
+<?php include('skipif.inc'); ?>
+--INI--
+session.use_cookies=0
+session.cache_limiter=
+session.save_handler=files
+--FILE--
+<?php
+session_start();
+
+class SelfRemover {
+    public function __destruct() {
+        unset($_SESSION['x']);
+    }
+}
+
+$_SESSION['x'] = new SelfRemover();
+session_unset();
+
+echo "done\n";
+?>
+--EXPECT--
+done

ext/standard/tests/serialize/gh21824.phpt

@@ -0,0 +1,25 @@
+--TEST--
+GH-21824 (Crash in unserialize() when a destructor reentrantly reuses the context)
+--FILE--
+<?php
+class Trigger {
+    public function __destruct() {
+        @unserialize('i:1;');
+    }
+}
+$payload = 'a:1019:{';
+for ($i = 0; $i < 1017; $i++) {
+    $payload .= "i:$i;s:1:\"A\";";
+}
+// Overflow into a second var_entries block, then a duplicate key pushes Trigger
+// onto the delayed-destructor list.
+$payload .= 'i:1017;O:7:"Trigger":0:{}';
+$payload .= 'i:1017;i:0;}';
+$result = @unserialize($payload);
+var_dump(is_array($result), count($result));
+echo "done\n";
+?>
+--EXPECT--
+bool(true)
+int(1018)
+done

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--
+<?php
+class Foo {
+    public function __destruct() {
+        global $a;
+        serialize($a);
+    }
+}
+$a = ['index.php', 'c' => new Foo];
+$a .= 'x';
+var_dump($a);
+?>
+--EXPECTF--
+Warning: Array to string conversion in %s on line %d
+string(6) "Arrayx"

ext/standard/var_unserializer.re

@@ -81,13 +81,15 @@ 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);
 	}
 	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) {