Use-after-free in stream_filter_remove() called from a stream callback

ext/standard/streamsfuncs.c

@@ -1344,11 +1344,27 @@ PHP_FUNCTION(stream_filter_remove)
 		RETURN_THROWS();
 	}
 
+	if (filter->running_calls > 0) {
+		php_error_docref(NULL, E_WARNING, "Unable to remove a filter while it is running");
+		RETURN_FALSE;
+	}
+
 	if (php_stream_filter_flush(filter, 1) == FAILURE) {
 		php_error_docref(NULL, E_WARNING, "Unable to flush filter, not removing");
 		RETURN_FALSE;
 	}
 
+	/* The flush can run PHP code, which may remove the filter or enter it. */
+	if (Z_RES_TYPE_P(zfilter) != php_file_le_stream_filter()) {
+		php_error_docref(NULL, E_WARNING, "Filter has already been removed");
+		RETURN_FALSE;
+	}
+
+	if (filter->running_calls > 0) {
+		php_error_docref(NULL, E_WARNING, "Unable to remove a filter while it is running");
+		RETURN_FALSE;
+	}
+
 	zend_list_close(Z_RES_P(zfilter));
 	php_stream_filter_remove(filter, 1);
 	RETURN_TRUE;

ext/standard/tests/filters/stream_filter_remove_during_flush.phpt

@@ -0,0 +1,35 @@
+--TEST--
+stream_filter_remove() of a filter that is removed while it is flushed
+--FILE--
+<?php
+class Sink {
+    public $context;
+
+    public function stream_open($path, $mode, $options, &$opened_path): bool {
+        return true;
+    }
+
+    public function stream_write(string $data): int {
+        if (isset($GLOBALS['remove'])) {
+            $filter = $GLOBALS['remove'];
+            unset($GLOBALS['remove']);
+            var_dump(stream_filter_remove($filter));
+        }
+
+        return strlen($data);
+    }
+}
+
+stream_wrapper_register('sink', 'Sink');
+
+$stream = fopen('sink://', 'w');
+$filter = stream_filter_append($stream, 'convert.base64-encode', STREAM_FILTER_WRITE);
+fwrite($stream, 'ab');
+$GLOBALS['remove'] = $filter;
+var_dump(stream_filter_remove($filter));
+?>
+--EXPECTF--
+bool(true)
+
+Warning: stream_filter_remove(): Filter has already been removed in %s on line %d
+bool(false)

ext/standard/tests/filters/stream_filter_remove_entered_during_flush.phpt

@@ -0,0 +1,43 @@
+--TEST--
+stream_filter_remove() of a user filter that a Fiber entered during its flush
+--FILE--
+<?php
+class Reader extends php_user_filter {
+    public function filter($in, $out, &$consumed, bool $closing): int {
+        global $stream, $fiber;
+
+        if ($closing && !$fiber) {
+            $fiber = new Fiber(fn() => fread($stream, 1));
+            $fiber->start();
+        }
+
+        if ($fiber && Fiber::getCurrent() === $fiber) {
+            Fiber::suspend();
+        }
+
+        while ($bucket = stream_bucket_make_writeable($in)) {
+            $consumed += $bucket->datalen;
+            stream_bucket_append($out, $bucket);
+        }
+
+        return PSFS_PASS_ON;
+    }
+}
+
+stream_filter_register('reader', 'Reader');
+
+$stream = fopen('php://memory', 'w+');
+fwrite($stream, 'abc');
+rewind($stream);
+$filter = stream_filter_append($stream, 'reader', STREAM_FILTER_READ);
+
+var_dump(stream_filter_remove($filter));
+$fiber->resume();
+var_dump($fiber->getReturn());
+var_dump(stream_filter_remove($filter));
+?>
+--EXPECTF--
+Warning: stream_filter_remove(): Unable to remove a filter while it is running in %s on line %d
+bool(false)
+string(1) "a"
+bool(true)

ext/standard/tests/filters/stream_filter_remove_in_filter.phpt

@@ -0,0 +1,56 @@
+--TEST--
+stream_filter_remove() of a user filter while it is running
+--FILE--
+<?php
+class Remover extends php_user_filter {
+    public function filter($in, $out, &$consumed, bool $closing): int {
+        while ($bucket = stream_bucket_make_writeable($in)) {
+            $consumed += $bucket->datalen;
+            stream_bucket_append($out, $bucket);
+        }
+
+        if ($closing && isset($GLOBALS['remove'])) {
+            $filter = $GLOBALS['remove'];
+            unset($GLOBALS['remove']);
+            var_dump(stream_filter_remove($filter));
+        }
+
+        if (Fiber::getCurrent()) {
+            Fiber::suspend();
+        }
+
+        return PSFS_PASS_ON;
+    }
+}
+
+stream_filter_register('remover', 'Remover');
+
+echo "From the filter itself:\n";
+$stream = fopen('php://memory', 'w+');
+$filter = stream_filter_append($stream, 'remover', STREAM_FILTER_WRITE);
+fwrite($stream, 'hello');
+$GLOBALS['remove'] = $filter;
+var_dump(stream_filter_remove($filter));
+
+echo "While the filter is suspended in a Fiber:\n";
+$stream = fopen('php://memory', 'w+');
+$filter = stream_filter_append($stream, 'remover', STREAM_FILTER_WRITE);
+$fiber = new Fiber(fn() => fwrite($stream, 'hello'));
+$fiber->start();
+var_dump(stream_filter_remove($filter));
+$fiber->resume();
+var_dump($fiber->getReturn());
+var_dump(stream_filter_remove($filter));
+?>
+--EXPECTF--
+From the filter itself:
+
+Warning: stream_filter_remove(): Unable to remove a filter while it is running in %s on line %d
+bool(false)
+bool(true)
+While the filter is suspended in a Fiber:
+
+Warning: stream_filter_remove(): Unable to remove a filter while it is running in %s on line %d
+bool(false)
+int(5)
+bool(true)

ext/standard/tests/filters/stream_filter_remove_in_multiple_fibers.phpt

@@ -0,0 +1,49 @@
+--TEST--
+stream_filter_remove() while the same user filter is suspended in several Fibers
+--FILE--
+<?php
+class Suspender extends php_user_filter {
+    public function filter($in, $out, &$consumed, bool $closing): int {
+        while ($bucket = stream_bucket_make_writeable($in)) {
+            $consumed += $bucket->datalen;
+            stream_bucket_append($out, $bucket);
+        }
+
+        if (Fiber::getCurrent()) {
+            Fiber::suspend();
+        }
+
+        return PSFS_PASS_ON;
+    }
+}
+
+stream_filter_register('suspender', 'Suspender');
+
+$stream = fopen('php://memory', 'w+');
+$filter = stream_filter_append($stream, 'suspender', STREAM_FILTER_WRITE);
+
+$fiber1 = new Fiber(fn() => fwrite($stream, 'one'));
+$fiber2 = new Fiber(fn() => fwrite($stream, 'two'));
+$fiber1->start();
+$fiber2->start();
+$fiber1->resume();
+var_dump($fiber1->getReturn());
+
+var_dump(stream_filter_remove($filter));
+
+$fiber2->resume();
+var_dump($fiber2->getReturn());
+
+var_dump(stream_filter_remove($filter));
+
+rewind($stream);
+var_dump(stream_get_contents($stream));
+?>
+--EXPECTF--
+int(3)
+
+Warning: stream_filter_remove(): Unable to remove a filter while it is running in %s on line %d
+bool(false)
+int(3)
+bool(true)
+string(6) "onetwo"

ext/standard/tests/filters/stream_filter_remove_in_multiple_fibers_reverse.phpt

@@ -0,0 +1,49 @@
+--TEST--
+stream_filter_remove() while the same user filter is suspended in several Fibers that finish in reverse order
+--FILE--
+<?php
+class Suspender extends php_user_filter {
+    public function filter($in, $out, &$consumed, bool $closing): int {
+        while ($bucket = stream_bucket_make_writeable($in)) {
+            $consumed += $bucket->datalen;
+            stream_bucket_append($out, $bucket);
+        }
+
+        if (Fiber::getCurrent()) {
+            Fiber::suspend();
+        }
+
+        return PSFS_PASS_ON;
+    }
+}
+
+stream_filter_register('suspender', 'Suspender');
+
+$stream = fopen('php://memory', 'w+');
+$filter = stream_filter_append($stream, 'suspender', STREAM_FILTER_WRITE);
+
+$fiber1 = new Fiber(fn() => fwrite($stream, 'one'));
+$fiber2 = new Fiber(fn() => fwrite($stream, 'two'));
+$fiber1->start();
+$fiber2->start();
+$fiber2->resume();
+var_dump($fiber2->getReturn());
+
+var_dump(stream_filter_remove($filter));
+
+$fiber1->resume();
+var_dump($fiber1->getReturn());
+
+var_dump(stream_filter_remove($filter));
+
+rewind($stream);
+var_dump(stream_get_contents($stream));
+?>
+--EXPECTF--
+int(3)
+
+Warning: stream_filter_remove(): Unable to remove a filter while it is running in %s on line %d
+bool(false)
+int(3)
+bool(true)
+string(6) "twoone"

ext/standard/tests/filters/stream_filter_remove_other_filter_in_filter.phpt

@@ -0,0 +1,35 @@
+--TEST--
+stream_filter_remove() of another filter of the stream from a user filter's callback
+--FILE--
+<?php
+class Remover extends php_user_filter {
+    public function filter($in, $out, &$consumed, bool $closing): int {
+        if (isset($GLOBALS['remove'])) {
+            $filter = $GLOBALS['remove'];
+            unset($GLOBALS['remove']);
+            var_dump(stream_filter_remove($filter));
+        }
+
+        while ($bucket = stream_bucket_make_writeable($in)) {
+            $consumed += $bucket->datalen;
+            stream_bucket_append($out, $bucket);
+        }
+
+        return PSFS_PASS_ON;
+    }
+}
+
+stream_filter_register('remover', 'Remover');
+
+$stream = fopen('php://memory', 'w+');
+stream_filter_append($stream, 'remover', STREAM_FILTER_WRITE);
+$GLOBALS['remove'] = stream_filter_append($stream, 'string.toupper', STREAM_FILTER_WRITE);
+var_dump(fwrite($stream, 'hello'));
+
+rewind($stream);
+var_dump(stream_get_contents($stream));
+?>
+--EXPECT--
+bool(true)
+int(5)
+string(5) "hello"

ext/standard/user_filters.c

@@ -147,6 +147,7 @@ static php_stream_filter_status_t userfilter_filter(
 	/* Make sure the stream is not closed while the filter callback executes. */
 	uint32_t orig_no_fclose = stream->flags & PHP_STREAM_FLAG_NO_FCLOSE;
 	stream->flags |= PHP_STREAM_FLAG_NO_FCLOSE;
+	thisfilter->running_calls++;
 
 	/* Give the userfilter class a hook back to the stream */
 	zend_class_entry *old_scope = EG(fake_scope);
@@ -171,6 +172,7 @@ static php_stream_filter_status_t userfilter_filter(
 			zend_string_release(stream_name);
 			stream->flags &= ~PHP_STREAM_FLAG_NO_FCLOSE;
 			stream->flags |= orig_no_fclose;
+			thisfilter->running_calls--;
 			return PSFS_ERR_FATAL;
 		}
 	}
@@ -234,6 +236,7 @@ static php_stream_filter_status_t userfilter_filter(
 
 	stream->flags &= ~PHP_STREAM_FLAG_NO_FCLOSE;
 	stream->flags |= orig_no_fclose;
+	thisfilter->running_calls--;
 
 	return ret;
 }

main/streams/php_stream_filter_api.h

@@ -116,6 +116,10 @@ struct _php_stream_filter {
 
 	/* filters are auto_registered when they're applied */
 	zend_resource *res;
+
+	/* calls of the filter that have not returned, Fibers suspended in them included;
+	 * the filter must not be freed while there is one */
+	uint32_t running_calls;
 };
 
 /* stack filter onto a stream */