Fix GH-11874: intl causing segfault in docker images

The segfault happens because zoi->wrapping_obj points to an object that has been freed.
This wrapping_obj is set in IntlIterator_from_StringEnumeration().
Notice how the refcount is not increased in this function.
By switching to ZVAL_OBJ_COPY, the segfault disappears.

We also need to move the responsibility of destroying the iterator to
the iterator itself and keep the object data destruction in the object
destruction. The existing code used a weird recursive destruction
between the iterator and object that was too hard to understand to be
honest. This patch simplifies everything and in the process gets rid of
the leak.

Iterators that are embedded are now responsible for their own
memory cleanup.

Closes GH-17343.

Author: nielsdos

NEWS

@@ -5,6 +5,9 @@ PHP                                                                        NEWS
 - FTP:
   . Fixed bug GH-16800 (ftp functions can abort with EINTR). (nielsdos)
 
+- Intl:
+  . Fixed bug GH-11874 (intl causing segfault in docker images). (nielsdos)
+
 02 Jan 2025, PHP 8.3.16RC1
 
 - Core:

ext/intl/breakiterator/breakiterator_iterators.cpp

@@ -51,6 +51,7 @@ inline BreakIterator *_breakiter_prolog(zend_object_iterator *iter)
 static void _breakiterator_destroy_it(zend_object_iterator *iter)
 {
 	zval_ptr_dtor(&iter->data);
+	/* Don't free iter here because it is allocated as an object on its own, not embedded. */
 }
 
 static void _breakiterator_move_forward(zend_object_iterator *iter)
@@ -79,8 +80,18 @@ static void _breakiterator_rewind(zend_object_iterator *iter)
 	ZVAL_LONG(&zoi_iter->current, (zend_long)pos);
 }
 
+static void zoi_with_current_dtor_self(zend_object_iterator *iter)
+{
+	// Note: wrapping_obj is unused, call to zoi_with_current_dtor() not necessary
+	zoi_with_current *zoi_iter = (zoi_with_current*)iter;
+	ZEND_ASSERT(Z_ISUNDEF(zoi_iter->wrapping_obj));
+
+	// Unlike the other iterators, this iterator is a new, standalone instance
+	zoi_iter->destroy_it(iter);
+}
+
 static const zend_object_iterator_funcs breakiterator_iterator_funcs = {
-	zoi_with_current_dtor,
+	zoi_with_current_dtor_self,
 	zoi_with_current_valid,
 	zoi_with_current_get_current_data,
 	NULL,
@@ -133,6 +144,7 @@ typedef struct zoi_break_iter_parts {
 static void _breakiterator_parts_destroy_it(zend_object_iterator *iter)
 {
 	zval_ptr_dtor(&iter->data);
+	efree(iter);
 }
 
 static void _breakiterator_parts_get_current_key(zend_object_iterator *iter, zval *key)
@@ -231,7 +243,7 @@ void IntlIterator_from_BreakIterator_parts(zval *break_iter_zv,
 	ii->iterator->index = 0;
 
 	((zoi_with_current*)ii->iterator)->destroy_it = _breakiterator_parts_destroy_it;
-	ZVAL_OBJ(&((zoi_with_current*)ii->iterator)->wrapping_obj, Z_OBJ_P(object));
+	ZVAL_OBJ_COPY(&((zoi_with_current*)ii->iterator)->wrapping_obj, Z_OBJ_P(object));
 	ZVAL_UNDEF(&((zoi_with_current*)ii->iterator)->current);
 
 	((zoi_break_iter_parts*)ii->iterator)->bio = Z_INTL_BREAKITERATOR_P(break_iter_zv);

ext/intl/common/common_enum.cpp

@@ -35,25 +35,7 @@ zend_object_handlers IntlIterator_handlers;
 void zoi_with_current_dtor(zend_object_iterator *iter)
 {
 	zoi_with_current *zoiwc = (zoi_with_current*)iter;
-
-	if (!Z_ISUNDEF(zoiwc->wrapping_obj)) {
-		/* we have to copy the pointer because zoiwc->wrapping_obj may be
-		 * changed midway the execution of zval_ptr_dtor() */
-		zval *zwo = &zoiwc->wrapping_obj;
-
-		/* object is still here, we can rely on it to call this again and
-		 * destroy this object */
-		zval_ptr_dtor(zwo);
-	} else {
-		/* Object not here anymore (we've been called by the object free handler)
-		 * Note that the iterator wrapper objects (that also depend on this
-		 * structure) call this function earlier, in the destruction phase, which
-		 * precedes the object free phase. Therefore there's no risk on this
-		 * function being called by the iterator wrapper destructor function and
-		 * not finding the memory of this iterator allocated anymore. */
-		iter->funcs->invalidate_current(iter);
-		zoiwc->destroy_it(iter);
-	}
+	zval_ptr_dtor(&zoiwc->wrapping_obj);
 }
 
 U_CFUNC int zoi_with_current_valid(zend_object_iterator *iter)
@@ -124,6 +106,7 @@ static void string_enum_rewind(zend_object_iterator *iter)
 static void string_enum_destroy_it(zend_object_iterator *iter)
 {
 	delete (StringEnumeration*)Z_PTR(iter->data);
+	efree(iter);
 }
 
 static const zend_object_iterator_funcs string_enum_object_iterator_funcs = {
@@ -148,7 +131,7 @@ U_CFUNC void IntlIterator_from_StringEnumeration(StringEnumeration *se, zval *ob
 	ii->iterator->funcs = &string_enum_object_iterator_funcs;
 	ii->iterator->index = 0;
 	((zoi_with_current*)ii->iterator)->destroy_it = string_enum_destroy_it;
-	ZVAL_OBJ(&((zoi_with_current*)ii->iterator)->wrapping_obj, Z_OBJ_P(object));
+	ZVAL_OBJ_COPY(&((zoi_with_current*)ii->iterator)->wrapping_obj, Z_OBJ_P(object));
 	ZVAL_UNDEF(&((zoi_with_current*)ii->iterator)->current);
 }
 
@@ -157,9 +140,7 @@ static void IntlIterator_objects_free(zend_object *object)
 	IntlIterator_object	*ii = php_intl_iterator_fetch_object(object);
 
 	if (ii->iterator) {
-		zval *wrapping_objp = &((zoi_with_current*)ii->iterator)->wrapping_obj;
-		ZVAL_UNDEF(wrapping_objp);
-		zend_iterator_dtor(ii->iterator);
+		((zoi_with_current*)ii->iterator)->destroy_it(ii->iterator);
 	}
 	intl_error_reset(INTLITERATOR_ERROR_P(ii));
 

ext/intl/tests/gh11874.phpt

@@ -0,0 +1,28 @@
+--TEST--
+GH-11874 (intl causing segfault in docker images)
+--EXTENSIONS--
+intl
+--FILE--
+<?php
+
+foreach(IntlCalendar::getKeywordValuesForLocale('calendar', 'fa_IR', true) as $id) {
+	var_dump($id);
+}
+
+$result = '';
+foreach(IntlTimeZone::createTimeZoneIDEnumeration(IntlTimeZone::TYPE_ANY) as $id) {
+	$result .= $id;
+}
+
+var_dump(strlen($result) > 0);
+echo "No crash\n";
+
+?>
+--EXPECT--
+string(7) "persian"
+string(9) "gregorian"
+string(7) "islamic"
+string(13) "islamic-civil"
+string(12) "islamic-tbla"
+bool(true)
+No crash