Fix GH-17037: UAF in user filter when adding existing filter name due…

… to incorrect error handling

There are two functions that can each fail in their own way. If the last
function fails we have to remove the filter entry from the hash table,
otherwise we risk a UAF. Note also that removing the entry from the
table on failure will also free its memory.

Closes GH-17038.

NEWS

@@ -2,6 +2,9 @@ PHP                                                                        NEWS
 |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
 ?? ??? ????, PHP 8.3.16
 
+- Streams:
+  . Fixed bug GH-17037 (UAF in user filter when adding existing filter name due
+    to incorrect error handling). (nielsdos)
 
 19 Dec 2024, PHP 8.3.15
 

ext/standard/tests/filters/gh17037.phpt

@@ -0,0 +1,8 @@
+--TEST--
+GH-17037 (UAF in user filter when adding existing filter name due to incorrect error handling)
+--FILE--
+<?php
+var_dump(stream_filter_register('string.toupper', 'filter_string_toupper'));
+?>
+--EXPECT--
+bool(false)

ext/standard/user_filters.c

@@ -516,13 +516,17 @@ PHP_FUNCTION(stream_filter_register)
 	fdat = ecalloc(1, sizeof(struct php_user_filter_data));
 	fdat->classname = zend_string_copy(classname);
 
-	if (zend_hash_add_ptr(BG(user_filter_map), filtername, fdat) != NULL &&
-			php_stream_filter_register_factory_volatile(filtername, &user_filter_factory) == SUCCESS) {
-		RETVAL_TRUE;
+	if (zend_hash_add_ptr(BG(user_filter_map), filtername, fdat) != NULL) {
+		if (php_stream_filter_register_factory_volatile(filtername, &user_filter_factory) == SUCCESS) {
+			RETURN_TRUE;
+		}
+
+		zend_hash_del(BG(user_filter_map), filtername);
 	} else {
 		zend_string_release_ex(classname, 0);
 		efree(fdat);
-		RETVAL_FALSE;
 	}
+
+	RETURN_FALSE;
 }
 /* }}} */