From b7fd0abe8136b523d9e32054b11c96a4784c57f2 Mon Sep 17 00:00:00 2001 From: Niels Dossche <7771979+nielsdos@users.noreply.github.com> Date: Sun, 29 Jun 2025 00:37:10 +0200 Subject: [PATCH] Fix OSS-Fuzz #385993744 PSFS_FEED_ME is supposed to be returned when the filter did not receive enough data and did not generate buckets for the output brigade. The test generates buckets anyway on the output brigade, and the stream layer did not handle that case causing a memory leak. To solve this, discard any such buckets as it would conflict with the status code returned by the filter. This keeps BC and solves the leak. --- .../tests/filters/oss_fuzz_385993744.phpt | 28 +++++++++++++++++++ main/streams/filter.c | 7 +++++ main/streams/streams.c | 22 ++++++++++++--- 3 files changed, 53 insertions(+), 4 deletions(-) create mode 100644 ext/standard/tests/filters/oss_fuzz_385993744.phpt diff --git a/ext/standard/tests/filters/oss_fuzz_385993744.phpt b/ext/standard/tests/filters/oss_fuzz_385993744.phpt new file mode 100644 index 0000000000000..bfed9e57a9185 --- /dev/null +++ b/ext/standard/tests/filters/oss_fuzz_385993744.phpt @@ -0,0 +1,28 @@ +--TEST-- +OSS-Fuzz #385993744 +--FILE-- +data .= $bucket->data; + } + + $bucket = stream_bucket_new($this->stream, $this->data); + stream_bucket_append($out, $bucket); + + return PSFS_FEED_ME; + } +} +stream_filter_register('sample.filter', SampleFilter::class); +var_dump(file_get_contents('php://filter/read=sample.filter/resource='. __FILE__)); + +?> +--EXPECT-- +string(0) "" diff --git a/main/streams/filter.c b/main/streams/filter.c index abfc5c26ae12d..a1d63c15f0a52 100644 --- a/main/streams/filter.c +++ b/main/streams/filter.c @@ -354,6 +354,13 @@ PHPAPI int php_stream_filter_append_ex(php_stream_filter_chain *chain, php_strea Reset stream's internal read buffer since the filter is "holding" it. */ stream->readpos = 0; stream->writepos = 0; + + /* Filter could have added buckets anyway, but signalled that it did not return any. Discard them. */ + while (brig_out.head) { + bucket = brig_out.head; + php_stream_bucket_unlink(bucket); + php_stream_bucket_delref(bucket); + } break; case PSFS_PASS_ON: /* If any data is consumed, we cannot rely upon the existing read buffer, diff --git a/main/streams/streams.c b/main/streams/streams.c index 1471c98558e18..7a1b521108257 100644 --- a/main/streams/streams.c +++ b/main/streams/streams.c @@ -630,6 +630,12 @@ PHPAPI zend_result _php_stream_fill_read_buffer(php_stream *stream, size_t size) /* when a filter needs feeding, there is no brig_out to deal with. * we simply continue the loop; if the caller needs more data, * we will read again, otherwise out job is done here */ + + /* Filter could have added buckets anyway, but signalled that it did not return any. Discard them. */ + while ((bucket = brig_outp->head)) { + php_stream_bucket_unlink(bucket); + php_stream_bucket_delref(bucket); + } break; case PSFS_ERR_FATAL: @@ -1263,14 +1269,22 @@ static ssize_t _php_stream_write_filtered(php_stream *stream, const char *buf, s php_stream_bucket_delref(bucket); } break; - case PSFS_FEED_ME: - /* need more data before we can push data through to the stream */ - break; case PSFS_ERR_FATAL: /* some fatal error. Theoretically, the stream is borked, so all * further writes should fail. */ - return (ssize_t) -1; + consumed = (ssize_t) -1; + ZEND_FALLTHROUGH; + + case PSFS_FEED_ME: + /* need more data before we can push data through to the stream */ + /* Filter could have added buckets anyway, but signalled that it did not return any. Discard them. */ + while (brig_inp->head) { + bucket = brig_inp->head; + php_stream_bucket_unlink(bucket); + php_stream_bucket_delref(bucket); + } + break; } return consumed;