From 0026d8a7838914296c0d7e99ddc639c84e6ebb8a Mon Sep 17 00:00:00 2001
From: Nikita Popov <nikita.ppv@gmail.com>
Date: Thu, 27 Aug 2020 12:05:06 +0200
Subject: [PATCH] Fix use-after-free is WeakMap key and value are the same

Drop the object from the WeakMap as the last step, as this might
end up destroying the object.
---
 Zend/tests/weakrefs/weakmap_weakness.phpt | 21 +++++++++++++++++++++
 Zend/zend_weakrefs.c                      | 10 +++++++---
 2 files changed, 28 insertions(+), 3 deletions(-)

diff --git a/Zend/tests/weakrefs/weakmap_weakness.phpt b/Zend/tests/weakrefs/weakmap_weakness.phpt
index 5e57dead56..d740d2b219 100644
--- a/Zend/tests/weakrefs/weakmap_weakness.phpt
+++ b/Zend/tests/weakrefs/weakmap_weakness.phpt
@@ -45,6 +45,14 @@ var_dump($map);
 gc_collect_cycles();
 var_dump($map);
 
+echo "\nStoring object as own value:\n";
+$map = new WeakMap;
+$obj = new stdClass;
+$map[$obj] = $obj;
+unset($obj);
+var_dump($map);
+unset($map);
+
 echo "\nStoring map in itself:\n";
 $map = new WeakMap;
 $map[$map] = $map;
@@ -95,6 +103,19 @@ object(WeakMap)#1 (1) {
 object(WeakMap)#1 (0) {
 }
 
+Storing object as own value:
+object(WeakMap)#3 (1) {
+  [0]=>
+  array(2) {
+    ["key"]=>
+    object(stdClass)#1 (0) {
+    }
+    ["value"]=>
+    object(stdClass)#1 (0) {
+    }
+  }
+}
+
 Storing map in itself:
 object(WeakMap)#3 (1) {
   [0]=>
diff --git a/Zend/zend_weakrefs.c b/Zend/zend_weakrefs.c
index 8a444c214e..8c1719f4c9 100644
--- a/Zend/zend_weakrefs.c
+++ b/Zend/zend_weakrefs.c
@@ -119,9 +119,11 @@ static void zend_weakref_unregister(zend_object *object, void *payload) {
 	uintptr_t tag = ZEND_WEAKREF_GET_TAG(tagged_ptr);
 	if (tag != ZEND_WEAKREF_TAG_HT) {
 		ZEND_ASSERT(tagged_ptr == payload);
-		zend_weakref_unref_single(ptr, tag, obj_addr);
 		zend_hash_index_del(&EG(weakrefs), obj_addr);
 		GC_DEL_FLAGS(object, IS_OBJ_WEAKLY_REFERENCED);
+
+		/* Do this last, as it may destroy the object. */
+		zend_weakref_unref_single(ptr, tag, obj_addr);
 		return;
 	}
 
@@ -129,8 +131,6 @@ static void zend_weakref_unregister(zend_object *object, void *payload) {
 	tagged_ptr = zend_hash_index_find_ptr(ht, (zend_ulong) payload);
 	ZEND_ASSERT(tagged_ptr && "Weakref not registered?");
 	ZEND_ASSERT(tagged_ptr == payload);
-	zend_weakref_unref_single(
-		ZEND_WEAKREF_GET_PTR(payload), ZEND_WEAKREF_GET_TAG(payload), obj_addr);
 	zend_hash_index_del(ht, (zend_ulong) payload);
 	if (zend_hash_num_elements(ht) == 0) {
 		GC_DEL_FLAGS(object, IS_OBJ_WEAKLY_REFERENCED);
@@ -138,6 +138,10 @@ static void zend_weakref_unregister(zend_object *object, void *payload) {
 		FREE_HASHTABLE(ht);
 		zend_hash_index_del(&EG(weakrefs), obj_addr);
 	}
+
+	/* Do this last, as it may destroy the object. */
+	zend_weakref_unref_single(
+		ZEND_WEAKREF_GET_PTR(payload), ZEND_WEAKREF_GET_TAG(payload), obj_addr);
 }
 
 void zend_weakrefs_init(void) {
-- 
2.40.0