From 950d3d6e9b94b75b266c67bf9e3a85ae9c31905d Mon Sep 17 00:00:00 2001 From: Vektah Date: Fri, 13 Mar 2015 13:15:57 +1100 Subject: Fix bug #69227 and #65967 This patch fixes a use (in zend_gc.c) after free (in spl_observer.c). See https://bugs.php.net/bug.php?id=69227 --- ext/spl/spl_observer.c | 53 +++++++++++++++++++++------------------------ ext/spl/tests/bug53071.phpt | 2 +- ext/spl/tests/bug65967.phpt | 14 ++++++++++++ 3 files changed, 40 insertions(+), 29 deletions(-) create mode 100644 ext/spl/tests/bug65967.phpt diff --git a/ext/spl/spl_observer.c b/ext/spl/spl_observer.c index 5e21088891..6bd0d9011c 100644 --- a/ext/spl/spl_observer.c +++ b/ext/spl/spl_observer.c @@ -86,6 +86,8 @@ typedef struct _spl_SplObjectStorage { /* {{{ */ long flags; zend_function *fptr_get_hash; HashTable *debug_info; + zval **gcdata; + long gcdata_len; } spl_SplObjectStorage; /* }}} */ /* {{{ storage is an assoc aray of [zend_object_value]=>[zval *obj, zval *inf] */ @@ -107,6 +109,10 @@ void spl_SplOjectStorage_free_storage(void *object TSRMLS_DC) /* {{{ */ efree(intern->debug_info); } + if (intern->gcdata_len > 0) { + efree(intern->gcdata); + } + efree(object); } /* }}} */ @@ -263,6 +269,9 @@ static zend_object_value spl_object_storage_new_ex(zend_class_entry *class_type, zend_object_std_init(&intern->std, class_type TSRMLS_CC); object_properties_init(&intern->std, class_type); + intern->gcdata = NULL; + intern->gcdata_len = 0; + zend_hash_init(&intern->storage, 0, NULL, (void (*)(void *))spl_object_storage_dtor, 0); retval.handle = zend_objects_store_put(intern, (zend_objects_store_dtor_t)zend_objects_destroy_object, (zend_objects_free_object_storage_t) spl_SplOjectStorage_free_storage, NULL TSRMLS_CC); @@ -324,7 +333,6 @@ static HashTable* spl_object_storage_debug_info(zval *obj, int *is_temp TSRMLS_D *is_temp = 0; props = Z_OBJPROP_P(obj); - zend_hash_del(props, "\x00gcdata", sizeof("\x00gcdata")); if (intern->debug_info == NULL) { ALLOC_HASHTABLE(intern->debug_info); @@ -360,46 +368,35 @@ static HashTable* spl_object_storage_debug_info(zval *obj, int *is_temp TSRMLS_D } /* }}} */ -/* overriden for garbage collection - * This is very hacky */ +/* overriden for garbage collection */ static HashTable *spl_object_storage_get_gc(zval *obj, zval ***table, int *n TSRMLS_DC) /* {{{ */ { spl_SplObjectStorage *intern = (spl_SplObjectStorage*)zend_object_store_get_object(obj TSRMLS_CC); spl_SplObjectStorageElement *element; - HashTable *props; HashPosition pos; - zval *gcdata_arr = NULL, - **gcdata_arr_pp; - - props = std_object_handlers.get_properties(obj TSRMLS_CC); - - *table = NULL; - *n = 0; + long i = 0; + long requiredLength = intern->storage.nNumOfElements * 2; - /* clean \x00gcdata, as it may be out of date */ - if (zend_hash_find(props, "\x00gcdata", sizeof("\x00gcdata"), (void**) &gcdata_arr_pp) == SUCCESS) { - gcdata_arr = *gcdata_arr_pp; - zend_hash_clean(Z_ARRVAL_P(gcdata_arr)); - } - - if (gcdata_arr == NULL) { - MAKE_STD_ZVAL(gcdata_arr); - array_init(gcdata_arr); - /* don't decrease refcount of members when destroying */ - Z_ARRVAL_P(gcdata_arr)->pDestructor = NULL; + if (requiredLength > intern->gcdata_len) { + if (intern->gcdata_len > 0) { + efree(intern->gcdata); + } - /* name starts with \x00 to make tampering in user-land more difficult */ - zend_hash_add(props, "\x00gcdata", sizeof("\x00gcdata"), &gcdata_arr, sizeof(gcdata_arr), NULL); + intern->gcdata = (zval**)erealloc(intern->gcdata, sizeof(zval*) * requiredLength); + intern->gcdata_len = requiredLength; } zend_hash_internal_pointer_reset_ex(&intern->storage, &pos); while (zend_hash_get_current_data_ex(&intern->storage, (void **)&element, &pos) == SUCCESS) { - add_next_index_zval(gcdata_arr, element->obj); - add_next_index_zval(gcdata_arr, element->inf); + intern->gcdata[i++] = element->obj; + intern->gcdata[i++] = element->inf; zend_hash_move_forward_ex(&intern->storage, &pos); } - return props; + *table = intern->gcdata; + *n = intern->gcdata_len; + + return std_object_handlers.get_properties(obj TSRMLS_CC); } /* }}} */ @@ -782,7 +779,7 @@ SPL_METHOD(SplObjectStorage, serialize) INIT_PZVAL(&members); Z_ARRVAL(members) = zend_std_get_properties(getThis() TSRMLS_CC); Z_TYPE(members) = IS_ARRAY; - zend_hash_del(Z_ARRVAL(members), "\x00gcdata", sizeof("\x00gcdata")); + pmembers = &members; php_var_serialize(&buf, &pmembers, &var_hash TSRMLS_CC); /* finishes the string */ diff --git a/ext/spl/tests/bug53071.phpt b/ext/spl/tests/bug53071.phpt index c2c2605e2e..20b929222c 100644 --- a/ext/spl/tests/bug53071.phpt +++ b/ext/spl/tests/bug53071.phpt @@ -23,5 +23,5 @@ echo "Done.\n"; ?> --EXPECTF-- -int(5) +int(4) Done. diff --git a/ext/spl/tests/bug65967.phpt b/ext/spl/tests/bug65967.phpt new file mode 100644 index 0000000000..e3bb6471c6 --- /dev/null +++ b/ext/spl/tests/bug65967.phpt @@ -0,0 +1,14 @@ +--TEST-- +Bug #65967: SplObjectStorage contains corrupt member variables after garbage collection +--INI-- +zend.enable_gc=1 +--FILE-- + +--EXPECT-- +SplObjectStorage::__set_state(array( +)) -- cgit v1.2.1