Fix GH-11189: Exceeding memory limit in zend_hash_do_resize leaves the array in an invalid state

There are more places in zend_hash.c where the resize happened after some values on the HashTable struct were set.
I reordered them all, but writing a test for these would rely on the particular amount of bytes allocated at given points in time.

Author: bwoebi

NEWS

@@ -5,6 +5,8 @@ PHP                                                                        NEWS
 - Core:
   . Fixed bug GH-9068 (Conditional jump or move depends on uninitialised
     value(s)). (nielsdos)
+  . Fixed bug GH-11189 (Exceeding memory limit in zend_hash_do_resize leaves
+    the array in an invalid state). (Bob)
 
 - Opcache:
   . Fixed bug GH-11134 (Incorrect match default branch optimization). (ilutov)

Zend/tests/gh11189.phpt

@@ -0,0 +1,29 @@
+--TEST--
+GH-11189: Exceeding memory limit in zend_hash_do_resize leaves the array in an invalid state (packed array)
+--SKIPIF--
+<?php
+if (getenv("USE_ZEND_ALLOC") === "0") die("skip ZMM is disabled");
+?>
+--INI--
+memory_limit=2M
+--FILE--
+<?php
+
+ob_start(function() {
+    global $a;
+    for ($i = count($a); $i > 0; --$i) {
+        $a[] = 2;
+    }
+    fwrite(STDOUT, "Success");
+});
+
+$a = [];
+// trigger OOM in a resize operation
+while (1) {
+    $a[] = 1;
+}
+
+?>
+--EXPECTF--
+Success
+Fatal error: Allowed memory size of %s bytes exhausted%s(tried to allocate %s bytes) in %s on line %d

Zend/tests/gh11189_1.phpt

@@ -0,0 +1,29 @@
+--TEST--
+GH-11189: Exceeding memory limit in zend_hash_do_resize leaves the array in an invalid state (not packed array)
+--SKIPIF--
+<?php
+if (getenv("USE_ZEND_ALLOC") === "0") die("skip ZMM is disabled");
+?>
+--INI--
+memory_limit=2M
+--FILE--
+<?php
+
+ob_start(function() {
+    global $a;
+    for ($i = count($a); $i > 0; --$i) {
+        $a[] = 2;
+    }
+    fwrite(STDOUT, "Success");
+});
+
+$a = ["not packed" => 1];
+// trigger OOM in a resize operation
+while (1) {
+    $a[] = 1;
+}
+
+?>
+--EXPECTF--
+Success
+Fatal error: Allowed memory size of %s bytes exhausted%s(tried to allocate %s bytes) in %s on line %d

Zend/zend_hash.c

@@ -309,8 +309,9 @@ ZEND_API void ZEND_FASTCALL zend_hash_packed_grow(HashTable *ht)
 	if (ht->nTableSize >= HT_MAX_SIZE) {
 		zend_error_noreturn(E_ERROR, "Possible integer overflow in memory allocation (%u * %zu + %zu)", ht->nTableSize * 2, sizeof(Bucket), sizeof(Bucket));
 	}
-	ht->nTableSize += ht->nTableSize;
-	HT_SET_DATA_ADDR(ht, perealloc2(HT_GET_DATA_ADDR(ht), HT_SIZE_EX(ht->nTableSize, HT_MIN_MASK), HT_USED_SIZE(ht), GC_FLAGS(ht) & IS_ARRAY_PERSISTENT));
+	uint32_t newTableSize = ht->nTableSize * 2;
+	HT_SET_DATA_ADDR(ht, perealloc2(HT_GET_DATA_ADDR(ht), HT_SIZE_EX(newTableSize, HT_MIN_MASK), HT_USED_SIZE(ht), GC_FLAGS(ht) & IS_ARRAY_PERSISTENT));
+	ht->nTableSize = newTableSize;
 }
 
 ZEND_API void ZEND_FASTCALL zend_hash_real_init(HashTable *ht, bool packed)
@@ -346,8 +347,9 @@ ZEND_API void ZEND_FASTCALL zend_hash_packed_to_hash(HashTable *ht)
 	ZEND_ASSERT(HT_SIZE_TO_MASK(nSize));
 
 	HT_ASSERT_RC1(ht);
-	HT_FLAGS(ht) &= ~HASH_FLAG_PACKED;
+	// Alloc before assign to avoid inconsistencies on OOM
 	new_data = pemalloc(HT_SIZE_EX(nSize, HT_SIZE_TO_MASK(nSize)), GC_FLAGS(ht) & IS_ARRAY_PERSISTENT);
+	HT_FLAGS(ht) &= ~HASH_FLAG_PACKED;
 	ht->nTableMask = HT_SIZE_TO_MASK(ht->nTableSize);
 	HT_SET_DATA_ADDR(ht, new_data);
 	memcpy(ht->arData, old_buckets, sizeof(Bucket) * ht->nNumUsed);
@@ -387,17 +389,18 @@ ZEND_API void ZEND_FASTCALL zend_hash_extend(HashTable *ht, uint32_t nSize, bool
 		if (packed) {
 			ZEND_ASSERT(HT_FLAGS(ht) & HASH_FLAG_PACKED);
 			if (nSize > ht->nTableSize) {
-				ht->nTableSize = zend_hash_check_size(nSize);
-				HT_SET_DATA_ADDR(ht, perealloc2(HT_GET_DATA_ADDR(ht), HT_SIZE_EX(ht->nTableSize, HT_MIN_MASK), HT_USED_SIZE(ht), GC_FLAGS(ht) & IS_ARRAY_PERSISTENT));
+				uint32_t newTableSize = zend_hash_check_size(nSize);
+				HT_SET_DATA_ADDR(ht, perealloc2(HT_GET_DATA_ADDR(ht), HT_SIZE_EX(newTableSize, HT_MIN_MASK), HT_USED_SIZE(ht), GC_FLAGS(ht) & IS_ARRAY_PERSISTENT));
+				ht->nTableSize = newTableSize;
 			}
 		} else {
 			ZEND_ASSERT(!(HT_FLAGS(ht) & HASH_FLAG_PACKED));
 			if (nSize > ht->nTableSize) {
 				void *new_data, *old_data = HT_GET_DATA_ADDR(ht);
 				Bucket *old_buckets = ht->arData;
 				nSize = zend_hash_check_size(nSize);
-				ht->nTableSize = nSize;
 				new_data = pemalloc(HT_SIZE_EX(nSize, HT_SIZE_TO_MASK(nSize)), GC_FLAGS(ht) & IS_ARRAY_PERSISTENT);
+				ht->nTableSize = nSize;
 				ht->nTableMask = HT_SIZE_TO_MASK(ht->nTableSize);
 				HT_SET_DATA_ADDR(ht, new_data);
 				memcpy(ht->arData, old_buckets, sizeof(Bucket) * ht->nNumUsed);
@@ -1217,8 +1220,8 @@ static void ZEND_FASTCALL zend_hash_do_resize(HashTable *ht)
 
 		ZEND_ASSERT(HT_SIZE_TO_MASK(nSize));
 
-		ht->nTableSize = nSize;
 		new_data = pemalloc(HT_SIZE_EX(nSize, HT_SIZE_TO_MASK(nSize)), GC_FLAGS(ht) & IS_ARRAY_PERSISTENT);
+		ht->nTableSize = nSize;
 		ht->nTableMask = HT_SIZE_TO_MASK(ht->nTableSize);
 		HT_SET_DATA_ADDR(ht, new_data);
 		memcpy(ht->arData, old_buckets, sizeof(Bucket) * ht->nNumUsed);