From af2110e664635329cbd7038727d4c7f222bb8a0e Mon Sep 17 00:00:00 2001
From: Ilija Tovilo <ilija.tovilo@me.com>
Date: Tue, 29 Aug 2023 16:53:38 +0200
Subject: [PATCH] Fix freeing of incompletely initialized closures

Addref to relevant fields before allocating any memory. Also only set/remove the
ZEND_ACC_HEAP_RT_CACHE flag after allocating memory.

Fixes GH-12073
Closes GH-12074
---
 NEWS                    |  2 ++
 Zend/tests/gh12073.phpt | 28 ++++++++++++++++++++++++++++
 Zend/zend_closures.c    | 13 +++++++------
 3 files changed, 37 insertions(+), 6 deletions(-)
 create mode 100644 Zend/tests/gh12073.phpt

diff --git a/NEWS b/NEWS
index e001635a235b6..c85c5c0da553b 100644
--- a/NEWS
+++ b/NEWS
@@ -6,6 +6,8 @@ PHP                                                                        NEWS
   . Fixed bug GH-11937 (Constant ASTs containing objects). (ilutov)
   . Fixed bug GH-11790 (On riscv64 require libatomic if actually needed).
     (Jeremie Courreges-Anglas)
+  . Fixed bug GH-12073 (Segfault when freeing incompletely initialized
+    closures). (ilutov)
 
 - DOM:
   . Fix memory leak when setting an invalid DOMDocument encoding. (nielsdos)
diff --git a/Zend/tests/gh12073.phpt b/Zend/tests/gh12073.phpt
new file mode 100644
index 0000000000000..ef115685ce7d4
--- /dev/null
+++ b/Zend/tests/gh12073.phpt
@@ -0,0 +1,28 @@
+--TEST--
+GH-12073: Freeing of non-ZMM pointer of incompletely allocated closure
+--SKIPIF--
+<?php
+if (getenv("USE_ZEND_ALLOC") === "0" && getenv("USE_TRACKED_ALLOC") !== "1") {
+    die("skip Zend MM disabled");
+}
+?>
+--FILE--
+<?php
+
+function test() {
+	$k = 1;
+	return function () use ($k) {
+		foo();
+	};
+}
+
+ini_set('memory_limit', '2M');
+
+$array = [];
+for ($i = 0; $i < 10_000; $i++) {
+	$array[] = test();
+}
+
+?>
+--EXPECTF--
+Fatal error: Allowed memory size of %d bytes exhausted%s(tried to allocate %d bytes) in %s on line %d
diff --git a/Zend/zend_closures.c b/Zend/zend_closures.c
index 0dab5537a3752..dd5ae16c9f13a 100644
--- a/Zend/zend_closures.c
+++ b/Zend/zend_closures.c
@@ -689,6 +689,11 @@ static void zend_create_closure_ex(zval *res, zend_function *func, zend_class_en
 		closure->func.common.fn_flags |= ZEND_ACC_CLOSURE;
 		closure->func.common.fn_flags &= ~ZEND_ACC_IMMUTABLE;
 
+		zend_string_addref(closure->func.op_array.function_name);
+		if (closure->func.op_array.refcount) {
+			(*closure->func.op_array.refcount)++;
+		}
+
 		/* For fake closures, we want to reuse the static variables of the original function. */
 		if (!is_fake) {
 			if (closure->func.op_array.static_variables) {
@@ -722,24 +727,20 @@ static void zend_create_closure_ex(zval *res, zend_function *func, zend_class_en
 				if (func->common.scope != scope) {
 					func->common.scope = scope;
 				}
-				closure->func.op_array.fn_flags &= ~ZEND_ACC_HEAP_RT_CACHE;
 				ptr = zend_arena_alloc(&CG(arena), func->op_array.cache_size);
 				ZEND_MAP_PTR_SET(func->op_array.run_time_cache, ptr);
 				ZEND_MAP_PTR_SET(closure->func.op_array.run_time_cache, ptr);
+				closure->func.op_array.fn_flags &= ~ZEND_ACC_HEAP_RT_CACHE;
 			} else {
 				/* Otherwise, we use a non-shared runtime cache */
-				closure->func.op_array.fn_flags |= ZEND_ACC_HEAP_RT_CACHE;
 				ptr = emalloc(sizeof(void*) + func->op_array.cache_size);
 				ZEND_MAP_PTR_INIT(closure->func.op_array.run_time_cache, ptr);
 				ptr = (char*)ptr + sizeof(void*);
 				ZEND_MAP_PTR_SET(closure->func.op_array.run_time_cache, ptr);
+				closure->func.op_array.fn_flags |= ZEND_ACC_HEAP_RT_CACHE;
 			}
 			memset(ptr, 0, func->op_array.cache_size);
 		}
-		zend_string_addref(closure->func.op_array.function_name);
-		if (closure->func.op_array.refcount) {
-			(*closure->func.op_array.refcount)++;
-		}
 	} else {
 		memcpy(&closure->func, func, sizeof(zend_internal_function));
 		closure->func.common.fn_flags |= ZEND_ACC_CLOSURE;