Fix GH-21687: array_walk creates references to readonly properties#21690
Fix GH-21687: array_walk creates references to readonly properties#21690iliaal wants to merge 1 commit intophp:masterfrom
Conversation
ndossche
left a comment
ndossche
left a comment
There was a problem hiding this comment.
No. You're missing the point that it corrupts the enum and shm.
array_walk() wrapped object properties in IS_REFERENCE for the callback, bypassing readonly enforcement. For enum singletons this permanently corrupted the shared object (and opcache SHM), crashing on any subsequent var_dump(). For regular readonly properties it allowed the callback to silently modify values that should be immutable. Check ZEND_ACC_READONLY before creating the reference, matching the check that foreach by-reference already performs. Throw the same "Cannot acquire reference to readonly property" error. Closes phpGH-21687
iliaal
force-pushed
the
fix/gh-21687-enum-array-walk-assertion
branch
from
56b2b9f to
1cbea32
Compare
iliaal
changed the title
|
Reworked. The fix is now in array_walk: check ZEND_ACC_READONLY before creating the reference, same check foreach already does. Enum properties are no longer corrupted, and regular readonly properties can no longer be silently modified through the callback. |
|
Just from looking at the code I think that you can violate the type of a typed property, e.g. put a string where an int is expected. |
Was a little to quick fixing the symptom as opposed to the cause 🤦 Here goes attempt #2 |
| ZVAL_NEW_REF(zv, zv); | ||
| ZEND_REF_ADD_TYPE_SOURCE(Z_REF_P(zv), prop_info); | ||
| if (UNEXPECTED(prop_info->flags & ZEND_ACC_READONLY)) { | ||
| zend_throw_error(NULL, |
There was a problem hiding this comment.
I think the canonical message is Cannot indirectly modify readonly property Foo::$prop
E.g. from:
<?php
class Foo {
public readonly int $prop;
public function __construct() {
$this->prop = 1;
}
}
$foo = new Foo;
$ref =& $foo->prop;There was a problem hiding this comment.
foreach by-ref uses "Cannot acquire reference to readonly property", direct $ref =& $foo->prop uses "Cannot indirectly modify readonly property."
class Foo {
public readonly int $prop;
public function __construct() { $this->prop = 1; }
}
$foo = new Foo;
foreach ($foo as &$v) {}
// Error: Cannot acquire reference to readonly property Foo::$propI matched the foreach wording since array_walk iterates properties and both are array related, but "Cannot indirectly modify" works too. @ndossche I can update the message, if you prefer, but there is a bit of inconsistency here
2 participants
Fixes #21687
`array_walk()` wrapped object properties in `IS_REFERENCE` for the callback, bypassing readonly enforcement. For enum singletons this permanently corrupted the shared object (and opcache SHM), crashing on any subsequent `var_dump()`. For regular readonly properties it allowed the callback to silently modify values that should be immutable.
Checks `ZEND_ACC_READONLY` before creating the reference, matching the check that `foreach` by-reference already performs. Throws the same "Cannot acquire reference to readonly property" error.