diff options
| author | Paul Buetow <paul@buetow.org> | 2026-06-30 17:38:35 +0300 |
|---|---|---|
| committer | Paul Buetow <paul@buetow.org> | 2026-06-30 17:38:35 +0300 |
| commit | 6d079fb6c623e0897c6fc098c8d684565faa7af7 (patch) | |
| tree | 2226a8b4e548f0b09e1194271b3540b61f484c5b | |
| parent | e4de236e355b16a849b086f0ac1bf3b2c2d4964b (diff) | |
Fix flush_stream hard-error use-after-free
On a hard write error (EPIPE/ECONNRESET) flush_stream did clear_stream()
(sets i_stream_fd = -1) then set_online(false) (adds the user to garbage),
while the user's stream context still had its EV_READ|EV_PERSIST event
armed on the real, still-open fd and held p_context->p_user. If the hourly
gcol::remove_garbage fired before the kernel delivered read EOF, it saw
i_stream_fd < 0 (already cleared) and deleted the user - leaving the
context's p_user dangling (UAF on the later disconnect). (SIGPIPE is
ignored via sign.cpp, so the write returns EPIPE/ECONNRESET safely.)
Fix: remove clear_stream() from the hard-error branch (keep i_stream_fd
>= 0). remove_garbage's skip-open-stream check (zr0) then keeps the user
alive until handle_stream_read (read EOF) reaps the context - the single
reaper - after which remove_garbage deletes it safely. set_online(false)
also removes the user from its room, so room broadcasts stop targeting
the dead fd (no repeated writes). set_online(false)'s b_online guard
makes handle_stream_read's later set_online(false) a no-op (no double
reap).
Verified: abruptly closing a user's stream then posting a room message
reaps the user (online list 1->0) with no crash (0 restarts). Independent
review: APPROVE.
| -rw-r--r-- | ychat/src/chat/user.cpp | 12 |
1 files changed, 10 insertions, 2 deletions
diff --git a/ychat/src/chat/user.cpp b/ychat/src/chat/user.cpp index 98f1dbc..9ffb649 100644 --- a/ychat/src/chat/user.cpp +++ b/ychat/src/chat/user.cpp @@ -514,8 +514,16 @@ user::flush_stream() if ( errno == EAGAIN || errno == EINTR ) break; // socket buffer full; keep buffered for later - // Hard error (EPIPE, ECONNRESET, ...): peer gone. - clear_stream(); + // Hard error (EPIPE, ECONNRESET, ...): peer gone. Reap the user from + // its room, but do NOT clear_stream() (keep i_stream_fd >= 0). + // gcol::remove_garbage skips users whose stream is still open, so the + // user is NOT deleted while its stream context (armed read event + + // p_context->p_user) is still alive - that would be a UAF on the later + // disconnect. The read EOF (handle_stream_read) is the single reaper: + // it set_online(false) (no-op here, already offline) + clear_stream + + // delete context, then remove_garbage deletes the user. Removing the + // user from the room also stops room broadcasts targeting it (no + // repeated writes to the dead fd). set_online( false ); return; } |
