diff --git a/ext/session/session.c b/ext/session/session.c index 83e5ff4105ef8..eb2ffedfa2478 100644 --- a/ext/session/session.c +++ b/ext/session/session.c @@ -243,11 +243,21 @@ static zend_string *php_session_encode(void) /* {{{ */ static int php_session_decode(zend_string *data) /* {{{ */ { + int res; + struct php_unserialize_data *prev_data; + unsigned prev_lock_level; + if (!PS(serializer)) { php_error_docref(NULL, E_WARNING, "Unknown session.serialize_handler. Failed to decode session object"); return FAILURE; } - if (PS(serializer)->decode(ZSTR_VAL(data), ZSTR_LEN(data)) == FAILURE) { + /* Make sure that any uses of unserialize() during session decoding do not share + * state with any unserialize() that is already in progress (e.g. because we are + * currently inside Serializable::unserialize(). */ + PHP_VAR_UNSERIALIZE_LOCK(prev_data, prev_lock_level); + res = PS(serializer)->decode(ZSTR_VAL(data), ZSTR_LEN(data)); + PHP_VAR_UNSERIALIZE_UNLOCK(prev_data, prev_lock_level); + if (res == FAILURE) { php_session_destroy(); php_session_track_init(); php_error_docref(NULL, E_WARNING, "Failed to decode session object. Session has been destroyed"); diff --git a/ext/standard/basic_functions.c b/ext/standard/basic_functions.c index 7971cce82ed68..3de3e3b04f964 100755 --- a/ext/standard/basic_functions.c +++ b/ext/standard/basic_functions.c @@ -909,7 +909,6 @@ static void basic_globals_ctor(php_basic_globals *basic_globals_p) /* {{{ */ BG(left) = -1; BG(user_tick_functions) = NULL; BG(user_filter_map) = NULL; - BG(serialize_lock) = 0; memset(&BG(serialize), 0, sizeof(BG(serialize))); memset(&BG(unserialize), 0, sizeof(BG(unserialize))); @@ -1160,7 +1159,6 @@ PHP_RINIT_FUNCTION(basic) /* {{{ */ { memset(BG(strtok_table), 0, 256); - BG(serialize_lock) = 0; memset(&BG(serialize), 0, sizeof(BG(serialize))); memset(&BG(unserialize), 0, sizeof(BG(unserialize))); diff --git a/ext/standard/basic_functions.h b/ext/standard/basic_functions.h index a010db88f680b..358edac589178 100644 --- a/ext/standard/basic_functions.h +++ b/ext/standard/basic_functions.h @@ -202,14 +202,15 @@ typedef struct _php_basic_globals { /* var.c */ zend_class_entry *incomplete_class; - unsigned serialize_lock; /* whether to use the locally supplied var_hash instead (__sleep/__wakeup) */ struct { struct php_serialize_data *data; unsigned level; + unsigned lock_level; } serialize; struct { struct php_unserialize_data *data; unsigned level; + unsigned lock_level; } unserialize; /* url_scanner_ex.re */ diff --git a/ext/standard/php_var.h b/ext/standard/php_var.h index d9abae86d2518..980147a3b2a09 100644 --- a/ext/standard/php_var.h +++ b/ext/standard/php_var.h @@ -66,6 +66,26 @@ PHPAPI zend_long php_var_unserialize_get_cur_depth(php_unserialize_data_t d); #define PHP_VAR_UNSERIALIZE_DESTROY(d) \ php_var_unserialize_destroy(d) +#define PHP_VAR_SERIALIZE_LOCK(prev_data, prev_lock_level) \ + prev_data = BG(serialize).data; \ + prev_lock_level = BG(serialize).lock_level; \ + BG(serialize).data = NULL; \ + BG(serialize).lock_level = BG(serialize).level; + +#define PHP_VAR_SERIALIZE_UNLOCK(prev_data, prev_lock_level) \ + BG(serialize).data = prev_data; \ + BG(serialize).lock_level = prev_lock_level; + +#define PHP_VAR_UNSERIALIZE_LOCK(prev_data, prev_lock_level) \ + prev_data = BG(unserialize).data; \ + prev_lock_level = BG(unserialize).lock_level; \ + BG(unserialize).data = NULL; \ + BG(unserialize).lock_level = BG(unserialize).level; + +#define PHP_VAR_UNSERIALIZE_UNLOCK(prev_data, prev_lock_level) \ + BG(unserialize).data = prev_data; \ + BG(unserialize).lock_level = prev_lock_level; + PHPAPI void var_replace(php_unserialize_data_t *var_hash, zval *ozval, zval *nzval); PHPAPI void var_push_dtor(php_unserialize_data_t *var_hash, zval *val); PHPAPI zval *var_tmp_var(php_unserialize_data_t *var_hashx); diff --git a/ext/standard/tests/serialize/bug70219_1.phpt b/ext/standard/tests/serialize/bug70219_1.phpt index 6bbc593b345d9..6492a9a21e306 100644 --- a/ext/standard/tests/serialize/bug70219_1.phpt +++ b/ext/standard/tests/serialize/bug70219_1.phpt @@ -18,6 +18,7 @@ class obj implements Serializable { } function unserialize($data) { session_decode($data); + return null; } } @@ -33,20 +34,18 @@ for ($i = 0; $i < 5; $i++) { var_dump($data); var_dump($_SESSION); ?> ---EXPECTF-- +--EXPECT-- array(2) { [0]=> - object(obj)#%d (1) { + object(obj)#1 (1) { ["data"]=> NULL } [1]=> - object(obj)#%d (1) { + object(obj)#2 (1) { ["data"]=> NULL } } -object(obj)#1 (1) { - ["data"]=> - NULL +array(0) { } diff --git a/ext/standard/tests/serialize/nested_serialize_under_lock.phpt b/ext/standard/tests/serialize/nested_serialize_under_lock.phpt new file mode 100644 index 0000000000000..193ac44ef6cad --- /dev/null +++ b/ext/standard/tests/serialize/nested_serialize_under_lock.phpt @@ -0,0 +1,64 @@ +--TEST-- +Check that nested serialization/unserialization still works correctly while under lock +--FILE-- +sharedProp = $prop; + } + public function __set($key, $value) + { + $this->$key = $value; + } + public function serialize() + { + return serialize(get_object_vars($this)); + } + public function unserialize($data) + { + $ar = unserialize($data); + if ($ar === false) { + return; + } + foreach ($ar as $k => $v) { + $this->__set($k, $v); + } + } +} + +// Going through spl_autoload_register() to lock serialization +spl_autoload_register(function($class) { + $testPropertyObj = new stdClass(); + $testPropertyObj->name = 'test'; + $array = [ + 'obj1' => new SerializableClass($testPropertyObj), + 'obj2' => new SerializableClass($testPropertyObj), + ]; + var_dump(unserialize(serialize($array))); + class X {} +}); + +unserialize('O:1:"X":0:{}'); +?> +--EXPECT-- +array(2) { + ["obj1"]=> + object(SerializableClass)#5 (1) { + ["sharedProp"]=> + object(stdClass)#6 (1) { + ["name"]=> + string(4) "test" + } + } + ["obj2"]=> + object(SerializableClass)#7 (1) { + ["sharedProp"]=> + object(stdClass)#6 (1) { + ["name"]=> + string(4) "test" + } + } +} diff --git a/ext/standard/var.c b/ext/standard/var.c index cd5ceda889327..d8455932ccfe5 100644 --- a/ext/standard/var.c +++ b/ext/standard/var.c @@ -716,11 +716,12 @@ static int php_var_serialize_call_sleep(zval *retval, zval *struc) /* {{{ */ { zval fname; int res; - + struct php_serialize_data *prev_data; + unsigned prev_lock_level; ZVAL_STRINGL(&fname, "__sleep", sizeof("__sleep") - 1); - BG(serialize_lock)++; + PHP_VAR_SERIALIZE_LOCK(prev_data, prev_lock_level); res = call_user_function(NULL, struc, &fname, retval, 0, 0); - BG(serialize_lock)--; + PHP_VAR_SERIALIZE_UNLOCK(prev_data, prev_lock_level); zval_ptr_dtor_str(&fname); if (res == FAILURE || Z_ISUNDEF_P(retval)) { @@ -742,11 +743,13 @@ static int php_var_serialize_call_magic_serialize(zval *retval, zval *obj) /* {{ { zval fname; int res; + struct php_serialize_data *prev_data; + unsigned prev_lock_level; ZVAL_STRINGL(&fname, "__serialize", sizeof("__serialize") - 1); - BG(serialize_lock)++; + PHP_VAR_SERIALIZE_LOCK(prev_data, prev_lock_level); res = call_user_function(CG(function_table), obj, &fname, retval, 0, 0); - BG(serialize_lock)--; + PHP_VAR_SERIALIZE_UNLOCK(prev_data, prev_lock_level); zval_ptr_dtor_str(&fname); if (res == FAILURE || Z_ISUNDEF_P(retval)) { @@ -1111,29 +1114,28 @@ PHPAPI void php_var_serialize(smart_str *buf, zval *struc, php_serialize_data_t PHPAPI php_serialize_data_t php_var_serialize_init() { struct php_serialize_data *d; - /* fprintf(stderr, "SERIALIZE_INIT == lock: %u, level: %u\n", BG(serialize_lock), BG(serialize).level); */ - if (BG(serialize_lock) || !BG(serialize).level) { + /* fprintf(stderr, "SERIALIZE_INIT == lock_level: %u, level: %u\n", BG(serialize).lock_level, BG(serialize).level); */ + if (BG(serialize).lock_level == BG(serialize).level) { d = emalloc(sizeof(struct php_serialize_data)); zend_hash_init(&d->ht, 16, NULL, ZVAL_PTR_DTOR, 0); d->n = 0; - if (!BG(serialize_lock)) { - BG(serialize).data = d; - BG(serialize).level = 1; - } + BG(serialize).data = d; } else { d = BG(serialize).data; - ++BG(serialize).level; + ZEND_ASSERT(d); } + ++BG(serialize).level; return d; } PHPAPI void php_var_serialize_destroy(php_serialize_data_t d) { - /* fprintf(stderr, "SERIALIZE_DESTROY == lock: %u, level: %u\n", BG(serialize_lock), BG(serialize).level); */ - if (BG(serialize_lock) || BG(serialize).level == 1) { + /* fprintf(stderr, "SERIALIZE_DESTROY == lock: %u, level: %u\n", BG(serialize).lock_level, BG(serialize).level); */ + ZEND_ASSERT(BG(serialize).level > 0); + ZEND_ASSERT(BG(serialize).lock_level < BG(serialize).level); + --BG(serialize).level; + if (BG(serialize).lock_level == BG(serialize).level) { zend_hash_destroy(&d->ht); efree(d); - } - if (!BG(serialize_lock) && !--BG(serialize).level) { BG(serialize).data = NULL; } } diff --git a/ext/standard/var_unserializer.re b/ext/standard/var_unserializer.re index feaccb2a50f9f..ed76652791a63 100644 --- a/ext/standard/var_unserializer.re +++ b/ext/standard/var_unserializer.re @@ -54,8 +54,8 @@ struct php_unserialize_data { PHPAPI php_unserialize_data_t php_var_unserialize_init() { php_unserialize_data_t d; - /* fprintf(stderr, "UNSERIALIZE_INIT == lock: %u, level: %u\n", BG(serialize_lock), BG(unserialize).level); */ - if (BG(serialize_lock) || !BG(unserialize).level) { + /* fprintf(stderr, "UNSERIALIZE_INIT == lock: %u, level: %u\n", BG(unserialize).lock_level, BG(unserialize).level); */ + if (BG(unserialize).lock_level == BG(unserialize).level) { d = emalloc(sizeof(struct php_unserialize_data)); d->last = &d->entries; d->first_dtor = d->last_dtor = NULL; @@ -65,24 +65,23 @@ PHPAPI php_unserialize_data_t php_var_unserialize_init() { d->max_depth = BG(unserialize_max_depth); d->entries.used_slots = 0; d->entries.next = NULL; - if (!BG(serialize_lock)) { - BG(unserialize).data = d; - BG(unserialize).level = 1; - } + BG(unserialize).data = d; } else { d = BG(unserialize).data; - ++BG(unserialize).level; + ZEND_ASSERT(d); } + ++BG(unserialize).level; return d; } PHPAPI void php_var_unserialize_destroy(php_unserialize_data_t d) { - /* fprintf(stderr, "UNSERIALIZE_DESTROY == lock: %u, level: %u\n", BG(serialize_lock), BG(unserialize).level); */ - if (BG(serialize_lock) || BG(unserialize).level == 1) { + /* fprintf(stderr, "UNSERIALIZE_DESTROY == lock: %u, level: %u\n", BG(unserialize).lock_level, BG(unserialize).level); */ + ZEND_ASSERT(BG(unserialize).level > 0); + ZEND_ASSERT(BG(unserialize).lock_level < BG(unserialize).level); + --BG(unserialize).level; + if (BG(unserialize).lock_level == BG(unserialize).level) { var_destroy(&d); efree(d); - } - if (!BG(serialize_lock) && !--BG(unserialize).level) { BG(unserialize).data = NULL; } } @@ -218,6 +217,8 @@ PHPAPI void var_destroy(php_unserialize_data_t *var_hashx) var_entries *var_hash = (*var_hashx)->entries.next; var_dtor_entries *var_dtor_hash = (*var_hashx)->first_dtor; zend_bool delayed_call_failed = 0; + struct php_unserialize_data *prev_data; + unsigned prev_lock_level; zval wakeup_name, unserialize_name; ZVAL_UNDEF(&wakeup_name); ZVAL_UNDEF(&unserialize_name); @@ -247,12 +248,12 @@ PHPAPI void var_destroy(php_unserialize_data_t *var_hashx) ZVAL_STRINGL(&wakeup_name, "__wakeup", sizeof("__wakeup") - 1); } - BG(serialize_lock)++; + PHP_VAR_UNSERIALIZE_LOCK(prev_data, prev_lock_level); if (call_user_function(NULL, zv, &wakeup_name, &retval, 0, 0) == FAILURE || Z_ISUNDEF(retval)) { delayed_call_failed = 1; GC_ADD_FLAGS(Z_OBJ_P(zv), IS_OBJ_DESTRUCTOR_CALLED); } - BG(serialize_lock)--; + PHP_VAR_UNSERIALIZE_UNLOCK(prev_data, prev_lock_level); zval_ptr_dtor(&retval); } else { @@ -268,12 +269,12 @@ PHPAPI void var_destroy(php_unserialize_data_t *var_hashx) ZVAL_STRINGL(&unserialize_name, "__unserialize", sizeof("__unserialize") - 1); } - BG(serialize_lock)++; + PHP_VAR_UNSERIALIZE_LOCK(prev_data, prev_lock_level); if (call_user_function(CG(function_table), zv, &unserialize_name, &retval, 1, ¶m) == FAILURE || Z_ISUNDEF(retval)) { delayed_call_failed = 1; GC_ADD_FLAGS(Z_OBJ_P(zv), IS_OBJ_DESTRUCTOR_CALLED); } - BG(serialize_lock)--; + PHP_VAR_UNSERIALIZE_UNLOCK(prev_data, prev_lock_level); zval_ptr_dtor(¶m); zval_ptr_dtor(&retval); @@ -1028,6 +1029,8 @@ object ":" uiv ":" ["] { zend_bool incomplete_class = 0; zend_bool custom_object = 0; zend_bool has_unserialize = 0; + struct php_unserialize_data *prev_data; + unsigned prev_lock_level; zval user_func; zval retval; @@ -1075,17 +1078,17 @@ object ":" uiv ":" ["] { } /* Try to find class directly */ - BG(serialize_lock)++; + PHP_VAR_UNSERIALIZE_LOCK(prev_data, prev_lock_level); ce = zend_lookup_class(class_name); if (ce) { - BG(serialize_lock)--; + PHP_VAR_UNSERIALIZE_UNLOCK(prev_data, prev_lock_level); if (EG(exception)) { zend_string_release_ex(class_name, 0); return 0; } break; } - BG(serialize_lock)--; + PHP_VAR_UNSERIALIZE_UNLOCK(prev_data, prev_lock_level); if (EG(exception)) { zend_string_release_ex(class_name, 0); @@ -1103,9 +1106,9 @@ object ":" uiv ":" ["] { ZVAL_STRING(&user_func, PG(unserialize_callback_func)); ZVAL_STR_COPY(&args[0], class_name); - BG(serialize_lock)++; + PHP_VAR_UNSERIALIZE_LOCK(prev_data, prev_lock_level); if (call_user_function_ex(NULL, NULL, &user_func, &retval, 1, args, 0, NULL) != SUCCESS) { - BG(serialize_lock)--; + PHP_VAR_UNSERIALIZE_UNLOCK(prev_data, prev_lock_level); if (EG(exception)) { zend_string_release_ex(class_name, 0); zval_ptr_dtor(&user_func); @@ -1119,7 +1122,7 @@ object ":" uiv ":" ["] { zval_ptr_dtor(&args[0]); break; } - BG(serialize_lock)--; + PHP_VAR_UNSERIALIZE_UNLOCK(prev_data, prev_lock_level); zval_ptr_dtor(&retval); if (EG(exception)) { zend_string_release_ex(class_name, 0); @@ -1129,13 +1132,13 @@ object ":" uiv ":" ["] { } /* The callback function may have defined the class */ - BG(serialize_lock)++; + PHP_VAR_UNSERIALIZE_LOCK(prev_data, prev_lock_level); if ((ce = zend_lookup_class(class_name)) == NULL) { php_error_docref(NULL, E_WARNING, "Function %s() hasn't defined the class it was called for", Z_STRVAL(user_func)); incomplete_class = 1; ce = PHP_IC_ENTRY; } - BG(serialize_lock)--; + PHP_VAR_UNSERIALIZE_UNLOCK(prev_data, prev_lock_level); zval_ptr_dtor(&user_func); zval_ptr_dtor(&args[0]);