From ff84cb08ef06d585b736631c211229487034adba 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. Closes GH-18972. --- NEWS | 1 + .../tests/filters/oss_fuzz_385993744.phpt | 28 +++++++++++++++++++ main/streams/filter.c | 7 +++++ main/streams/streams.c | 22 ++++++++++++--- 4 files changed, 54 insertions(+), 4 deletions(-) create mode 100644 ext/standard/tests/filters/oss_fuzz_385993744.phpt diff --git a/NEWS b/NEWS index 151c3df9613..2ded7b11adc 100644 --- a/NEWS +++ b/NEWS @@ -59,6 +59,7 @@ PHP NEWS - Streams: . Remove incorrect call to zval_ptr_dtor() in user_wrapper_metadata(). (nielsdos) + . Fix OSS-Fuzz #385993744. (nielsdos) - Tidy: . Fixed GH-19021 build issue with libtidy in regard of tidyOptIsReadonly 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 00000000000..bfed9e57a91 --- /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 abfc5c26ae1..a1d63c15f0a 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 1471c98558e..7a1b5211082 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;