Skip to content

Commit 624c061

Browse files
committed
Disallow indirect modification on readonly properties within __clone()
Indirect modification isn't allowed in __construct() because it allows references to leak, so it doesn't make much sense to allow it in __clone().
1 parent a59103f commit 624c061

File tree

5 files changed

+36
-56
lines changed

5 files changed

+36
-56
lines changed

Zend/tests/readonly_props/readonly_clone_error5.phpt

Lines changed: 3 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -57,20 +57,8 @@ try {
5757
}
5858

5959
?>
60-
--EXPECTF--
61-
object(TestSetOnce)#%d (1) {
62-
["prop"]=>
63-
array(1) {
64-
[0]=>
65-
int(1)
66-
}
67-
}
68-
object(TestSetOnce)#%d (1) {
69-
["prop"]=>
70-
array(1) {
71-
[0]=>
72-
int(1)
73-
}
74-
}
60+
--EXPECT--
61+
Cannot indirectly modify readonly property TestSetOnce::$prop
62+
Cannot indirectly modify readonly property TestSetOnce::$prop
7563
Cannot indirectly modify readonly property TestSetTwice::$prop
7664
Cannot indirectly modify readonly property TestSetTwice::$prop
Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,33 @@
1+
--TEST--
2+
__clone() cannot indirectly modify unlocked readonly properties
3+
--FILE--
4+
<?php
5+
6+
class Foo {
7+
public function __construct(
8+
public readonly array $bar
9+
) {}
10+
11+
public function __clone()
12+
{
13+
$this->bar['bar'] = 'bar';
14+
}
15+
}
16+
17+
$foo = new Foo([]);
18+
// First call fills the cache slot
19+
try {
20+
var_dump(clone $foo);
21+
} catch (Error $e) {
22+
echo $e->getMessage(), "\n";
23+
}
24+
try {
25+
var_dump(clone $foo);
26+
} catch (Error $e) {
27+
echo $e->getMessage(), "\n";
28+
}
29+
30+
?>
31+
--EXPECT--
32+
Cannot indirectly modify readonly property Foo::$bar
33+
Cannot indirectly modify readonly property Foo::$bar

Zend/tests/readonly_props/readonly_clone_success3.phpt

Lines changed: 0 additions & 37 deletions
This file was deleted.

Zend/zend_execute.c

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3364,8 +3364,6 @@ static zend_always_inline void zend_fetch_property_address(zval *result, zval *c
33643364
ZEND_ASSERT(type == BP_VAR_W || type == BP_VAR_RW || type == BP_VAR_UNSET);
33653365
if (Z_TYPE_P(ptr) == IS_OBJECT) {
33663366
ZVAL_COPY(result, ptr);
3367-
} else if (Z_PROP_FLAG_P(ptr) & IS_PROP_REINITABLE) {
3368-
Z_PROP_FLAG_P(ptr) &= ~IS_PROP_REINITABLE;
33693367
} else {
33703368
zend_readonly_property_indirect_modification_error(prop_info);
33713369
ZVAL_ERROR(result);

Zend/zend_object_handlers.c

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -694,8 +694,6 @@ ZEND_API zval *zend_std_read_property(zend_object *zobj, zend_string *name, int
694694
* to make sure no actual modification is possible. */
695695
ZVAL_COPY(rv, retval);
696696
retval = rv;
697-
} else if (Z_PROP_FLAG_P(retval) & IS_PROP_REINITABLE) {
698-
Z_PROP_FLAG_P(retval) &= ~IS_PROP_REINITABLE;
699697
} else {
700698
zend_readonly_property_indirect_modification_error(prop_info);
701699
retval = &EG(uninitialized_zval);

0 commit comments

Comments
 (0)