| Age | Commit message (Collapse) | Author |
|---|
|
In a no-database (guest-mode) build registration is non-functional
(yc_register's body is #ifdef DATABASE), so the password field, the
'Register' link and register.html are misleading. Remove them for the
no-DB build while keeping the full login page for a future DB build:
- html/index.html: restored to the full login (password field + Register
link) - used by a DB build.
- html/index_guest.html: new guest login page - no password field, no
Register link, a 'guest mode, no password needed' note.
- Dockerfile (no-DB build): serve index_guest.html as httpd.startsite, and
drop register.html and yc_register.so from the image (the module preload
scans the dir, so a missing .so is simply not loaded; an on-demand
get_module for the register event fails gracefully -> no-op).
Verified: start page is index_guest.html (no password/register);
register.html -> 'Page not found'; yc_register.so absent from the image;
normal login + multi-user chat still work.
|
|
In Mode A (no DB) the register flow is a no-op: yc_register's body is
#ifdef DATABASE, so the registration form at register.html persists nothing
and is misleading. Remove the 'Register a new nick' link from index.html,
keeping the helpful note that guests log in without a password. (register.html
is left in place but no longer linked; the no-DB build has no registration.)
|
|
Root cause of the earlier 2s0 oversized-delete crashes: the context
constructor initialized p_user=NULL but NOT p_map_params or p_response,
so ~context's 'if (p_map_params) delete p_map_params' / 'if (p_response)
delete p_response' ran on garbage (non-NULL) pointers on the early-delete
paths (EOF-empty, hard read error, oversized) -> a corrupted
std::map<string,string> destructor -> infinite _M_erase recursion ->
SIGSEGV. It was never a libevent issue.
Fix:
- src/sock/context.cpp: initialize p_map_params = NULL and p_response =
NULL in the constructor (p_user was already NULL). ~context null-guards
them, so all early-delete paths are now safe.
- src/sock/sock.cpp: the oversized-request branches (headers-incomplete-
full and body-incomplete-full) now drop the request inline via
del_event()+delete (exactly like the EOF/hard-error sibling branches)
instead of leaking the fd/context. This closes the 2s0 fd-exhaustion
DoS (the deliberate leak tradeoff) with no crash.
Verified: 30 oversized POSTs -> open fd count stays at baseline (7->7,
immediate reclaim, no leak), 0 restarts; normal login + multi-user chat
work. Independent review: APPROVE (preferred the simplification to
inline-delete, applied).
|
|
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.
|
|
tool::shell_command (the CGI executor, only called from reqp when
httpd.enablecgi=true and the request ends in .cgi) did popen(s_command,
"r") = /bin/sh -c <templatedir+request>. The request path is URL-
derived, so shell metacharacters (; | $() etc.) in the path were
interpreted by the shell -> command injection / RCE the moment CGI is
enabled. (Disabled by default; reqp's path-traversal guard (yr0) already
prevents '..' escaping the template dir but does not filter
metacharacters.)
Replace popen with fork/execve of the file directly (no shell):
- stat() the path; require a regular file.
- pipe + fork; child dup2's stdout to the pipe, closes inherited fds
(3..OPEN_MAX) so the CGI can't see/hold the listen socket or other
client conns, then execve(path, [path, NULL], [NULL]) with an empty
env; _exit(127) on exec failure.
- parent reads the pipe to EOF (retrying on EINTR) then waitpid.
s_command is passed by value, so the child's COW copy is safe to use
post-fork.
Verified with httpd.enablecgi=true: a /bin/sh CGI returns its output
(CGI-OK); injection attempts sh.cgi;id / test.cgi$(id) / test.cgi|id
return empty (no command execution); server stays up. With enablecgi=false
(default) test.cgi is served as a static template and normal chat works.
Independent review: APPROVE-WITH-NITS; the inherited-fd and EINTR nits
were addressed; the remaining nits (empty envp/no RFC3875 vars, no CGI
timeout, stat vs lstat) are acceptable for a dormant off-by-default
feature and noted for if CGI is ever reactivated.
|
|
chat::get_user(string&, bool&) returned the user only when found and
fell off the end of the non-void function on a miss (undefined behavior;
compiler warns 'control reaches end of non-void function'). The 1-arg
overload inherits the same UB. All callers check b_found before using the
pointer, so add an explicit 'return NULL' on the not-found path to make
the return well-defined. (The 1-arg overload appears unused - left in
place, now safe via this fix.)
Verified: normal chat still works, no crash.
|
|
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.
|
|
wrap.cpp had '//TIMR->run(); // TODO' but timr::run() was never defined
(wouldn't compile if uncommented), and the intended timer thread
(timr::start) ran in a pthread and would race with the single-threaded
libevent main loop (no locking on the shared room/user maps). Net effect:
idle timeouts never ran, ghost users lingered in the online list forever
after disconnect, and message timestamps were always 00:00:00 (s_time
was only updated by the never-run timer).
Also, even single-threaded, check_timeout was unsafe: user::set_online(false)
does p_room->del_elem during hashmap::run_func's live begin/end iteration
-> iterator invalidation/crash.
Fix:
- hashmap::run_func (both overloads): snapshot the values into a vector
before invoking the callback, so callbacks may safely delete from the map
during iteration.
- timr::tick(): single-threaded 1s-cadence tick (called from the libevent
main loop) that updates s_time/s_uptime every tick, and at the top of
each minute runs check_timeout (idle timeout + auto-away), posts a PING
keepalive to all streams, every 10 min cleans the ip cache, hourly runs
garbage collection.
- main.cpp: register a periodic libevent timer (1s, EV_PERSIST) calling
timr::tick() in the event loop - no new thread.
B1 use-after-free (found by independent review, fixed): when an idle user is
reaped it moves to garbage while its long-lived stream context is still
alive; the hourly remove_garbage would delete the user, leaving the
context's p_user dangling (UAF on later disconnect). Fixed by:
- gcol::remove_garbage now keeps garbage users whose stream is still open
(get_stream_fd() >= 0) until the stream closes, deleting only closed-stream
users (collected via a new collect_users_ helper since the hash_map base
is privately inherited and not directly iterable).
- handle_stream_read now reaps the disconnecting user from its room
(set_online(false), a no-op if already idle-reaped) so a clean disconnect
immediately clears the online list instead of lingering until idle-reap.
Also removed the debug 'cout << SETTING OFFLINE' spam in set_online.
Verified: timestamps now real (e.g. 10:29:20 vs old 00:00:00); idle users
reaped at the next top-of-minute tick (no crash); clean disconnect removes
the user from the online list; idle-reaped-then-stream-closed does not UAF
(0 restarts); normal multi-user chat still works. Two independent
fresh-context reviews: APPROVE-WITH-NITS; the B1 UAF and disconnect-reap
findings were addressed; the pre-existing flush_stream hard-error UAF edge
was filed as a separate follow-up.
|
|
sman::generate_id seeded rand() per call with time(0)+chat.session.kloakkey.
Two problems: (1) two logins in the same wall-clock second produced
identical tmpids; the collision retry then re-seeded with the same time
and recursed forever -> stack overflow (SIGSEGV) -> DoS. (2) IDs were
predictable (time-based seed + weak rand) -> session hijacking.
Fix: generate IDs from /dev/urandom (one-time rand() fallback seeded with
time^getpid, seeded once, not per call), and replace the unbounded
recursion with a bounded retry loop (8 attempts). The give-up path
returns a final candidate only if it does not collide (never overwriting/
leaking an existing session), else returns empty so login degrades
gracefully instead of crashing. Also clear the urandom stream's failbit
on a failed read so a transient failure self-heals, and guard i_len<=0.
The runtime-disabled (md5hash=false) md5 transform block is left as-is;
its separate bad-substr bug is out of scope here.
Verified: 15 rapid same-second logins no longer crash (0 restarts); IDs
are distinct; normal login + chat streaming still work. Independent
fresh-context review: APPROVE-WITH-NITS; the give-up/leak, urandom-stuck,
and i_len nits were addressed.
|
|
In guest/no-DB mode (Mode A, no authentication) chat::login granted
operator status (rang 0) to anyone who logged in with the nick matching
chat.defaultop ('Snoop'). An operator can run /exec (mods/commands/
yc_exec.cpp) which does popen() on an attacker-controlled shell string,
i.e. unauth -> RCE, and /set to re-enable disabled commands.
Fix: gate the defaultop grant on p_user->get_is_reg(). In Mode A
is_reg is only ever set under #ifdef DATABASE (compiled out), so no
guest can become operator; the only set_status(0) path is now dead.
Registered defaultops still get op once DB auth (Mode B) is enabled.
Defense-in-depth (per independent review): also physically omit
mods/commands/yc_exec.so from the revival image so the popen RCE
primitive is absent, not merely unreachable.
Verified in a container: logging in as 'Snoop' no longer grants op
(/exec returns 'No such command', no command output); /time and normal
guest use still work. Builds clean. Independent fresh-context review:
APPROVE-WITH-NITS; the drop-yc_exec.so hardening was applied; blanking
chat.defaultop was deliberately skipped to preserve the Mode B
bootstrap-op path.
|
|
Three path-traversal vulnerabilities, all caused by sanitizing the raw
URL before url_decode (so %-encoded %2e%2e / %2f survive the check and
decode to '..' afterwards), and by concatenating attacker-controlled
names into filesystem paths:
1. Arbitrary file read via the HTML template path. src/sock/sock.cpp
stripped the literal '/..' from the raw query before tool::url_decode,
so 'GET /..%2f..%2fetc%2fpasswd' decoded to '../../etc/passwd' and
src/html.cpp opened ifstream(templatedir + request), escaping html/.
Confirmed: returned /etc/passwd, /etc/hosts, and the app's own
etc/ychat.conf (leaking the MySQL password).
Fix: add tool::path_has_traversal (rejects any '.'/'..' path component
and embedded NULs on the DECODED path) and redirect to the notfound
page in handle_client_read when it triggers.
2. Arbitrary shared-object load (RCE) via the html-module dlopen path.
reqp::run_html_mod built 'htmldir + yc_ + s_event + .so' from the
attacker-controlled event query param and dlopen()'d it; 'event=..%2f..'
could load a .so outside the modules dir.
Fix: reject non-alphanumeric s_event before building the path.
3. Same via the command-module dlopen path. user::command built
'commandsdir + yc_ + s_command2 + .so' from the first token of a '/'
chat message and dlopen()'d it.
Fix: reject non-alphanumeric s_command2 before building the path.
Verified in a container: file-read traversal now returns the notfound
page (not file contents); command/module-name traversal is blocked and
not dlopen'd; legit /time, normal pages, login and streaming all still
work. Builds clean.
Independent fresh-context review: APPROVE-WITH-NITS; the dlopen-traversal
and NUL-hardening findings were addressed here; the (disabled-by-default)
CGI popen command-injection was noted as a separate follow-up.
|
|
style.css was served with an empty Content-Type (httpd.contenttypes had no
'css' entry), so browsers refused to apply the stylesheet and the page
rendered unstyled. Add css->text/css and ico->image/x-icon.
|
|
The message-delivery layer was gutted (user::msg_post commented out;
set_context/_send referenced only in comments, never defined), so the
chat could log in but never show or send messages: the stream frame
returned an empty response (502 behind Traefik) and posted messages went
nowhere.
Reimplemented server-streamed chat:
- user: add i_stream_fd (long-lived chat-display connection) + b_stream_ready
+ s_msg buffer. msg_post appends to s_msg and flushes to the fd once the
initial response is sent (best-effort non-blocking write; buffer on
EAGAIN; on hard write error drop the reference and mark offline).
flush_stream/clear_stream helpers; clear_stream on clean()/disconnect.
- reqp (event=stream): build the initial HTTP response (headers, no
Content-Length so it streams incrementally, + the parsed stream.html
body), attach this fd to the user, set p_context->p_user, KEEP_ALIVE=yes.
- sock: route KEEP_ALIVE responses through handle_stream_write (sends the
initial page, then arms a persistent read event) and handle_stream_read
(drains stray bytes; on EOF/error reaps the context and clears the
user's stream). The connection stays open for pushed messages.
- context: init p_user=NULL; track i_buf_len across reads (POST body behind
a proxy arrives in a separate segment).
Verified locally: login -> open stream -> post message -> message appears
in the stream; two users in a room both receive a posted message.
|
|
handle_client_read did a single read() and assumed the whole request
(headers + POST body) arrived in one packet. Behind a reverse proxy
(Traefik) the body is forwarded in a separate TCP segment, so the form
params (event/nick/tmpid/...) were never parsed: logins silently no-op'd,
the frameset was served with tmpid= (empty), and every frame hit
'Session: Could not find session' -> 502 in all frames.
Now accumulate into c_buf across re-arms of the read event (track i_buf_len
in the context), drain non-blocking until EAGAIN, and for POST wait until
the headers AND the full Content-Length body are present before parsing.
EOF/empty and hard read errors are handled without crashing.
|
|
|
|
|
|
session
Multiple latent bugs made the chat crash on real use (login POST segfaulted):
- logd::flush: when a log file can't be opened, log the error to stderr and
exit(1). Previously it called wrap::system_message, which routes back through
LOGD->log_simple_line->flush on the same failing logd -> infinite recursion
-> stack overflow (SIGSEGV). Bitten in k8s where an emptyDir on /app/log
hides the image's /app/log/rooms, so the room log open failed on login.
- tool::trim: rewrite the right-trim; the original did s_str[s_str.size()]
(out-of-bounds under _GLIBCXX_ASSERTIONS / UB) and erased at i_pos=size.
- sock::_make_server_socket: move SO_REUSEADDR setsockopt BEFORE bind so a
quick container restart rebinds port 2000 (was EADDRINUSE -> fallback to
2001, which the k8s Service doesn't target -> 502).
- sock::process_request: init accept() addrlen and bail on any accept error
(not just EAGAIN/EINTR) so we never proceed with fd=-1 (EBADF).
- chat.session.md5hash=false at runtime: the md5 session-id path does
s_hash.substr(s_ret.find(s_salt)+salt.len()+3); the default salt has chars
not in chat.session.validchars so find() returns npos and the substr/append
corrupts the heap and segfaults on login. Overridden via -o in the image CMD.
- docker-entrypoint.sh: mkdir -p /app/log/rooms at startup (volume mount hides
the image's copy).
|
|
parse() constructed the full 'HTTP/1.1 200 OK ... Content-Length ... Content-Type'
response into a local s_resp but never assigned it back, so the socket writer
sent only the body (HTTP/0.9-style). Traefik/proxies 500'd on these responses.
This makes ychat emit proper HTTP/1.1 responses so the f3s ingress can proxy it.
|
|
Multi-stage Dockerfile (Rocky 9 builder + runtime) builds ychat entirely in
a container with SSL/MySQL/readline disabled (in-memory guest chat). Three
minimal legacy-C++ patches so it builds on GCC 11:
- glob.h/modl.cpp: rename project 'function' typedef to 'mod_func_t'
(collides with std::function via 'using namespace std')
- logd.cpp: 'ofstream == NULL' -> '!is_open()'
- sock.cpp: assign i_server_sock before set_nonblock() (was EBADF on startup)
Add DOCKER.md with build/push/deploy workflow and quirks (HTTP/0.9 responses,
ephemeral in-memory state).
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
