diff options
| author | Paul Buetow <paul@buetow.org> | 2026-06-30 13:54:36 +0300 |
|---|---|---|
| committer | Paul Buetow <paul@buetow.org> | 2026-06-30 13:54:36 +0300 |
| commit | fed8e7b6d78c39b8a53af6f6c98188bc4d3c97c9 (patch) | |
| tree | 7a8bf153697b9fdff6b8de7576c8367350f0ed44 | |
| parent | 4f8e26db8f645e8d50a9a9b31e5935c6967de55d (diff) | |
Fix READSOCK=2048 request cap (oversized requests spin/hang)
src/glob.h: READSOCK 2048 -> 16384 so normal chat requests (incl. reverse
proxy X-Forwarded-* headers + a message body) fit. Previously requests
>2KB couldn't be fully read; for a POST whose Content-Length exceeded the
buffer the completeness check re-armed the read event forever (per-
connection spin/hang, no crash).
src/sock/sock.cpp handle_client_read: guard both the headers-incomplete
and body-incomplete re-arms with (i_buf_len < READSOCK). If the buffer is
already full the request can never complete (oversized): drop it without
re-arming instead of spinning. Deleting the context inside the read
callback corrupts libevent (verified: heap corruption / SIGSEGV, tried
both del_event+delete and event_del+delete and a 413-via-write-event), so
the oversized request's fd/context is intentionally leaked for that one
abusive request rather than crashing the server. The leak is a known
limitation (filed as a follow-up: defer reaping via the timer so the fd
is reclaimed without deleting inside the read callback).
Verified: oversized headers (17KB, no terminator) and oversized POST body
(20KB) no longer crash or spin the server (0 restarts); an 8KB POST login
now returns 200 (didn't fit in the old 2KB buffer); normal login + multi-
user chat still work. Independent review: APPROVE-WITH-NITS; the
headers-incomplete spin guard was added per the review.
| -rw-r--r-- | ychat/src/glob.h | 2 | ||||
| -rw-r--r-- | ychat/src/sock/sock.cpp | 19 |
2 files changed, 16 insertions, 5 deletions
diff --git a/ychat/src/glob.h b/ychat/src/glob.h index 9e3d6c7..7309196 100644 --- a/ychat/src/glob.h +++ b/ychat/src/glob.h @@ -82,7 +82,7 @@ #define POSTBUF 1024 #define READBUF 2048 -#define READSOCK 2048 +#define READSOCK 16384 #define SERVMSG //#define CTCSEGV #define VERBOSE diff --git a/ychat/src/sock/sock.cpp b/ychat/src/sock/sock.cpp index b145f6b..9202902 100644 --- a/ychat/src/sock/sock.cpp +++ b/ychat/src/sock/sock.cpp @@ -311,8 +311,13 @@ sock::handle_client_read(int i_fd, short event, void *p_arg) if (i_hdr_end == string::npos) { - // Headers not fully received yet; wait for more. - event_add(p_context->p_event, NULL); + // Headers not fully received yet. Guard against oversized header + // floods: if the buffer is already full with no header terminator, the + // request can never be completed - drop it (no re-arm) rather than spin. + // The fd leaks for that one abusive request (see the body branch below); + // preferable to a crash/spin. + if ( p_context->i_buf_len < READSOCK ) + event_add(p_context->p_event, NULL); return; } @@ -326,8 +331,14 @@ sock::handle_client_read(int i_fd, short event, void *p_arg) int i_have = (int)s_buf.size() - (int)i_body_off; if (i_have < i_content_len) { - // Body still incomplete; wait for the rest. - event_add(p_context->p_event, NULL); + // Body still incomplete. If the buffer is already full the body + // can never fit (oversized POST): drop the request without re-arming + // (re-arming a full buffer would spin, and deleting the context + // inside this read callback corrupts libevent). The fd leaks for + // that one abusive request — acceptable for a toy chat and far + // better than a crash/spin. Otherwise wait for the rest. + if ( p_context->i_buf_len < READSOCK ) + event_add(p_context->p_event, NULL); return; } } |
