Skip to content

Discard buckets abandoned by a filter that returns no output - #304

Closed
iliaal wants to merge 1 commit into
PHP-8.4from
fix/filter-brigade-drain-84
Closed

Discard buckets abandoned by a filter that returns no output#304
iliaal wants to merge 1 commit into
PHP-8.4from
fix/filter-brigade-drain-84

Conversation

@iliaal

@iliaal iliaal commented Sep 3, 2026

Copy link
Copy Markdown
Owner

A user stream filter can append buckets to its $out brigade and still answer PSFS_FEED_ME or PSFS_ERR_FATAL. Two of the four call sites that dispatch a filter never discard those buckets: _php_stream_filter_flush() returns on both statuses without touching either brigade, and _php_stream_write_filtered() drains only brig_inp, which is the brigade the filter consumed from rather than the one it appended to. php_stream_fill_read_buffer() and php_stream_filter_append_ex() already drain the output brigade on the same statuses, so this is a gap in two of four siblings rather than a design choice.

Every brigade involved is a local on the dispatching frame, and php_stream_bucket_unlink() is the only thing that clears bucket->brigade. An abandoned bucket therefore keeps a pointer into a frame that has returned. If a filter also holds the StreamBucket object in a property, so the bucket is never freed, re-appending it on a later call reaches php_stream_bucket_attach() and unlinks through that stale pointer. ASAN reports stack-use-after-return in php_stream_bucket_unlink() for that shape. It needs the ability to author the filter() method, so it is a correctness bug rather than a security issue.

Both tests fail on an unfixed debug build (6 and 2 leaks) and pass with the fix. ext/standard/tests/filters is green on PHP-8.4; the one failure in ext/standard/tests/streams is bug76136.phpt, which fails identically on an unpatched tree here.

A user filter can append to its $out brigade and still answer PSFS_FEED_ME
or PSFS_ERR_FATAL. _php_stream_filter_flush() returned without touching
either brigade, and _php_stream_write_filtered() drained only brig_inp, so
those buckets leaked with bucket->brigade still pointing at the caller's
stack frame; re-appending such a bucket later writes through it.
php_stream_fill_read_buffer() and php_stream_filter_append_ex() already
discard them.

Closes phpGH-23564
@iliaal
iliaal force-pushed the fix/filter-brigade-drain-84 branch from 905c28e to a844281 Compare September 4, 2026 12:15
@iliaal

iliaal commented Sep 4, 2026

Copy link
Copy Markdown
Owner Author

Promoted upstream as php#23564.

@iliaal iliaal closed this Sep 4, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant