From 23ce8a366d0fb25ade97dac66a6bcc962b8d71a4 Mon Sep 17 00:00:00 2001 From: Ilija Tovilo Date: Thu, 10 Mar 2022 18:06:03 +0100 Subject: [PATCH] Disallow assigning reference to unset readonly property Closes GH-7942 --- .../array_append_initialization.phpt | 13 +- Zend/tests/readonly_props/gh7942.phpt | 22 ++++ Zend/tests/readonly_props/variation.phpt | 118 ++++++++++++++++++ .../readonly_props/variation_nested.phpt | 77 ++++++++++++ Zend/zend_execute.c | 6 + Zend/zend_execute.h | 1 + Zend/zend_object_handlers.c | 17 ++- ext/opcache/tests/jit/fetch_obj_006.phpt | 7 +- 8 files changed, 248 insertions(+), 13 deletions(-) create mode 100644 Zend/tests/readonly_props/gh7942.phpt create mode 100644 Zend/tests/readonly_props/variation.phpt create mode 100644 Zend/tests/readonly_props/variation_nested.phpt diff --git a/Zend/tests/readonly_props/array_append_initialization.phpt b/Zend/tests/readonly_props/array_append_initialization.phpt index 079fecb8b3373..d9b3e8c074438 100644 --- a/Zend/tests/readonly_props/array_append_initialization.phpt +++ b/Zend/tests/readonly_props/array_append_initialization.phpt @@ -19,7 +19,11 @@ function init() { var_dump($c->a); } -(new C)->init(); +try { + (new C)->init(); +} catch (Error $e) { + echo $e->getMessage(), "\n"; +} try { init(); @@ -29,8 +33,5 @@ try { ?> --EXPECT-- -array(1) { - [0]=> - int(1) -} -Cannot initialize readonly property C::$a from global scope +Cannot indirectly modify readonly property C::$a +Cannot indirectly modify readonly property C::$a diff --git a/Zend/tests/readonly_props/gh7942.phpt b/Zend/tests/readonly_props/gh7942.phpt new file mode 100644 index 0000000000000..89db53439a2f2 --- /dev/null +++ b/Zend/tests/readonly_props/gh7942.phpt @@ -0,0 +1,22 @@ +--TEST-- +GH-7942: Disallow assigning reference to unset readonly property +--FILE-- +bar = &$bar; + } +} + +try { + $i = 42; + new Foo($i); +} catch (Error $e) { + echo $e->getMessage(), PHP_EOL; +} + +?> +--EXPECT-- +Cannot indirectly modify readonly property Foo::$bar diff --git a/Zend/tests/readonly_props/variation.phpt b/Zend/tests/readonly_props/variation.phpt new file mode 100644 index 0000000000000..a8ea4be60887e --- /dev/null +++ b/Zend/tests/readonly_props/variation.phpt @@ -0,0 +1,118 @@ +--TEST-- +Readonly variations +--FILE-- +prop = 1; + } + + public function r() { + echo $this->prop; + } + + public function w() { + $this->prop = 1; + echo 'done'; + } + + public function rw() { + $this->prop += 1; + echo 'done'; + } + + public function im() { + $this->prop[] = 1; + echo 'done'; + } + + public function is() { + echo (int) isset($this->prop); + } + + public function us() { + unset($this->prop); + echo 'done'; + } +} + +function r($test) { + echo $test->prop; +} + +function w($test) { + $test->prop = 0; + echo 'done'; +} + +function rw($test) { + $test->prop += 1; + echo 'done'; +} + +function im($test) { + $test->prop[] = 1; + echo 'done'; +} + +function is($test) { + echo (int) isset($test->prop); +} + +function us($test) { + unset($test->prop); + echo 'done'; +} + +foreach ([true, false] as $init) { + foreach ([true, false] as $scope) { + foreach (['r', 'w', 'rw', 'im', 'is', 'us'] as $op) { + $test = new Test(); + if ($init) { + $test->init(); + } + + echo 'Init: ' . ((int) $init) . ', scope: ' . ((int) $scope) . ', op: ' . $op . ": "; + try { + if ($scope) { + $test->{$op}(); + } else { + $op($test); + } + } catch (Error $e) { + echo $e->getMessage(); + } + echo "\n"; + } + } +} + +?> +--EXPECT-- +Init: 1, scope: 1, op: r: 1 +Init: 1, scope: 1, op: w: Cannot modify readonly property Test::$prop +Init: 1, scope: 1, op: rw: Cannot modify readonly property Test::$prop +Init: 1, scope: 1, op: im: Cannot modify readonly property Test::$prop +Init: 1, scope: 1, op: is: 1 +Init: 1, scope: 1, op: us: Cannot unset readonly property Test::$prop +Init: 1, scope: 0, op: r: 1 +Init: 1, scope: 0, op: w: Cannot modify readonly property Test::$prop +Init: 1, scope: 0, op: rw: Cannot modify readonly property Test::$prop +Init: 1, scope: 0, op: im: Cannot modify readonly property Test::$prop +Init: 1, scope: 0, op: is: 1 +Init: 1, scope: 0, op: us: Cannot unset readonly property Test::$prop +Init: 0, scope: 1, op: r: Typed property Test::$prop must not be accessed before initialization +Init: 0, scope: 1, op: w: done +Init: 0, scope: 1, op: rw: Typed property Test::$prop must not be accessed before initialization +Init: 0, scope: 1, op: im: Cannot indirectly modify readonly property Test::$prop +Init: 0, scope: 1, op: is: 0 +Init: 0, scope: 1, op: us: done +Init: 0, scope: 0, op: r: Typed property Test::$prop must not be accessed before initialization +Init: 0, scope: 0, op: w: Cannot initialize readonly property Test::$prop from global scope +Init: 0, scope: 0, op: rw: Typed property Test::$prop must not be accessed before initialization +Init: 0, scope: 0, op: im: Cannot indirectly modify readonly property Test::$prop +Init: 0, scope: 0, op: is: 0 +Init: 0, scope: 0, op: us: Cannot unset readonly property Test::$prop from global scope diff --git a/Zend/tests/readonly_props/variation_nested.phpt b/Zend/tests/readonly_props/variation_nested.phpt new file mode 100644 index 0000000000000..72a7925da9905 --- /dev/null +++ b/Zend/tests/readonly_props/variation_nested.phpt @@ -0,0 +1,77 @@ +--TEST-- +Readonly nested variations +--FILE-- +prop = new Inner(); + } +} + +function r($test) { + echo $test->prop->prop; +} + +function w($test) { + $test->prop->prop = 0; + echo 'done'; +} + +function rw($test) { + $test->prop->prop += 1; + echo 'done'; +} + +function im($test) { + $test->prop->array[] = 1; + echo 'done'; +} + +function is($test) { + echo (int) isset($test->prop->prop); +} + +function us($test) { + unset($test->prop->prop); + echo 'done'; +} + +foreach ([true, false] as $init) { + foreach (['r', 'w', 'rw', 'im', 'is', 'us'] as $op) { + $test = new Test(); + if ($init) { + $test->init(); + } + + echo 'Init: ' . ((int) $init) . ', op: ' . $op . ": "; + try { + $op($test); + } catch (Error $e) { + echo $e->getMessage(); + } + echo "\n"; + } +} + +?> +--EXPECT-- +Init: 1, op: r: 1 +Init: 1, op: w: done +Init: 1, op: rw: done +Init: 1, op: im: done +Init: 1, op: is: 1 +Init: 1, op: us: done +Init: 0, op: r: Typed property Test::$prop must not be accessed before initialization +Init: 0, op: w: Cannot indirectly modify readonly property Test::$prop +Init: 0, op: rw: Typed property Test::$prop must not be accessed before initialization +Init: 0, op: im: Cannot indirectly modify readonly property Test::$prop +Init: 0, op: is: 0 +Init: 0, op: us: done diff --git a/Zend/zend_execute.c b/Zend/zend_execute.c index ccd10d85c4f95..05bd59e5169db 100644 --- a/Zend/zend_execute.c +++ b/Zend/zend_execute.c @@ -849,6 +849,12 @@ ZEND_API ZEND_COLD void ZEND_FASTCALL zend_readonly_property_modification_error( ZSTR_VAL(info->ce->name), zend_get_unmangled_property_name(info->name)); } +ZEND_API ZEND_COLD void ZEND_FASTCALL zend_readonly_property_indirect_modification_error(zend_property_info *info) +{ + zend_throw_error(NULL, "Cannot indirectly modify readonly property %s::$%s", + ZSTR_VAL(info->ce->name), zend_get_unmangled_property_name(info->name)); +} + static zend_class_entry *resolve_single_class_type(zend_string *name, zend_class_entry *self_ce) { if (zend_string_equals_literal_ci(name, "self")) { return self_ce; diff --git a/Zend/zend_execute.h b/Zend/zend_execute.h index d13087a5b0264..0a80daf25092e 100644 --- a/Zend/zend_execute.h +++ b/Zend/zend_execute.h @@ -74,6 +74,7 @@ ZEND_API ZEND_COLD zval* ZEND_FASTCALL zend_undefined_index_write(HashTable *ht, ZEND_API ZEND_COLD void zend_wrong_string_offset_error(void); ZEND_API ZEND_COLD void ZEND_FASTCALL zend_readonly_property_modification_error(zend_property_info *info); +ZEND_API ZEND_COLD void ZEND_FASTCALL zend_readonly_property_indirect_modification_error(zend_property_info *info); ZEND_API bool zend_verify_scalar_type_hint(uint32_t type_mask, zval *arg, bool strict, bool is_internal_arg); ZEND_API ZEND_COLD void zend_verify_arg_error( diff --git a/Zend/zend_object_handlers.c b/Zend/zend_object_handlers.c index 966ba0dd6ed72..5fa80c1adc5e5 100644 --- a/Zend/zend_object_handlers.c +++ b/Zend/zend_object_handlers.c @@ -604,6 +604,17 @@ ZEND_API zval *zend_std_read_property(zend_object *zobj, zend_string *name, int } } goto exit; + } else { + if (prop_info && UNEXPECTED(prop_info->flags & ZEND_ACC_READONLY)) { + if (type == BP_VAR_W || type == BP_VAR_RW) { + zend_readonly_property_indirect_modification_error(prop_info); + retval = &EG(uninitialized_zval); + goto exit; + } else if (type == BP_VAR_UNSET) { + retval = &EG(uninitialized_zval); + goto exit; + } + } } if (UNEXPECTED(Z_PROP_FLAG_P(retval) == IS_PROP_UNINIT)) { /* Skip __get() for uninitialized typed properties */ @@ -1024,9 +1035,9 @@ ZEND_API zval *zend_std_get_property_ptr_ptr(zend_object *zobj, zend_string *nam ZVAL_NULL(retval); zend_error(E_WARNING, "Undefined property: %s::$%s", ZSTR_VAL(zobj->ce->name), ZSTR_VAL(name)); } - } else if (prop_info && UNEXPECTED(prop_info->flags & ZEND_ACC_READONLY) - && !verify_readonly_initialization_access(prop_info, zobj->ce, name, "initialize")) { - retval = &EG(error_zval); + } else if (prop_info && UNEXPECTED(prop_info->flags & ZEND_ACC_READONLY)) { + /* Readonly property, delegate to read_property + write_property. */ + retval = NULL; } } else { /* we do have getter - fail and let it try again with usual get/set */ diff --git a/ext/opcache/tests/jit/fetch_obj_006.phpt b/ext/opcache/tests/jit/fetch_obj_006.phpt index 120c2f4dbeddd..afb0668cd2507 100644 --- a/ext/opcache/tests/jit/fetch_obj_006.phpt +++ b/ext/opcache/tests/jit/fetch_obj_006.phpt @@ -21,11 +21,10 @@ $appendProp2 = (function() { $this->prop[] = 1; })->bindTo($test, Test::class); $appendProp2(); -$appendProp2(); ?> --EXPECTF-- -Fatal error: Uncaught Error: Cannot modify readonly property Test::$prop in %sfetch_obj_006.php:13 +Fatal error: Uncaught Error: Cannot indirectly modify readonly property Test::$prop in %s:%d Stack trace: -#0 %sfetch_obj_006.php(16): Test->{closure}() +#0 %sfetch_obj_006.php(15): Test->{closure}() #1 {main} - thrown in %sfetch_obj_006.php on line 13 \ No newline at end of file + thrown in %sfetch_obj_006.php on line 13