Test for unserializing twice

Author: TysonAndre

ext/spl/spl_cachediterable.c

@@ -45,6 +45,8 @@ typedef struct _zval_pair {
 	zval value;
 } zval_pair;
 
+static const zval_pair empty_element_list[0];
+
 typedef struct _spl_cached_entries {
 	size_t size;
 	zval_pair *entries;
@@ -75,18 +77,22 @@ static spl_cachediterable_object *cached_iterable_from_obj(zend_object *obj)
  */
 static bool spl_cached_entries_empty(spl_cached_entries *array)
 {
-	if (array->entries) {
-		ZEND_ASSERT(array->size > 0);
+	if (array->size > 0) {
+		ZEND_ASSERT(array->entries != empty_element_list);
 		return false;
 	}
-	ZEND_ASSERT(array->size == 0);
+	ZEND_ASSERT(array->entries == empty_element_list || array->entries == NULL);
 	return true;
 }
 
-static void spl_cached_entries_default_ctor(spl_cached_entries *array)
+static bool spl_cached_entries_uninitialized(spl_cached_entries *array)
 {
-	array->size = 0;
-	array->entries = NULL;
+	if (array->entries == NULL) {
+		ZEND_ASSERT(array->size == 0);
+		return true;
+	}
+	ZEND_ASSERT((array->entries == empty_element_list && array->size == 0) || array->size > 0);
+	return false;
 }
 
 /* Initializes the range [from, to) to null. Does not dtor existing entries. */
@@ -127,7 +133,8 @@ static void spl_cached_entries_init_from_array(spl_cached_entries *array, zend_a
 			i++;
 		} ZEND_HASH_FOREACH_END();
 	} else {
-		spl_cached_entries_default_ctor(array);
+		array->size = 0;
+		array->entries = (zval_pair *)empty_element_list;
 	}
 }
 
@@ -227,8 +234,13 @@ static void spl_cachediterable_copy_range(spl_cached_entries *array, size_t offs
 static void spl_cached_entries_copy_ctor(spl_cached_entries *to, spl_cached_entries *from)
 {
 	zend_long size = from->size;
+	if (!size) {
+		to->size = 0;
+		to->entries = (zval_pair *)empty_element_list;
+		return;
+	}
 
-	to->size = 0; /* reset size in case ecalloc() fails */
+	to->size = 0; /* reset size in case emalloc() fails */
 	to->entries = safe_emalloc(size, sizeof(zval_pair), 0);
 	to->size = size;
 
@@ -268,6 +280,8 @@ static HashTable* spl_cachediterable_object_get_gc(zend_object *obj, zval **tabl
 	*table = &intern->array.entries[0].key;
 	*n = (int)intern->array.size * 2;
 
+	// TODO: This is probably redundant if dynamic properties are not allowed
+	// and the properties can't be mutated.
 	return ht;
 }
 
@@ -313,6 +327,8 @@ static zend_object *spl_cachediterable_object_new_ex(zend_class_entry *class_typ
 	if (orig && clone_orig) {
 		spl_cachediterable_object *other = cached_iterable_from_obj(orig);
 		spl_cached_entries_copy_ctor(&intern->array, &other->array);
+	} else {
+		intern->array.entries = NULL;
 	}
 
 	return &intern->std;
@@ -365,9 +381,10 @@ PHP_METHOD(CachedIterable, __construct)
 
 	spl_cachediterable_object *intern = Z_CACHEDITERABLE_P(object);
 
-	if (!spl_cached_entries_empty(&intern->array)) {
+	if (UNEXPECTED(!spl_cached_entries_uninitialized(&intern->array))) {
+		zend_throw_exception(spl_ce_RuntimeException, "Called CachedIterable::__construct twice", 0);
 		/* called __construct() twice, bail out */
-		return;
+		RETURN_THROWS();
 	}
 
 	switch (Z_TYPE_P(iterable)) {
@@ -503,10 +520,11 @@ PHP_METHOD(CachedIterable, __unserialize)
 		RETURN_THROWS();
 	}
 	spl_cachediterable_object *intern = Z_CACHEDITERABLE_P(ZEND_THIS);
-	if (intern->array.size) {
+	if (UNEXPECTED(!spl_cached_entries_uninitialized(&intern->array))) {
 		zend_throw_exception(spl_ce_RuntimeException, "Already unserialized", 0);
 		RETURN_THROWS();
 	}
+
 	ZEND_ASSERT(intern->array.entries == NULL);
 
 	size_t num_entries = raw_size / 2;

ext/spl/tests/CachedIterable/CachedIterable_reinit_forbidden.phpt

@@ -0,0 +1,29 @@
+--TEST--
+CachedIterable cannot be re-initialized
+--FILE--
+<?php
+
+$it = new CachedIterable([]);
+
+try {
+    $it->__construct(['first']);
+    echo "Unexpectedly called constructor\n";
+} catch (Throwable $t) {
+    printf("Caught %s: %s\n", $t::class, $t->getMessage());
+}
+var_dump($it);
+try {
+    $it->__unserialize([new ArrayObject(), new stdClass()]);
+    echo "Unexpectedly called __unserialize\n";
+} catch (Throwable $t) {
+    printf("Caught %s: %s\n", $t::class, $t->getMessage());
+}
+var_dump($it);
+?>
+--EXPECT--
+Caught RuntimeException: Called CachedIterable::__construct twice
+object(CachedIterable)#1 (0) {
+}
+Caught RuntimeException: Already unserialized
+object(CachedIterable)#1 (0) {
+}