From 39b8d5c871a400b4789951d7afe5aeccbe253666 Mon Sep 17 00:00:00 2001 From: Niels Dossche <7771979+nielsdos@users.noreply.github.com> Date: Wed, 6 Mar 2024 23:14:47 +0100 Subject: [PATCH 1/2] Fix GH-13612: Corrupted memory in destructor with weak references Inside `zend_object_std_dtor` the weakrefs are notified after the destruction of properties already took place. In this test case, the destructor of an anon class will be invoked due to the property destruction. That class has a weak reference to its parent. This means that the destructor can access parent properties that already have been destroyed, resulting in a UAF. Fix this by notifying the weakrefs at the start of the object's destruction. Closes GH-13613. --- NEWS | 4 ++++ Zend/tests/weakrefs/gh13612.phpt | 39 ++++++++++++++++++++++++++++++++ Zend/zend_objects.c | 8 +++---- 3 files changed, 47 insertions(+), 4 deletions(-) create mode 100644 Zend/tests/weakrefs/gh13612.phpt diff --git a/NEWS b/NEWS index 019f4a79a69..780e43cf6d6 100644 --- a/NEWS +++ b/NEWS @@ -2,6 +2,10 @@ PHP NEWS ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||| ?? ??? ????, PHP 8.2.18 +- Core: + . Fixed bug GH-13612 (Corrupted memory in destructor with weak references). + (nielsdos) + - Gettext: - Fixed sigabrt raised with dcgettext/dcngettext calls with gettext 0.22.5 with category set to LC_ALL. (David Carlier) diff --git a/Zend/tests/weakrefs/gh13612.phpt b/Zend/tests/weakrefs/gh13612.phpt new file mode 100644 index 00000000000..4ca4c925087 --- /dev/null +++ b/Zend/tests/weakrefs/gh13612.phpt @@ -0,0 +1,39 @@ +--TEST-- +GH-13612 (Corrupted memory in destructor with weak references) +--FILE-- +weakAnalysingMap = \WeakReference::create($analysingMap); + } + + public function __destruct() + { + var_dump($this->weakAnalysingMap->get()); + } + }; + + $this->destroyed[] = 1; + $this->ownerDestructorHandlers[] = $handler; + } +} + +new WeakAnalysingMapRepro(); + +echo "Done\n"; + +?> +--EXPECT-- +NULL +Done diff --git a/Zend/zend_objects.c b/Zend/zend_objects.c index b09ce3b990d..aeaecac539a 100644 --- a/Zend/zend_objects.c +++ b/Zend/zend_objects.c @@ -47,6 +47,10 @@ ZEND_API void zend_object_std_dtor(zend_object *object) { zval *p, *end; + if (UNEXPECTED(GC_FLAGS(object) & IS_OBJ_WEAKLY_REFERENCED)) { + zend_weakrefs_notify(object); + } + if (object->properties) { if (EXPECTED(!(GC_FLAGS(object->properties) & IS_ARRAY_IMMUTABLE))) { if (EXPECTED(GC_DELREF(object->properties) == 0) @@ -85,10 +89,6 @@ ZEND_API void zend_object_std_dtor(zend_object *object) FREE_HASHTABLE(guards); } } - - if (UNEXPECTED(GC_FLAGS(object) & IS_OBJ_WEAKLY_REFERENCED)) { - zend_weakrefs_notify(object); - } } ZEND_API void zend_objects_destroy_object(zend_object *object) From 608ef99a65aedf9991ad24471d732ab328ebbf9b Mon Sep 17 00:00:00 2001 From: Niels Dossche <7771979+nielsdos@users.noreply.github.com> Date: Fri, 8 Mar 2024 18:26:36 +0100 Subject: [PATCH 2/2] [ci skip] NEWS --- NEWS | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/NEWS b/NEWS index 780e43cf6d6..21c7d6165d5 100644 --- a/NEWS +++ b/NEWS @@ -7,7 +7,7 @@ PHP NEWS (nielsdos) - Gettext: - - Fixed sigabrt raised with dcgettext/dcngettext calls with gettext 0.22.5 + . Fixed sigabrt raised with dcgettext/dcngettext calls with gettext 0.22.5 with category set to LC_ALL. (David Carlier) - MySQLnd: