Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

PHP 8.2 readonly classes can have dynamic properties created by unserialize() #9325

Closed
TysonAndre opened this issue Aug 13, 2022 · 6 comments

Comments

@TysonAndre
Copy link
Contributor

Description

The following code:

<?php
readonly class C{}
var_export(unserialize('O:1:"C":1:{s:1:"x";b:1;}'));

Resulted in this output:

\C::__set_state(array(
   'x' => true,
))

But I expected this output instead:

Uncaught Error: Cannot create dynamic property C::$x

PHP Version

PHP 8.2.0beta2

Operating System

No response

@iluuu1994
Copy link
Member

iluuu1994 commented Aug 13, 2022

Related to #9186.

@TysonAndre
Copy link
Contributor Author

TysonAndre commented Aug 13, 2022

@nikic @kocsismate fyi

Checks would probably be similar to those found in zend_object_handlers

We may want a different error message for private/protected properties with embedded null bytes (if they were declared when serializing but not declared when unserializing), e.g. "\0*\0protectedPropName", "\0DeclaringClass\0privatePropName" - I don't remember a helper method to reuse to do that, but skipping past the second(last) null byte would work

static zend_always_inline const char *zend_get_unmangled_property_name(const zend_string *mangled_prop) {
			if (UNEXPECTED(zobj->ce->ce_flags & ZEND_ACC_NO_DYNAMIC_PROPERTIES)) {
				zend_forbidden_dynamic_property(zobj->ce, name);
				variable_ptr = &EG(error_zval);
				goto exit;
			}
			if (UNEXPECTED(!(zobj->ce->ce_flags & ZEND_ACC_ALLOW_DYNAMIC_PROPERTIES))) {
				if (UNEXPECTED(!zend_deprecated_dynamic_property(zobj, name))) {
					variable_ptr = &EG(error_zval);
					goto exit;
				}
			}

@TysonAndre
Copy link
Contributor Author

To avoid Creation of dynamic property __PHP_Incomplete_Class::$dynamicProp is deprecated:

Looking at ext/standard/basic_functions.stub.php - __PHP_Incomplete_Class should also have #[AllowDynamicProperties], because unserialize() will create instances of that class and add dynamic properties to it when unserializing classes where definitions couldn't be found

@ghost
Copy link

ghost commented Aug 15, 2022

Why are you pinging nikic in your PRs? Didn't he switch focus from PHP to LLMV?

@kocsismate
Copy link
Member

I created #9354 for fixing the original, more general bug (#9186) as well as the dynamic property deprecation @TysonAndre mentioned here.

@kocsismate
Copy link
Member

Fixed via adb45a6

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants