Fix GH-22063: stream filter chain UAF on self-removal during callback

NEWS

@@ -218,6 +218,10 @@ PHP                                                                        NEWS
     lookup, and an unchecked CreateFileA() failure. (Ilia Alshanetsky)
   . Fix persistent stream context lifetime during shutdown (Levi Morrison)
 
+- Streams:
+  . Fixed bug GH-22063 (Stream filter chain UAF on self-removal during
+    callback). (Ilia Alshanetsky)
+
 - Tidy:
   . Fixed a use-after-free when a tidyNode is used after its document is
     reparsed. (Ilia Alshanetsky)

ext/standard/streamsfuncs.c

@@ -1344,7 +1344,23 @@ PHP_FUNCTION(stream_filter_remove)
 		RETURN_THROWS();
 	}
 
-	if (php_stream_filter_flush(filter, 1) == FAILURE) {
+	if (!filter->chain || !filter->chain->stream) {
+		php_error_docref(NULL, E_WARNING, "Unable to flush filter, not removing");
+		RETURN_FALSE;
+	}
+
+	php_stream *stream = filter->chain->stream;
+	uint32_t no_remove_flag = filter->chain == &stream->readfilters ?
+			PHP_STREAM_FLAG_NO_READ_FILTER_REMOVE : PHP_STREAM_FLAG_NO_WRITE_FILTER_REMOVE;
+	if (stream->flags & no_remove_flag) {
+		php_error_docref(NULL, E_WARNING, "Cannot remove filter while it is being applied");
+		RETURN_FALSE;
+	}
+
+	stream->flags |= no_remove_flag;
+	zend_result flushed = php_stream_filter_flush(filter, 1);
+	stream->flags &= ~no_remove_flag;
+	if (flushed == FAILURE) {
 		php_error_docref(NULL, E_WARNING, "Unable to flush filter, not removing");
 		RETURN_FALSE;
 	}

ext/standard/tests/streams/gh22063.phpt

@@ -0,0 +1,93 @@
+--TEST--
+GH-22063 (Stream filter chain UAF via self-removal during callback)
+--FILE--
+<?php
+class SelfRemovingFilter extends php_user_filter {
+    public $stream;
+    public static ?string $key = null;
+    public static ?string $other_key = null;
+    public static bool $on_closing_only = false;
+
+    public function filter($in, $out, &$consumed, $closing): int
+    {
+        while ($bucket = stream_bucket_make_writeable($in)) {
+            $consumed += $bucket->datalen;
+            stream_bucket_append($out, $bucket);
+        }
+        if (self::$key !== null && (!self::$on_closing_only || $closing)) {
+            $res = $GLOBALS[self::$key];
+            $other = self::$other_key !== null ? $GLOBALS[self::$other_key] : null;
+            self::$key = null;
+            self::$other_key = null;
+            var_dump(stream_filter_remove($res));
+            if ($other) {
+                var_dump(stream_filter_remove($other));
+            }
+        }
+        return PSFS_PASS_ON;
+    }
+}
+
+stream_filter_register('self-removing', SelfRemovingFilter::class);
+
+echo "write side:\n";
+$f = fopen('php://memory', 'r+');
+SelfRemovingFilter::$key = 'write_res';
+SelfRemovingFilter::$on_closing_only = false;
+$GLOBALS['write_res'] = stream_filter_append($f, 'self-removing', STREAM_FILTER_WRITE);
+fwrite($f, 'hello');
+fwrite($f, ' world');
+rewind($f);
+echo stream_get_contents($f), "\n";
+
+echo "cross-filter side:\n";
+$f = fopen('php://memory', 'r+');
+SelfRemovingFilter::$key = 'cross_res';
+SelfRemovingFilter::$other_key = 'cross_other';
+SelfRemovingFilter::$on_closing_only = false;
+$GLOBALS['cross_res'] = stream_filter_append($f, 'self-removing', STREAM_FILTER_WRITE);
+$GLOBALS['cross_other'] = stream_filter_append($f, 'string.rot13', STREAM_FILTER_WRITE);
+fwrite($f, 'hello world');
+rewind($f);
+echo stream_get_contents($f), "\n";
+
+echo "read side:\n";
+$f = fopen('php://memory', 'r+');
+fwrite($f, 'abcdefghij');
+rewind($f);
+SelfRemovingFilter::$key = 'read_res';
+SelfRemovingFilter::$on_closing_only = false;
+$GLOBALS['read_res'] = stream_filter_append($f, 'self-removing', STREAM_FILTER_READ);
+echo fread($f, 4), '|', fread($f, 6), "\n";
+
+echo "closing-flush side:\n";
+$f = fopen('php://memory', 'r+');
+SelfRemovingFilter::$key = 'close_res';
+SelfRemovingFilter::$on_closing_only = true;
+$GLOBALS['close_res'] = stream_filter_append($f, 'self-removing', STREAM_FILTER_WRITE);
+var_dump(stream_filter_remove($GLOBALS['close_res']));
+?>
+--EXPECTF--
+write side:
+
+Warning: stream_filter_remove(): Cannot remove filter while it is being applied in %s on line %d
+bool(false)
+hello world
+cross-filter side:
+
+Warning: stream_filter_remove(): Cannot remove filter while it is being applied in %s on line %d
+bool(false)
+
+Warning: stream_filter_remove(): Cannot remove filter while it is being applied in %s on line %d
+bool(false)
+uryyb jbeyq
+read side:
+
+Warning: stream_filter_remove(): Cannot remove filter while it is being applied in %s on line %d
+bool(false)
+abcd|efghij
+closing-flush side:
+
+Warning: stream_filter_remove(): Cannot remove filter while it is being applied in %s on line %d
+bool(false)
+bool(true)

ext/standard/tests/streams/gh22063_wrapper.phpt

@@ -0,0 +1,46 @@
+--TEST--
+GH-22063 (stream filter removal denied from a wrapper write during its own removal flush)
+--FILE--
+<?php
+class HoldFilter extends php_user_filter {
+    private string $buf = '';
+    public function filter($in, $out, &$consumed, $closing): int {
+        while ($b = stream_bucket_make_writeable($in)) {
+            $this->buf .= $b->data;
+            $consumed += $b->datalen;
+        }
+        if ($closing && $this->buf !== '') {
+            stream_bucket_append($out, stream_bucket_new($this->stream, $this->buf));
+            $this->buf = '';
+            return PSFS_PASS_ON;
+        }
+        return PSFS_FEED_ME;
+    }
+}
+class RemovingWrapper {
+    public $context;
+    public static bool $armed = false;
+    public function stream_open($path, $mode, $options, &$opened) { return true; }
+    public function stream_write($data) {
+        if (self::$armed) {
+            self::$armed = false;
+            var_dump(stream_filter_remove($GLOBALS['w']));
+        }
+        return strlen($data);
+    }
+}
+stream_filter_register('hold', HoldFilter::class);
+stream_wrapper_register('rmw', RemovingWrapper::class);
+$fp = fopen('rmw://x', 'w');
+$GLOBALS['w'] = stream_filter_append($fp, 'hold', STREAM_FILTER_WRITE);
+fwrite($fp, 'abc');
+RemovingWrapper::$armed = true;
+var_dump(stream_filter_remove($GLOBALS['w']));
+echo "done\n";
+?>
+--EXPECTF--
+
+Warning: stream_filter_remove(): Cannot remove filter while it is being applied in %s on line %d
+bool(false)
+bool(true)
+done

main/php_streams.h

@@ -191,6 +191,9 @@ struct _php_stream_wrapper	{
 
 #define PHP_STREAM_FLAG_NO_IO						0x400
 
+#define PHP_STREAM_FLAG_NO_READ_FILTER_REMOVE		0x800
+#define PHP_STREAM_FLAG_NO_WRITE_FILTER_REMOVE		0x1000
+
 #define PHP_STREAM_FLAG_WAS_WRITTEN					0x80000000
 
 struct _php_stream  {

main/streams/filter.c

@@ -328,7 +328,11 @@ PHPAPI int php_stream_filter_append_ex(php_stream_filter_chain *chain, php_strea
 
 		bucket = php_stream_bucket_new(stream, (char*) stream->readbuf + stream->readpos, stream->writepos - stream->readpos, 0, 0);
 		php_stream_bucket_append(brig_inp, bucket);
+		uint32_t orig_no_remove = stream->flags & PHP_STREAM_FLAG_NO_READ_FILTER_REMOVE;
+		stream->flags |= PHP_STREAM_FLAG_NO_READ_FILTER_REMOVE;
 		status = filter->fops->filter(stream, filter, brig_inp, brig_outp, &consumed, PSFS_FLAG_NORMAL);
+		stream->flags &= ~PHP_STREAM_FLAG_NO_READ_FILTER_REMOVE;
+		stream->flags |= orig_no_remove;
 
 		if (stream->readpos + consumed > (uint32_t)stream->writepos) {
 			/* No behaving filter should cause this. */
@@ -420,16 +424,24 @@ PHPAPI int _php_stream_filter_flush(php_stream_filter *filter, int finish)
 
 	chain = filter->chain;
 	stream = chain->stream;
+	uint32_t no_remove_flag = chain == &stream->readfilters ?
+			PHP_STREAM_FLAG_NO_READ_FILTER_REMOVE : PHP_STREAM_FLAG_NO_WRITE_FILTER_REMOVE;
+	uint32_t orig_no_remove = stream->flags & no_remove_flag;
+	stream->flags |= no_remove_flag;
 
 	for(current = filter; current; current = current->next) {
 		php_stream_filter_status_t status;
 
 		status = current->fops->filter(stream, current, inp, outp, NULL, flags);
 		if (status == PSFS_FEED_ME) {
 			/* We've flushed the data far enough */
+			stream->flags &= ~no_remove_flag;
+			stream->flags |= orig_no_remove;
 			return SUCCESS;
 		}
 		if (status == PSFS_ERR_FATAL) {
+			stream->flags &= ~no_remove_flag;
+			stream->flags |= orig_no_remove;
 			return FAILURE;
 		}
 		/* Otherwise we have data available to PASS_ON
@@ -442,6 +454,8 @@ PHPAPI int _php_stream_filter_flush(php_stream_filter *filter, int finish)
 
 		flags = PSFS_FLAG_NORMAL;
 	}
+	stream->flags &= ~no_remove_flag;
+	stream->flags |= orig_no_remove;
 
 	/* Last filter returned data via PSFS_PASS_ON
 		Do something with it */

main/streams/streams.c

@@ -598,6 +598,8 @@ PHPAPI zend_result _php_stream_fill_read_buffer(php_stream *stream, size_t size)
 			}
 
 			/* wind the handle... */
+			uint32_t orig_no_remove = stream->flags & PHP_STREAM_FLAG_NO_READ_FILTER_REMOVE;
+			stream->flags |= PHP_STREAM_FLAG_NO_READ_FILTER_REMOVE;
 			for (filter = stream->readfilters.head; filter; filter = filter->next) {
 				status = filter->fops->filter(stream, filter, brig_inp, brig_outp, NULL, flags);
 
@@ -613,6 +615,8 @@ PHPAPI zend_result _php_stream_fill_read_buffer(php_stream *stream, size_t size)
 				brig_outp = brig_swap;
 				memset(brig_outp, 0, sizeof(*brig_outp));
 			}
+			stream->flags &= ~PHP_STREAM_FLAG_NO_READ_FILTER_REMOVE;
+			stream->flags |= orig_no_remove;
 
 			switch (status) {
 				case PSFS_PASS_ON:
@@ -1251,6 +1255,8 @@ static ssize_t _php_stream_write_filtered(php_stream *stream, const char *buf, s
 		php_stream_bucket_append(&brig_in, bucket);
 	}
 
+	uint32_t orig_no_remove = stream->flags & PHP_STREAM_FLAG_NO_WRITE_FILTER_REMOVE;
+	stream->flags |= PHP_STREAM_FLAG_NO_WRITE_FILTER_REMOVE;
 	for (filter = stream->writefilters.head; filter; filter = filter->next) {
 		/* for our return value, we are interested in the number of bytes consumed from
 		 * the first filter in the chain */
@@ -1268,6 +1274,8 @@ static ssize_t _php_stream_write_filtered(php_stream *stream, const char *buf, s
 		brig_outp = brig_swap;
 		memset(brig_outp, 0, sizeof(*brig_outp));
 	}
+	stream->flags &= ~PHP_STREAM_FLAG_NO_WRITE_FILTER_REMOVE;
+	stream->flags |= orig_no_remove;
 
 	switch (status) {
 		case PSFS_PASS_ON: