Skip to content

Fix GH-22844: StreamPollHandle use-after-free after fclose()#22848

Open
iliaal wants to merge 1 commit into
php:masterfrom
iliaal:fix/gh-22844-streampollhandle-uaf
Open

Fix GH-22844: StreamPollHandle use-after-free after fclose()#22848
iliaal wants to merge 1 commit into
php:masterfrom
iliaal:fix/gh-22844-streampollhandle-uaf

Conversation

@iliaal

@iliaal iliaal commented Jul 21, 2026

Copy link
Copy Markdown
Contributor

StreamPollHandle stored the raw php_stream * but referenced only the resource container, so fclose() freed the stream (via pefree()) and cleared res->ptr/res->type while the handle kept a dangling pointer. isValid(), getStream() and the internal get_fd (reached via Context::add()) then dereferenced freed memory. The accessors now re-derive the stream from the held resource and report a closed resource as absent: getStream() returns null, isValid() returns false.

A watcher re-derived its registration fd from that same stream in remove(), so a watcher removed after its stream closed couldn't recover the fd and left the Poll backend holding the freed watcher, which a reused fd would then write through in wait(). It now caches the fd from add() and removes by that. Reproduced under ASAN; getStream()'s stub return becomes resource|null.

Fixes #22844

StreamPollHandle cached the raw php_stream* but referenced only the
resource container, not the stream, so fclose() freed the stream while
the handle kept a dangling pointer that isValid(), getStream() and
get_fd dereferenced. Re-derive the stream from the held resource each
time. A watcher likewise re-derived its registration fd from that
stream in remove(), so removing a watcher after its stream closed
leaked the Poll-backend entry and could be re-hit through a reused fd
in wait(); cache the fd at add() and use it for removal.

Fixes phpGH-22844

@arnaud-lb arnaud-lb left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

We should not cache the fd as it becomes unrelated to the original stream after fclose(). At this point fd may be invalid (best case) or may point to another file, so calling php_poll_remove() may remove an unrelated watch:

use Io\Poll\{Context, Event};

$context = new Context();

list($r0, $w0) = stream_socket_pair(STREAM_PF_UNIX, STREAM_SOCK_STREAM, 0);

$watcher0 = $context->add(new StreamPollHandle($r0), [Event::Read]);

fclose($r0);

// Likely reuses $r0's fd number
list($r1, $w1) = stream_socket_pair(STREAM_PF_UNIX, STREAM_SOCK_STREAM, 0);

$watcher1 = $context->add(new StreamPollHandle($r1), [Event::Read]);

// Likely removes r1
$watcher0->remove();

fwrite($w1, 'hello');

// Should return [$watcher1], but will likely return []
var_dump($context->wait(0));

But if we don't remove fd, we may run into epoll's edge cases with duplicated fds (I don't know about other backends).

We should probably add a callback from php_stream_free to notify Io\Poll when we close a stream, so that Io\Poll gets an opportunity to remove a fd just before it's closed. I did that in arnaud-lb@ecc3309 to support weak Io\Poll handles.

Comment thread ext/standard/io_poll.c
Comment on lines +184 to +189
if (!data || !data->res
|| (data->res->type != php_file_le_stream()
&& data->res->type != php_file_le_pstream())) {
return NULL;
}
return (php_stream *) data->res->ptr;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
if (!data || !data->res
|| (data->res->type != php_file_le_stream()
&& data->res->type != php_file_le_pstream())) {
return NULL;
}
return (php_stream *) data->res->ptr;
if (!data || !data->res) {
return NULL;
}
return php_stream_from_res_no_verify(data->res);

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.

StreamPollHandle UAF

2 participants