Skip to content

Commit 514a9e1

Browse files
committed
Fix phpGH-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).
1 parent f453d1a commit 514a9e1

File tree

2 files changed

+61
-2
lines changed

2 files changed

+61
-2
lines changed

ext/mbstring/mbstring.c

Lines changed: 17 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3289,7 +3289,7 @@ static int mb_recursive_convert_variable(mbfl_buffer_converter *convd, zval *var
32893289
if (ret != NULL) {
32903290
zval_ptr_dtor(orig_var);
32913291
// TODO: avoid reallocation ???
3292-
ZVAL_STRINGL(orig_var, (char *)ret->val, ret->len);
3292+
ZVAL_STRINGL(orig_var, (const char *) ret->val, ret->len);
32933293
efree(ret->val);
32943294
}
32953295
} 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
33053305

33063306
ht = HASH_OF(var);
33073307
if (ht != NULL) {
3308-
ZEND_HASH_FOREACH_VAL_IND(ht, entry) {
3308+
ZEND_HASH_FOREACH_VAL(ht, entry) {
3309+
/* Can be a typed property declaration, in which case we need to remove the reference from the source list.
3310+
* Just using ZEND_TRY_ASSIGN_STRINGL is not sufficient because that would not unwrap the reference
3311+
* and change values through references (see bug #26639). */
3312+
if (Z_TYPE_P(entry) == IS_INDIRECT) {
3313+
ZEND_ASSERT(Z_TYPE_P(var) == IS_OBJECT);
3314+
3315+
entry = Z_INDIRECT_P(entry);
3316+
if (Z_ISREF_P(entry) && Z_TYPE_P(Z_REFVAL_P(entry)) == IS_STRING) {
3317+
zend_property_info *info = zend_get_typed_property_info_for_slot(Z_OBJ_P(var), entry);
3318+
if (info) {
3319+
ZEND_REF_DEL_TYPE_SOURCE(Z_REF_P(entry), info);
3320+
}
3321+
}
3322+
}
3323+
33093324
if (mb_recursive_convert_variable(convd, entry)) {
33103325
if (Z_REFCOUNTED_P(var)) {
33113326
Z_UNPROTECT_RECURSION_P(var);

ext/mbstring/tests/gh16261.phpt

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,44 @@
1+
--TEST--
2+
GH-16261 (Reference invariant broken in mb_convert_variables())
3+
--EXTENSIONS--
4+
mbstring
5+
--FILE--
6+
<?php
7+
class Test {
8+
public string $x;
9+
public string $y;
10+
public array $z;
11+
}
12+
$test = new Test;
13+
$ref = "hello";
14+
$ref2 = "world";
15+
$ref3 = [&$ref2];
16+
$test->x =& $ref;
17+
$test->z =& $ref3;
18+
mb_convert_variables("EUC-JP", "Shift_JIS", $test);
19+
20+
class Test2 {
21+
public function __construct(public string $x) {}
22+
}
23+
$test2 = new Test2("foo");
24+
25+
mb_convert_variables("EUC-JP", "Shift_JIS", $test->x);
26+
27+
var_dump($test, $test2);
28+
?>
29+
--EXPECT--
30+
object(Test)#1 (2) {
31+
["x"]=>
32+
string(5) "hello"
33+
["y"]=>
34+
uninitialized(string)
35+
["z"]=>
36+
&array(1) {
37+
[0]=>
38+
string(5) "world"
39+
}
40+
}
41+
object(Test2)#2 (1) {
42+
["x"]=>
43+
string(3) "foo"
44+
}

0 commit comments

Comments
 (0)