From 514a9e1ce5a2b63106d7b4576a3c8b125bc09b6c Mon Sep 17 00:00:00 2001
From: Niels Dossche <7771979+nielsdos@users.noreply.github.com>
Date: Sun, 6 Oct 2024 19:57:21 +0200
Subject: [PATCH] Fix GH-16261: Reference invariant broken in
 mb_convert_variables()

The behaviour is weird in the sense that the reference must get
unwrapped. What ended up happening is that when destroying the old
reference the sources list was not cleaned properly. We add handling for
that. Normally we would use use ZEND_TRY_ASSIGN_STRINGL but that doesn't
work here as it would keep the reference and change values through
references (see bug #26639).
---
 ext/mbstring/mbstring.c         | 19 ++++++++++++--
 ext/mbstring/tests/gh16261.phpt | 44 +++++++++++++++++++++++++++++++++
 2 files changed, 61 insertions(+), 2 deletions(-)
 create mode 100644 ext/mbstring/tests/gh16261.phpt

diff --git a/ext/mbstring/mbstring.c b/ext/mbstring/mbstring.c
index 5aa25b57f01a2..5c9d1bea39fa6 100644
--- a/ext/mbstring/mbstring.c
+++ b/ext/mbstring/mbstring.c
@@ -3289,7 +3289,7 @@ static int mb_recursive_convert_variable(mbfl_buffer_converter *convd, zval *var
 		if (ret != NULL) {
 			zval_ptr_dtor(orig_var);
 			// TODO: avoid reallocation ???
-			ZVAL_STRINGL(orig_var, (char *)ret->val, ret->len);
+			ZVAL_STRINGL(orig_var, (const char *) ret->val, ret->len);
 			efree(ret->val);
 		}
 	} else if (Z_TYPE_P(var) == IS_ARRAY || Z_TYPE_P(var) == IS_OBJECT) {
@@ -3305,7 +3305,22 @@ static int mb_recursive_convert_variable(mbfl_buffer_converter *convd, zval *var
 
 		ht = HASH_OF(var);
 		if (ht != NULL) {
-			ZEND_HASH_FOREACH_VAL_IND(ht, entry) {
+			ZEND_HASH_FOREACH_VAL(ht, entry) {
+				/* Can be a typed property declaration, in which case we need to remove the reference from the source list.
+				 * Just using ZEND_TRY_ASSIGN_STRINGL is not sufficient because that would not unwrap the reference
+				 * and change values through references (see bug #26639). */
+				if (Z_TYPE_P(entry) == IS_INDIRECT) {
+					ZEND_ASSERT(Z_TYPE_P(var) == IS_OBJECT);
+
+					entry = Z_INDIRECT_P(entry);
+					if (Z_ISREF_P(entry) && Z_TYPE_P(Z_REFVAL_P(entry)) == IS_STRING) {
+						zend_property_info *info = zend_get_typed_property_info_for_slot(Z_OBJ_P(var), entry);
+						if (info) {
+							ZEND_REF_DEL_TYPE_SOURCE(Z_REF_P(entry), info);
+						}
+					}
+				}
+
 				if (mb_recursive_convert_variable(convd, entry)) {
 					if (Z_REFCOUNTED_P(var)) {
 						Z_UNPROTECT_RECURSION_P(var);
diff --git a/ext/mbstring/tests/gh16261.phpt b/ext/mbstring/tests/gh16261.phpt
new file mode 100644
index 0000000000000..3573bd191c63d
--- /dev/null
+++ b/ext/mbstring/tests/gh16261.phpt
@@ -0,0 +1,44 @@
+--TEST--
+GH-16261 (Reference invariant broken in mb_convert_variables())
+--EXTENSIONS--
+mbstring
+--FILE--
+<?php
+class Test {
+    public string $x;
+    public string $y;
+    public array $z;
+}
+$test = new Test;
+$ref = "hello";
+$ref2 = "world";
+$ref3 = [&$ref2];
+$test->x =& $ref;
+$test->z =& $ref3;
+mb_convert_variables("EUC-JP", "Shift_JIS", $test);
+
+class Test2 {
+    public function __construct(public string $x) {}
+}
+$test2 = new Test2("foo");
+
+mb_convert_variables("EUC-JP", "Shift_JIS", $test->x);
+
+var_dump($test, $test2);
+?>
+--EXPECT--
+object(Test)#1 (2) {
+  ["x"]=>
+  string(5) "hello"
+  ["y"]=>
+  uninitialized(string)
+  ["z"]=>
+  &array(1) {
+    [0]=>
+    string(5) "world"
+  }
+}
+object(Test2)#2 (1) {
+  ["x"]=>
+  string(3) "foo"
+}