ext/xml: reject changing handlers while parsing (#23818)

The SAX handler setters store their callback as a zend_fcall_info_cache on
the parser and xml_set_handler() releases the previous one with
zend_fcc_dtor(). None of the setters checked parser->isparsing, so calling
xml_set_element_handler() (or any xml_set_*_handler()) from inside a running
handler freed the fcc that was still executing. For a non-closure
[$object, 'method'] handler whose object is only referenced by that fcc, the
zend_fcc_dtor() releases the last reference and frees $this while its method
is still on the stack, leading to a use-after-free.

Guard xml_set_element_handler() and the XML_SET_HANDLER_PHP_FUNCTION setters
with the existing isparsing flag and throw "Cannot change handlers while
parsing", mirroring the checks already used by xml_parser_set_option() and
xml_parse(). The parsed fcc is released on the error path like the other
failure branches.

Author: jvoisin

ext/xml/tests/xml_set_handler_reentrancy.phpt

@@ -0,0 +1,43 @@
+--TEST--
+GH-23818 (Changing XML handlers while parsing is rejected)
+--EXTENSIONS--
+xml
+--FILE--
+<?php
+
+$startHandler = static function (XMLParser $parser): void {
+    $replacement = static function () {};
+
+    try {
+        xml_set_element_handler($parser, $replacement, $replacement);
+    } catch (Error $error) {
+        echo 'xml_set_element_handler: ', $error->getMessage(), "\n";
+    }
+
+    try {
+        xml_set_character_data_handler($parser, $replacement);
+    } catch (Error $error) {
+        echo 'xml_set_character_data_handler: ', $error->getMessage(), "\n";
+    }
+};
+
+$parser = xml_parser_create();
+xml_set_element_handler($parser, $startHandler, static function () {
+    echo "end\n";
+});
+xml_set_character_data_handler($parser, static function (XMLParser $parser, string $data) {
+    echo $data, "\n";
+});
+
+var_dump(xml_parse($parser, '<root>text</root>', true));
+var_dump(xml_set_element_handler($parser, null, null));
+var_dump(xml_set_character_data_handler($parser, null));
+?>
+--EXPECT--
+xml_set_element_handler: Cannot change handlers while parsing
+xml_set_character_data_handler: Cannot change handlers while parsing
+text
+end
+int(1)
+bool(true)
+bool(true)

ext/xml/xml.c

@@ -1308,6 +1308,12 @@ PHP_FUNCTION(xml_set_element_handler)
 	}
 
 	set_handlers:
+	if (parser->isparsing) {
+		zend_release_fcall_info_cache(&start_fcc);
+		zend_release_fcall_info_cache(&end_fcc);
+		zend_throw_error(NULL, "Cannot change handlers while parsing");
+		RETURN_THROWS();
+	}
 	xml_set_handler(&parser->startElementHandler, &start_fcc);
 	xml_set_handler(&parser->endElementHandler, &end_fcc);
 	XML_SetElementHandler(parser->parser, xml_startElementHandler, xml_endElementHandler);
@@ -1361,6 +1367,11 @@ static void php_xml_set_handler_parse_callable(
 		php_xml_set_handler_parse_callable(INTERNAL_FUNCTION_PARAM_PASSTHRU, &parser, &handler_fcc); \
 		if (EG(exception)) { return; } \
 		ZEND_ASSERT(parser); \
+		if (parser->isparsing) { \
+			zend_release_fcall_info_cache(&handler_fcc); \
+			zend_throw_error(NULL, "Cannot change handlers while parsing"); \
+			RETURN_THROWS(); \
+		} \
 		xml_set_handler(&parser->parser_handler_name, &handler_fcc); \
 		parse_function(parser->parser, c_function); \
 		RETURN_TRUE; \