Fix phpGH-19094: Attaching class with no Iterator implementation to MultipleIterator causes crash

Closes phpGH-19097.

Author: ndossche

NEWS

@@ -47,6 +47,10 @@ PHP                                                                        NEWS
     on object destruction). (nielsdos)
   . Fix memory leak when URL parsing fails in redirect. (Girgias)
 
+- SPL:
+  . Fixed bug GH-19094 (Attaching class with no Iterator implementation to
+    MultipleIterator causes crash). (nielsdos)
+
 - Standard:
   . Fix misleading errors in printf(). (nielsdos)
   . Fix RCN violations in array functions. (nielsdos)

ext/spl/spl_observer.c

@@ -43,6 +43,7 @@ PHPAPI zend_class_entry     *spl_ce_SplObjectStorage;
 PHPAPI zend_class_entry     *spl_ce_MultipleIterator;
 
 PHPAPI zend_object_handlers spl_handler_SplObjectStorage;
+static zend_object_handlers spl_handler_MultipleIterator; /* TODO: make public ? */
 
 /* Bit flags for marking internal functionality overridden by SplObjectStorage subclasses. */
 #define SOS_OVERRIDDEN_READ_DIMENSION  1
@@ -493,6 +494,20 @@ static void spl_object_storage_write_dimension(zend_object *object, zval *offset
 	spl_object_storage_attach_handle(intern, Z_OBJ_P(offset), inf);
 }
 
+static void spl_multiple_iterator_write_dimension(zend_object *object, zval *offset, zval *inf)
+{
+	spl_SplObjectStorage *intern = spl_object_storage_from_obj(object);
+	if (UNEXPECTED(offset == NULL || Z_TYPE_P(offset) != IS_OBJECT || (intern->flags & SOS_OVERRIDDEN_WRITE_DIMENSION))) {
+		zend_std_write_dimension(object, offset, inf);
+		return;
+	}
+	if (UNEXPECTED(!Z_OBJCE_P(offset)->iterator_funcs_ptr || !Z_OBJCE_P(offset)->iterator_funcs_ptr->zf_valid)) {
+		zend_type_error("Can only attach objects that implement the Iterator interface");
+		return;
+	}
+	spl_object_storage_attach_handle(intern, Z_OBJ_P(offset), inf);
+}
+
 static void spl_object_storage_unset_dimension(zend_object *object, zval *offset)
 {
 	spl_SplObjectStorage *intern = spl_object_storage_from_obj(object);
@@ -1352,9 +1367,13 @@ PHP_MINIT_FUNCTION(spl_observer)
 	spl_handler_SplObjectStorage.has_dimension   = spl_object_storage_has_dimension;
 	spl_handler_SplObjectStorage.unset_dimension = spl_object_storage_unset_dimension;
 
+	memcpy(&spl_handler_MultipleIterator, &spl_handler_SplObjectStorage, sizeof(zend_object_handlers));
+
+	spl_handler_MultipleIterator.write_dimension = spl_multiple_iterator_write_dimension;
+
 	spl_ce_MultipleIterator = register_class_MultipleIterator(zend_ce_iterator);
 	spl_ce_MultipleIterator->create_object = spl_SplObjectStorage_new;
-	spl_ce_MultipleIterator->default_object_handlers = &spl_handler_SplObjectStorage;
+	spl_ce_MultipleIterator->default_object_handlers = &spl_handler_MultipleIterator;
 
 	return SUCCESS;
 }

ext/spl/tests/gh19094.phpt

@@ -0,0 +1,56 @@
+--TEST--
+GH-19094 (Attaching class with no Iterator implementation to MultipleIterator causes crash)
+--FILE--
+<?php
+
+class MyIterator implements Iterator {
+    public function valid(): bool {
+        return false;
+    }
+
+    public function current(): mixed {
+        return null;
+    }
+
+    public function key(): string {
+        return "";
+    }
+
+    public function next(): void {
+    }
+
+    public function rewind(): void {
+    }
+}
+
+class MyAggregate implements IteratorAggregate {
+    public function getIterator(): Traversable
+    {
+        throw new Error;
+    }
+}
+
+$cls = new MultipleIterator();
+$canary = new stdClass;
+try {
+    $cls[$canary] = 1;
+} catch (TypeError $e) {
+    echo $e->getMessage(), "\n";
+}
+try {
+    $cls[new MyAggregate] = 1;
+} catch (TypeError $e) {
+    echo $e->getMessage(), "\n";
+}
+$cls[new MyIterator] = 1;
+try {
+    $cls->key();
+} catch (RuntimeException $e) {
+    echo $e->getMessage(), "\n";
+}
+
+?>
+--EXPECT--
+Can only attach objects that implement the Iterator interface
+Can only attach objects that implement the Iterator interface
+Called key() with non valid sub iterator