Skip to content

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

Open
iliaal wants to merge 1 commit into
php:masterfrom
iliaal:fix/gh-22063-stream-filter-uaf
Open

Fix GH-22063: stream filter chain UAF on self-removal during callback#22083
iliaal wants to merge 1 commit into
php:masterfrom
iliaal:fix/gh-22063-stream-filter-uaf

Conversation

@iliaal
Copy link
Copy Markdown
Contributor

@iliaal iliaal commented May 18, 2026

A php_user_filter that calls stream_filter_remove on its own resource from inside filter() frees the php_stream_filter struct while userfilter_filter still dereferences &thisfilter->abstract and while the chain walks in _php_stream_filter_flush, _php_stream_write_filtered, and _php_stream_fill_read_buffer still need current->next. Two new fields on php_stream_filter, in_callback and deferred_dtor, carry the deferral: php_stream_filter_remove(filter, true) unlinks and runs zend_list_delete immediately but defers the pefree.

A stream filter struct must stay live while a fops->filter() callback
or chain iteration holds it. A php_user_filter that removes its own
resource inside filter() frees the struct under userfilter_filter
(&thisfilter->abstract deref) and under the three chain-walk sites
(current->next read). Defer pefree via an in_callback counter until
every C-level frame holding the filter releases it.

Closes phpGH-22063
Copy link
Copy Markdown
Member

@arnaud-lb arnaud-lb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks right, but I'm wondering if it is possible to fix this in a way that doesn't force all filter iterators to pay attention to removal.

This affects only user filters, and only when removing themselves.

A related issue with user filters is that they were able to close the stream. This has been fixed by flagging the stream in userfilter_filter().

@iliaal
Copy link
Copy Markdown
Contributor Author

iliaal commented May 18, 2026

The iterators' current = current->next advance happens after userfilter_filter returns, so a flag scoped to userfilter_filter alone misses the second UAF site. Same issue with the closing-flush in zif_stream_filter_remove, where the outer flush returns and the caller re-derefs the freed pointer. The iterator boundary has to participate.

Closest alternative: stream-level deferred-free flag + list, each iterator brackets its loop with set/clear/drain. That moves per-iteration logic to per-loop boundaries, I can try that approach if you want?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants