Skip to content

Commit

Permalink
Fix GH-13612: Corrupted memory in destructor with weak references
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
nielsdos committed Mar 8, 2024
1 parent e3f0d03 commit 39b8d5c
Show file tree
Hide file tree
Showing 3 changed files with 47 additions and 4 deletions.
4 changes: 4 additions & 0 deletions NEWS
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
39 changes: 39 additions & 0 deletions Zend/tests/weakrefs/gh13612.phpt
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
--TEST--
GH-13612 (Corrupted memory in destructor with weak references)
--FILE--
<?php

class WeakAnalysingMapRepro
{
public array $destroyed = [];
public array $ownerDestructorHandlers = [];

public function __construct()
{
$handler = new class($this) {
private \WeakReference $weakAnalysingMap;

public function __construct(WeakAnalysingMapRepro $analysingMap)
{
$this->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
8 changes: 4 additions & 4 deletions Zend/zend_objects.c
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down

0 comments on commit 39b8d5c

Please sign in to comment.