Skip to content

Fix use-after-free when pclose() closes a stream from its user filter - #24175

Open
EdmondDantes wants to merge 1 commit into
php:PHP-8.4from
true-async:stream-pclose-in-filter
Open

EdmondDantes wants to merge 1 commit into
php:PHP-8.4from
true-async:stream-pclose-in-filter

Conversation

@EdmondDantes

Copy link
Copy Markdown
Contributor

While a user filter's filter() runs, its stream carries PHP_STREAM_FLAG_NO_FCLOSE, so fclose() from the callback fails with a warning. pclose() did not check the flag: it freed the stream and the filter under the running callback (use-after-free under Valgrind, no Fibers needed).

pclose() now refuses such a stream with the same warning as fclose() and returns -1, its documented error value. One visible change: pclose() of an opendir() handle, which also carries the flag, now fails the way fclose() of it already does.

Test: ext/standard/tests/filters/pclose_in_filter.phpt.

Warning: pclose(): %d is not a valid stream resource in %s on line %d
int(-1)
int(3)
int(%i)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This seems to match too much, i.e. int(-1). Perhaps check is_resource($fp) after calling pclose?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

The test looks like it was written by a paranoid person... Probably a single check would have been enough. I'll check it again.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Fixed

fclose() refuses a stream marked PHP_STREAM_FLAG_NO_FCLOSE, which a user
filter's callback sets on its stream; pclose() closed it anyway, freeing the
stream and the filter under the running callback.
@EdmondDantes
EdmondDantes force-pushed the stream-pclose-in-filter branch from f08b74d to e0643a1 Compare October 7, 2026 10:12
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