diff --git a/NEWS b/NEWS index ce986ea27f92..1482218dd009 100644 --- a/NEWS +++ b/NEWS @@ -2,6 +2,10 @@ PHP NEWS ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||| ?? ??? ????, PHP 8.6.0RC5 +- Core: + . Fixed set visibility bypass through a primed ZEND_ASSIGN_OBJ cache slot. + (Matthias Dötsch) + - Opcache: . Fixed tracing JIT returning a wrapped integer instead of a float on integer overflow after an integer was compared with a float or diff --git a/Zend/tests/asymmetric_visibility/cache_slot_magic_set.phpt b/Zend/tests/asymmetric_visibility/cache_slot_magic_set.phpt new file mode 100644 index 000000000000..df5b3fc2ec88 --- /dev/null +++ b/Zend/tests/asymmetric_visibility/cache_slot_magic_set.phpt @@ -0,0 +1,28 @@ +--TEST-- +Asymmetric visibility: cache slot primed by a __set() fallback must not skip the set visibility check +--FILE-- +x); } +} +function w(C $c, int $v) { + try { + $c->x = $v; // same opline every call + } catch (Error $e) { + echo $e->getMessage(), "\n"; + } +} +$a = new C(); +$a->unsetX(); +w($a, 1); // unset property: goes to __set(), cache slot stays primed +$b = new C(); +w($b, 2); // initialized property, must fail: private(set) from global scope +var_dump($b->x); + +?> +--EXPECT-- +__set(x) +Cannot modify private(set) property C::$x from global scope +int(0) diff --git a/Zend/tests/readonly_props/cache_slot_clone_reinit.phpt b/Zend/tests/readonly_props/cache_slot_clone_reinit.phpt new file mode 100644 index 000000000000..70a3bc77e0e5 --- /dev/null +++ b/Zend/tests/readonly_props/cache_slot_clone_reinit.phpt @@ -0,0 +1,26 @@ +--TEST-- +Readonly: cache slot primed by a failed modification must not skip the set visibility check on reinitialization +--FILE-- +x = 1; } + public function __clone() { w($this, 3); } +} +function w(P $p, int $v) { + try { + $p->x = $v; // same opline every call + } catch (Error $e) { + echo $e->getMessage(), "\n"; + } +} +$p = new P(); +w($p, 2); // readonly modification error, cache slot stays primed +$q = clone $p; // __clone -> w(): global scope must not be able to reinitialize +var_dump($q->x); + +?> +--EXPECT-- +Cannot modify readonly property P::$x +Cannot modify protected(set) readonly property P::$x from global scope +int(1) diff --git a/Zend/zend_object_handlers.c b/Zend/zend_object_handlers.c index 800f9111c3de..20d8cff84b3f 100644 --- a/Zend/zend_object_handlers.c +++ b/Zend/zend_object_handlers.c @@ -1059,6 +1059,16 @@ ZEND_API zval *zend_std_read_property(zend_object *zobj, zend_string *name, int } /* }}} */ +/* The ZEND_ASSIGN_OBJ fast path skips the set visibility check when the cache slot is + * primed, assuming write_property() only primes it after that check passed. Reset the + * slot on the paths that leave it primed without having checked. */ +static zend_always_inline void zend_reset_property_cache_slot(void **cache_slot) { + if (cache_slot) { + CACHE_POLYMORPHIC_PTR_EX(cache_slot, NULL, NULL); + CACHE_PTR_EX(cache_slot + 2, NULL); + } +} + static zend_always_inline bool property_uses_strict_types(void) { const zend_execute_data *execute_data = EG(current_execute_data); return execute_data @@ -1131,18 +1141,19 @@ ZEND_API zval *zend_std_write_property(zend_object *zobj, zend_string *name, zva && !(Z_PROP_FLAG_P(variable_ptr) & IS_PROP_REINITABLE)) { zend_readonly_property_modification_error(prop_info); variable_ptr = &EG(error_zval); + /* The set visibility was not checked. */ + zend_reset_property_cache_slot(cache_slot); goto exit; } if ((prop_info->flags & ZEND_ACC_PPP_SET_MASK) && !zend_asymmetric_property_has_set_access(prop_info)) { zend_asymmetric_visibility_property_modification_error(prop_info, "modify"); variable_ptr = &EG(error_zval); - if (cache_slot) { - /* Reset cache slot to dodge fast path in next execution. */ - CACHE_POLYMORPHIC_PTR_EX(cache_slot, NULL, NULL); - CACHE_PTR_EX(cache_slot + 2, NULL); - } + zend_reset_property_cache_slot(cache_slot); goto exit; } + } else { + /* __set() will be called without checking the set visibility. */ + zend_reset_property_cache_slot(cache_slot); } }