Fix bug #70068 (Dangling pointer in the unserialization of ArrayObject items)

2 files changed, 56 insertions, 43 deletions

diff --git a/ext/spl/spl_array.c b/ext/spl/spl_array.c
index ec9ce217d3..a37eced002 100644
--- a/ext/spl/spl_array.c
+++ b/ext/spl/spl_array.c
@@ -309,7 +309,7 @@ static zval **spl_array_get_dimension_ptr_ptr(int check_inherited, zval *object,
 	if (!offset) {
 		return &EG(uninitialized_zval_ptr);
 	}
-	
+
 	if ((type == BP_VAR_W || type == BP_VAR_RW) && (ht->nApplyCount > 0)) {
 		zend_error(E_WARNING, "Modification of ArrayObject during sorting is prohibited");
 		return &EG(error_zval_ptr);;
@@ -341,8 +341,8 @@ static zval **spl_array_get_dimension_ptr_ptr(int check_inherited, zval *object,
 	case IS_RESOURCE:
 		zend_error(E_STRICT, "Resource ID#%ld used as offset, casting to integer (%ld)", Z_LVAL_P(offset), Z_LVAL_P(offset));
 	case IS_DOUBLE:
-	case IS_BOOL: 
-	case IS_LONG: 
+	case IS_BOOL:
+	case IS_LONG:
 		if (offset->type == IS_DOUBLE) {
 			index = (long)Z_DVAL_P(offset);
 		} else {
@@ -386,7 +386,7 @@ static zval *spl_array_read_dimension_ex(int check_inherited, zval *object, zval
 			} else {
 				SEPARATE_ARG_IF_REF(offset);
 			}
-			zend_call_method_with_1_params(&object, Z_OBJCE_P(object), &intern->fptr_offset_get, "offsetGet", &rv, offset);	
+			zend_call_method_with_1_params(&object, Z_OBJCE_P(object), &intern->fptr_offset_get, "offsetGet", &rv, offset);
 			zval_ptr_dtor(&offset);
 			if (rv) {
 				zval_ptr_dtor(&intern->retval);
@@ -444,7 +444,7 @@ static void spl_array_write_dimension_ex(int check_inherited, zval *object, zval
 		zval_ptr_dtor(&offset);
 		return;
 	}
-	
+
 	if (!offset) {
 		ht = spl_array_get_hash_table(intern, 0 TSRMLS_CC);
 		if (ht->nApplyCount > 0) {
@@ -467,8 +467,8 @@ static void spl_array_write_dimension_ex(int check_inherited, zval *object, zval
 		return;
 	case IS_DOUBLE:
 	case IS_RESOURCE:
-	case IS_BOOL: 
-	case IS_LONG: 
+	case IS_BOOL:
+	case IS_LONG:
 		ht = spl_array_get_hash_table(intern, 0 TSRMLS_CC);
 		if (ht->nApplyCount > 0) {
 			zend_error(E_WARNING, "Modification of ArrayObject during sorting is prohibited");
@@ -556,13 +556,13 @@ static void spl_array_unset_dimension_ex(int check_inherited, zval *object, zval
 					    obj->std.properties_table[property_info->offset] = NULL;
 					}
 				}
-			}			
+			}
 		}
 		break;
 	case IS_DOUBLE:
 	case IS_RESOURCE:
-	case IS_BOOL: 
-	case IS_LONG: 
+	case IS_BOOL:
+	case IS_LONG:
 		if (offset->type == IS_DOUBLE) {
 			index = (long)Z_DVAL_P(offset);
 		} else {
@@ -608,7 +608,7 @@ static int spl_array_has_dimension_ex(int check_inherited, zval *object, zval *o
 		}
 		return 0;
 	}
-	
+
 	switch(Z_TYPE_P(offset)) {
 		case IS_STRING:
 			{
@@ -627,9 +627,9 @@ static int spl_array_has_dimension_ex(int check_inherited, zval *object, zval *o
 			return 0;
 		case IS_DOUBLE:
 		case IS_RESOURCE:
-		case IS_BOOL: 
+		case IS_BOOL:
 		case IS_LONG:
-			{	
+			{
 				HashTable *ht = spl_array_get_hash_table(intern, 0 TSRMLS_CC);
 				if (offset->type == IS_DOUBLE) {
 					index = (long)Z_DVAL_P(offset);
@@ -727,7 +727,7 @@ void spl_array_iterator_append(zval *object, zval *append_value TSRMLS_DC) /* {{
 		php_error_docref(NULL TSRMLS_CC, E_NOTICE, "Array was modified outside object and is no longer an array");
 		return;
 	}
-	
+
 	if (Z_TYPE_P(intern->array) == IS_OBJECT) {
 		php_error_docref(NULL TSRMLS_CC, E_RECOVERABLE_ERROR, "Cannot append properties to objects, use %s::offsetSet() instead", Z_OBJCE_P(object)->name);
 		return;
@@ -771,7 +771,7 @@ SPL_METHOD(Array, getArrayCopy)
 {
 	zval *object = getThis(), *tmp;
 	spl_array_object *intern = (spl_array_object*)zend_object_store_get_object(object TSRMLS_CC);
-    
+
     array_init(return_value);
 	zend_hash_copy(HASH_OF(return_value), spl_array_get_hash_table(intern, 0 TSRMLS_CC), (copy_ctor_func_t) zval_add_ref, &tmp, sizeof(zval*));
 } /* }}} */
@@ -990,7 +990,7 @@ static void spl_array_it_dtor(zend_object_iterator *iter TSRMLS_DC) /* {{{ */
 	efree(iterator);
 }
 /* }}} */
-	
+
 static int spl_array_it_valid(zend_object_iterator *iter TSRMLS_DC) /* {{{ */
 {
 	spl_array_it       *iterator = (spl_array_it *)iter;
@@ -1037,7 +1037,7 @@ static int spl_array_it_get_current_key(zend_object_iterator *iter, char **str_k
 		if (spl_array_object_verify_pos_ex(object, aht, "ArrayIterator::current(): " TSRMLS_CC) == FAILURE) {
 			return HASH_KEY_NON_EXISTANT;
 		}
-	
+
 		return zend_hash_get_current_key_ex(aht, str_key, str_key_len, int_key, 1, &object->pos);
 	}
 }
@@ -1057,7 +1057,7 @@ static void spl_array_it_move_forward(zend_object_iterator *iter TSRMLS_DC) /* {
 			php_error_docref(NULL TSRMLS_CC, E_NOTICE, "ArrayIterator::current(): Array was modified outside object and is no longer an array");
 			return;
 		}
-	
+
 		if ((object->ar_flags & SPL_ARRAY_IS_REF) && spl_hash_verify_pos_ex(object, aht TSRMLS_CC) == FAILURE) {
 			php_error_docref(NULL TSRMLS_CC, E_NOTICE, "ArrayIterator::next(): Array was modified outside object and internal position is no longer valid");
 		} else {
@@ -1115,7 +1115,7 @@ static void spl_array_set_array(zval *object, spl_array_object *intern, zval **a
 		if (just_array)	{
 			spl_array_object *other = (spl_array_object*)zend_object_store_get_object(*array TSRMLS_CC);
 			ar_flags = other->ar_flags & ~SPL_ARRAY_INT_MASK;
-		}		
+		}
 		ar_flags |= SPL_ARRAY_USE_OTHER;
 		intern->array = *array;
 	} else {
@@ -1173,7 +1173,7 @@ zend_object_iterator *spl_array_get_iterator(zend_class_entry *ce, zval *object,
 	iterator->intern.ce = ce;
 	iterator->intern.value = NULL;
 	iterator->object = array_object;
-	
+
 	return (zend_object_iterator*)iterator;
 }
 /* }}} */
@@ -1238,7 +1238,7 @@ SPL_METHOD(Array, getIteratorClass)
 {
 	zval *object = getThis();
 	spl_array_object *intern = (spl_array_object*)zend_object_store_get_object(object TSRMLS_CC);
-	
+
 	if (zend_parse_parameters_none() == FAILURE) {
 		return;
 	}
@@ -1253,11 +1253,11 @@ SPL_METHOD(Array, getFlags)
 {
 	zval *object = getThis();
 	spl_array_object *intern = (spl_array_object*)zend_object_store_get_object(object TSRMLS_CC);
-	
+
 	if (zend_parse_parameters_none() == FAILURE) {
 		return;
 	}
-	
+
 	RETURN_LONG(intern->ar_flags & ~SPL_ARRAY_INT_MASK);
 }
 /* }}} */
@@ -1273,7 +1273,7 @@ SPL_METHOD(Array, setFlags)
 	if (zend_parse_parameters(ZEND_NUM_ARGS() TSRMLS_CC, "l", &ar_flags) == FAILURE) {
 		return;
 	}
-	
+
 	intern->ar_flags = (intern->ar_flags & SPL_ARRAY_INT_MASK) | (ar_flags & ~SPL_ARRAY_INT_MASK);
 }
 /* }}} */
@@ -1287,7 +1287,7 @@ SPL_METHOD(Array, exchangeArray)
 
 	array_init(return_value);
 	zend_hash_copy(HASH_OF(return_value), spl_array_get_hash_table(intern, 0 TSRMLS_CC), (copy_ctor_func_t) zval_add_ref, &tmp, sizeof(zval*));
-	
+
 	if (zend_parse_parameters(ZEND_NUM_ARGS() TSRMLS_CC, "Z", &array) == FAILURE) {
 		return;
 	}
@@ -1305,7 +1305,7 @@ SPL_METHOD(Array, getIterator)
 	spl_array_object *intern = (spl_array_object*)zend_object_store_get_object(object TSRMLS_CC);
 	spl_array_object *iterator;
 	HashTable *aht = spl_array_get_hash_table(intern, 0 TSRMLS_CC);
-	
+
 	if (zend_parse_parameters_none() == FAILURE) {
 		return;
 	}
@@ -1328,7 +1328,7 @@ SPL_METHOD(Array, rewind)
 {
 	zval *object = getThis();
 	spl_array_object *intern = (spl_array_object*)zend_object_store_get_object(object TSRMLS_CC);
-	
+
 	if (zend_parse_parameters_none() == FAILURE) {
 		return;
 	}
@@ -1361,9 +1361,9 @@ SPL_METHOD(Array, seek)
 	if (position >= 0) { /* negative values are not supported */
 		spl_array_rewind(intern TSRMLS_CC);
 		result = SUCCESS;
-		
+
 		while (position-- > 0 && (result = spl_array_next(intern TSRMLS_CC)) == SUCCESS);
-	
+
 		if (result == SUCCESS && zend_hash_has_more_elements_ex(aht, &intern->pos) == SUCCESS) {
 			return; /* ok */
 		}
@@ -1383,7 +1383,7 @@ int static spl_array_object_count_elements_helper(spl_array_object *intern, long
 	}
 
 	if (Z_TYPE_P(intern->array) == IS_OBJECT) {
-		/* We need to store the 'pos' since we'll modify it in the functions 
+		/* We need to store the 'pos' since we'll modify it in the functions
 		 * we're going to call and which do not support 'pos' as parameter. */
 		pos = intern->pos;
 		*count = 0;
@@ -1427,7 +1427,7 @@ SPL_METHOD(Array, count)
 {
 	long count;
 	spl_array_object *intern = (spl_array_object*)zend_object_store_get_object(getThis() TSRMLS_CC);
-	
+
 	if (zend_parse_parameters_none() == FAILURE) {
 		return;
 	}
@@ -1443,11 +1443,11 @@ static void spl_array_method(INTERNAL_FUNCTION_PARAMETERS, char *fname, int fnam
 	HashTable *aht = spl_array_get_hash_table(intern, 0 TSRMLS_CC);
 	zval *tmp, *arg = NULL;
 	zval *retval_ptr = NULL;
-	
+
 	MAKE_STD_ZVAL(tmp);
 	Z_TYPE_P(tmp) = IS_ARRAY;
 	Z_ARRVAL_P(tmp) = aht;
-	
+
 	if (!use_arg) {
 		aht->nApplyCount++;
 		zend_call_method(NULL, NULL, NULL, fname, fname_len, &retval_ptr, 1, tmp, NULL TSRMLS_CC);
@@ -1524,7 +1524,7 @@ SPL_METHOD(Array, current)
 	spl_array_object *intern = (spl_array_object*)zend_object_store_get_object(object TSRMLS_CC);
 	zval **entry;
 	HashTable *aht = spl_array_get_hash_table(intern, 0 TSRMLS_CC);
-	
+
 	if (zend_parse_parameters_none() == FAILURE) {
 		return;
 	}
@@ -1547,7 +1547,7 @@ SPL_METHOD(Array, key)
 	if (zend_parse_parameters_none() == FAILURE) {
 		return;
 	}
-	
+
 	spl_array_iterator_key(getThis(), return_value TSRMLS_CC);
 } /* }}} */
 
@@ -1594,7 +1594,7 @@ SPL_METHOD(Array, next)
 
 	spl_array_next_no_verify(intern, aht TSRMLS_CC);
 }
-/* }}} */ 
+/* }}} */
 
 /* {{{ proto bool ArrayIterator::valid()
    Check whether array contains more entries */
@@ -1623,7 +1623,7 @@ SPL_METHOD(Array, hasChildren)
 	zval *object = getThis(), **entry;
 	spl_array_object *intern = (spl_array_object*)zend_object_store_get_object(object TSRMLS_CC);
 	HashTable *aht = spl_array_get_hash_table(intern, 0 TSRMLS_CC);
-	
+
 	if (zend_parse_parameters_none() == FAILURE) {
 		return;
 	}
@@ -1647,7 +1647,7 @@ SPL_METHOD(Array, getChildren)
 	zval *object = getThis(), **entry, *flags;
 	spl_array_object *intern = (spl_array_object*)zend_object_store_get_object(object TSRMLS_CC);
 	HashTable *aht = spl_array_get_hash_table(intern, 0 TSRMLS_CC);
-	
+
 	if (zend_parse_parameters_none() == FAILURE) {
 		return;
 	}
@@ -1687,7 +1687,7 @@ SPL_METHOD(Array, serialize)
 	php_serialize_data_t var_hash;
 	smart_str buf = {0};
 	zval *flags;
-	
+
 	if (zend_parse_parameters_none() == FAILURE) {
 		return;
 	}
@@ -1747,7 +1747,7 @@ SPL_METHOD(Array, unserialize)
 	zval *pmembers, *pflags = NULL;
 	HashTable *aht;
 	long flags;
-	
+
 	if (zend_parse_parameters(ZEND_NUM_ARGS() TSRMLS_CC, "s", &buf, &buf_len) == FAILURE) {
 		return;
 	}
@@ -1774,13 +1774,11 @@ SPL_METHOD(Array, unserialize)
 
 	ALLOC_INIT_ZVAL(pflags);
 	if (!php_var_unserialize(&pflags, &p, s + buf_len, &var_hash TSRMLS_CC) || Z_TYPE_P(pflags) != IS_LONG) {
-		zval_ptr_dtor(&pflags);
 		goto outexcept;
 	}
 
 	--p; /* for ';' */
 	flags = Z_LVAL_P(pflags);
-	zval_ptr_dtor(&pflags);
 	/* flags needs to be verified and we also need to verify whether the next
 	 * thing we get is ';'. After that we require an 'm' or somethign else
 	 * where 'm' stands for members and anything else should be an array. If
@@ -1830,10 +1828,16 @@ SPL_METHOD(Array, unserialize)
 	/* done reading $serialized */
 
 	PHP_VAR_UNSERIALIZE_DESTROY(var_hash);
+	if (pflags) {
+		zval_ptr_dtor(&pflags);
+	}
 	return;
 
 outexcept:
 	PHP_VAR_UNSERIALIZE_DESTROY(var_hash);
+	if (pflags) {
+		zval_ptr_dtor(&pflags);
+	}
 	zend_throw_exception_ex(spl_ce_UnexpectedValueException, 0 TSRMLS_CC, "Error at offset %ld of %d bytes", (long)((char*)p - buf), buf_len);
 	return;
 
@@ -1982,7 +1986,7 @@ PHP_MINIT_FUNCTION(spl_array)
 	REGISTER_SPL_IMPLEMENTS(ArrayIterator, Countable);
 	memcpy(&spl_handler_ArrayIterator, &spl_handler_ArrayObject, sizeof(zend_object_handlers));
 	spl_ce_ArrayIterator->get_iterator = spl_array_get_iterator;
-	
+
 	REGISTER_SPL_SUB_CLASS_EX(RecursiveArrayIterator, ArrayIterator, spl_array_object_new, spl_funcs_RecursiveArrayIterator);
 	REGISTER_SPL_IMPLEMENTS(RecursiveArrayIterator, RecursiveIterator);
 	spl_ce_RecursiveArrayIterator->get_iterator = spl_array_get_iterator;
diff --git a/ext/spl/tests/bug70068.phpt b/ext/spl/tests/bug70068.phpt
new file mode 100644
index 0000000000..92a38dfbd6
--- /dev/null
+++ b/ext/spl/tests/bug70068.phpt
@@ -0,0 +1,9 @@
+--TEST--
+Bug #70068 (Dangling pointer in the unserialization of ArrayObject items)
+--FILE--
+<?php
+$a = unserialize('a:3:{i:0;C:11:"ArrayObject":20:{x:i:0;r:3;;m:a:0:{};}i:1;d:11;i:2;S:31:"AAAAAAAABBBBCCCC\01\00\00\00\04\00\00\00\00\00\00\00\00\00\00";}');
+?>
+OK
+--EXPECT--
+OK
\ No newline at end of file