| Age | Commit message (Collapse) | Author |
|---|
|
Post-0.9.1 release; the chat color-picker / POST body parsing fix
(deacb3f) is now in and verified live on f3s.
|
|
The color picker (colors.html) saved fine in the UI but the chosen
colors were never applied to chat messages. End-to-end testing traced
this to two bugs in sock.cpp's HTTP parameter parsing, both affecting
any POSTed chat value containing spaces (notably the "/col col1 col2"
command the 'Save colors' button sends via a hidden form):
1. The POST body was never URL-decoded. Browsers encode the body
(spaces -> '+', '/' -> %2F), so "/col 0000FF FF0000" arrived as
"%2Fcol+0000FF+FF0000" and was never recognized as a command (the
leading '/' was hidden behind %2F). The GET path already url_decodes
s_query up front; the POST body now gets the same treatment.
2. The last request parameter was truncated at the first space, so even
a raw-space body "message=/col 0000FF FF0000" became "message=/col"
and the /col command lost its color arguments, falling back to the
default colors. This also truncated every multi-word chat message to
its first word. The last-param block now takes the full value after
'=' (matching the while-loop branch), with the legacy \AND->& escape.
To keep GET session lookups working (stream.html / online.html, where
tmpid is the last query parameter), the request-line extraction of
s_query now uses i_http_pos (not i_http_pos+1) so the space before
"HTTP/1.1" is excluded -- otherwise it leaked into the last param as a
trailing space (tmpid=...\x20) and broke get_session(). The old
first-space truncation happened to mask this; removing it exposed the
trailing space, which is now fixed at the source.
Verified end-to-end in Docker: /col 0000FF FF0000 (raw and URL-encoded
bodies) now sets nick=#0000ff / text=#ff0000, multi-word chat messages
are preserved, and GET stream/online/session lookups still work.
|
|
Update VERSION define to 0.9.1 in ychat/src/build.h, yhttpd/src/msgs.h,
and ycurses/src/msgs.h. Also bump the 'Version 0.9.0-CURRENT' file-header
comments across ychat/src to 0.9.1-CURRENT, and the README example log
line.
|
|
Remove all f3s/k3s/cluster-specific deployment content from README.md,
AGENTS.md, and ychat/DOCKER.md. The DB-backed build's deployment status,
live URL, image tag, PVC, Helm chart, and ArgoCD app details now live in
the private homelab skill's references/ychat.md sub-reference instead of
this public repo.
- README.md: drop the 'Deploying to the f3s k3s cluster' section and the
'Deployed to f3s' table note; genericize the emptyDir log note.
- AGENTS.md: drop the explicit f3s skill name from the deployment section;
keep only a generic out-of-scope pointer to the homelab skill.
- ychat/DOCKER.md: remove the 'still running live on the f3s k3s cluster'
History mention, the 'Not yet deployed to f3s' block, and the entire
'Deploying to the f3s k3s cluster' section.
|
|
Align all three subprojects on 0.9.0 (ychat was already 0.9.0):
- yhttpd src/msgs.h: VERSION 0.8 -> 0.9.0 (BRANCH stays CURRENT, BUILDNR
unchanged at 4027).
- yhttpd VERSION file: 0.8.3-CURRENT Build 4003 -> 0.9.0-CURRENT Build 4027,
reconciled with the msgs.h macros (the file and macro previously
disagreed on both the version string and the build number).
- ycurses src/msgs.h: VERSION 0.1 -> 0.9.0.
ychat src/build.h is unchanged (already 0.9.0-CURRENT, BUILDNR 4325).
|
|
Two residual stability bugs in yhttpd's own sock.cpp (ychat never had
them -- yhttpd's read_http request parser is structurally different
from ychat's), found while auditing for ychat engine-fix backports:
1. sock::read_http malformed Content-Length crash (a3908e1-class).
read_http matched the header on the 15-char prefix "Content-Length:"
(no space required) but then assumed the canonical
"Content-Length: <value>" form and substr'd from index 16. Two
reachable crash cases for any unauthenticated client:
- bare "Content-Length:" (15 chars): substr(16, len-16) had
pos > size -> std::out_of_range throw -> uncaught -> process crash.
- "Content-Length:\n" (16 chars, no value): the substring was
empty so the do/while digit scan read past the buffer (OOB read)
until a stray '\n' in adjacent memory.
Now guarded: require the space separator + a value before substr,
and bound the scan to the substring length. Verified in Docker:
both malformed cases close gracefully, a valid Content-Length: 0
POST still returns 200, server stays up.
2. sock::start unchecked accept(). The accept() return was used
unchecked; on failure (fd == -1, e.g. EMFILE/ENFILE under fd
exhaustion, EINTR) FD_SET(-1, &active_fd_set) is UB (bit-op on a
negative index) and the later _create_container(-1) would
read/write fd -1 (EBADF). Now bails with ACCPERR and continues on
any accept error (the accept-bail half of ychat's 1c36abe, which
the original yhttpd port only carried the size_t->socklen_t init of).
yhttpd is not deployed to the cluster (no Helm chart/ArgoCD app); this
is a build-and-verify-in-Docker project, so no deploy step. ycurses
shares no socket/template engine with ychat (it is a standalone curses
library demo) so nothing applies there.
|
|
When a chat request carried an invalid/expired tmpid (no matching
session), reqp::parse returned early with an empty response, leaving
the browser with a blank page -- e.g. after a server restart (sessions
are in-memory) or reloading a bookmarked frameset URL with a stale
tmpid.
Instead serve a small redirect.html page that does a top-level JS
redirect (top.location.href) back to the login page (httpd.startsite,
i.e. index.html). Using a *top-level* redirect matters because the
chat UI is a frameset of iframes (stream/online/input) that each
reload with ?tmpid=... ; a plain in-iframe redirect would render three
stacked login forms inside the frameset, while top.location sends the
whole chat window back to the login form. top === self when this page
is loaded directly (no parent frameset), so one line covers both
cases. A noscript <a> link is included as a fallback.
The redirect response is built in the p_sess==NULL branch (mirroring
the header-wrapping at the end of parse()) and returned, so the normal
template-render path -- which would re-render the originally-requested
frame -- does not run.
|
|
Two refinements to the mobile chat layout in body.chatlayout:
* height: 100dvh (with 100vh kept first as a fallback). 100vh is the
largest possible viewport, i.e. with the mobile browser's address bar
hidden; with this layout's overflow:hidden that pushed the bottom row
(input frame, Send/Select buttons) off-screen whenever the address bar
was actually showing, which is most of the time. 100dvh tracks the real
currently-visible viewport instead.
* .online-frame mobile height 90px -> 56px. 90px left a lot of visibly
empty black space below the room name and a user or two -- most rooms
don't have enough online users to fill it. 56px fits the room heading
+ 1-2 names; the frame still scrolls internally (iframes do by
default) if a room gets busier.
|
|
The 96px .chatlayout-bottom height on narrow screens was sized from
desktop CSS math, but real mobile browsers render form controls taller
than that math assumed. Verified with a headless-Chrome screenshot of
input.html inside a same-size iframe: at 96px the link row (Colors/
Options/Help/.../Logout + scroll checkbox) was clipped entirely below
body.inputframe's overflow-y:auto fallback -- present in the DOM but
invisible without an undiscoverable scroll inside a tiny strip, which
is what showed up as "cramped" in a real phone screenshot. Re-verified
at several heights the same way; 120px fits both rows with room to
spare.
|
|
sock::handle_client_read computed `s_buf.find(" HTTP", 0) + 1` directly
into an int for both the GET and POST request-line parsers. When a
request has no " HTTP" token at all (e.g. a scanner sending a bare
"GET" with no path/version and closing the connection), find() returns
string::npos and the "+ 1" wraps a 64-bit npos to 0 before the result
is ever compared against string::npos, so the intended invalid-request
guard never fired. Execution fell through to substr(5, ...), which
throws std::out_of_range whenever the received buffer is shorter than
5 bytes -- an uncaught exception that kills the whole process (seen
live on f3s: a vulnerability-scanner probe crashed the pod twice).
Fixed by checking find()'s result for npos, and requiring at least 5
bytes to extract from, before doing any arithmetic on it -- both cases
now hit the existing "invalid request" (HTTPERR) path instead.
Reproduced against the previously deployed image (b8d28a1): a raw
3-byte "GET" with an immediate connection close crashed it every time.
The fixed build survives that plus a batch of other short/malformed
request lines (bare "GET"/"POST" variants, truncated methods, empty
requests), while register/login/wrong-password behavior is unchanged.
|
|
Six popup links (Colors/Options/Help/Users/Admin/Logout) plus a scroll
toggle wrapped onto several lines at the default 16px on a phone-width
screen, which is what was actually eating the input frame's vertical
budget. On narrow screens the row is now a single horizontally
scrollable strip at 13px instead of wrapping, and the "Scrolling:" text
label is dropped in favor of a title tooltip on the checkbox since
there's no room for it. With the row height now predictable (one line,
not an unbounded wrap), .chatlayout-bottom shrinks back from 108px to
96px.
|
|
The message text box in the input frame kept its size="60" intrinsic
width instead of shrinking to fit next to the Send/Select buttons on a
narrow screen: flex items default to min-width:auto, which for an
<input> resolves to its size attribute, not its CSS width, so
max-width:100% alone had no effect. Fixed with min-width:0 + flex:1 1
auto on .input-row .text. Also tightened the input frame's body padding
(it's a short fixed-height strip) and added overflow-y:auto as a
fallback so controls scroll into view instead of clipping if they still
don't fit.
Also added margin-top above the "Register a new nick..." line on the
login page, which had no separation from the login form above it.
|
|
frameset.html used a classic HTML <frameset> (cols=*,150 / rows=*,0) to
split the screen into message stream, userlist, and input areas. Framesets
can't reflow via CSS media queries (cols/rows are fixed HTML attributes,
not CSS) and are deprecated/unreliable in modern mobile browsers, making
this the main mobile blocker. Replaced it with named iframes inside a
flexbox layout (style.css: body.chatlayout and friends) — named iframes
still expose the same window.<name> access the existing JS relies on
(parent.stream.autoscroll()/stopscroll()), so no JS changes were needed.
A media query restacks the userlist below the stream and grows the input
row's height on narrow viewports instead of it eating ~40% of a phone's
width as a fixed side column.
Other mobile fixes:
- Added <meta name="viewport"> to every template (was missing everywhere).
- style.css: capped .text input width (size=60/40 attributes overflowed a
~375px screen; CSS width wins over the size attribute so one rule fixes
every such field), set 16px input font-size to stop iOS Safari's
auto-zoom-on-focus, gave submit/button inputs larger tap targets, and let
wide tables (color picker, admin/help content) scroll horizontally
instead of blowing out the page width.
- input.html: replaced the two position:absolute control rows (fixed pixel
offsets, no reflow) with wrapping flexbox rows (.input-row/.input-links)
so the message box and action links stack on narrow screens.
Verified by building the Dockerfile image, running it, registering/logging
in a test user via curl, and confirming the *served* (token-substituted)
HTML for index.html, register.html, frameset.html, and the stream/online/
input frames it references all contain the viewport meta tag and new CSS
classes, with %%tmpid%% correctly substituted in iframe src URLs.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
|
|
Amp-Thread-ID: https://ampcode.com/threads/T-019f299f-7596-73b6-ba71-0f71fcd3c131
Co-authored-by: Amp <amp@ampcode.com>
|
|
Amp-Thread-ID: https://ampcode.com/threads/T-019f299f-7596-73b6-ba71-0f71fcd3c131
Co-authored-by: Amp <amp@ampcode.com>
|
|
Amp-Thread-ID: https://ampcode.com/threads/T-019f299f-7596-73b6-ba71-0f71fcd3c131
Co-authored-by: Amp <amp@ampcode.com>
|
|
Reflects the f3s Helm chart change (persistent-volume.yaml + deployment.yaml
update) that rolled image tag 67babb2 out live.
|
|
Amp-Thread-ID: https://ampcode.com/threads/T-019f299f-7596-73b6-ba71-0f71fcd3c131
Co-authored-by: Amp <amp@ampcode.com>
|
|
DATABASE/SQLite is no longer optional. src/configure.ac and the generated
src/configure now check sqlite3.h/-lsqlite3 unconditionally, the same way
pthread/libevent already were, right after those checks (matching order in
both files) - there's no --enable-sqlite opt-in any more, and configure
aborts via header_error/lib_error if SQLite isn't available rather than
silently producing the old in-memory-only, no-account "Mode A" guest chat.
--enable-mysql is left alone (pre-existing, separately broken, out of scope
- this is about ychat always having *a* database, not about MySQL).
With DATABASE guaranteed, the three recent no-DB-build UI special-cases
(651f762, 0cdec77, 6c3a65b) are dead code, so they're reverted: deleted
html/index_guest.html, reverted html/input.html + src/reqp.cpp to always
render a static Options link (dropped the #ifdef DATABASE/%%OPTIONS_LINK%%
templating), and the Dockerfile no longer strips register.html/options.html
or their .so modules. Unregistered guest chatting itself is untouched -
chat.enableguest is a runtime config toggle independent of the compile-time
database requirement, and a guest's is_reg is still always false so a guest
can never claim operator via chat.defaultop.
Consolidated the two Dockerfiles into one (SQLite-backed; deleted
Dockerfile.sqlite) and merged DOCKER.md/DOCKER-SQLITE.md into a single
DOCKER.md. Updated root README.md and etc/ychat.conf's option descriptions
to stop claiming the no-DB build is live/default.
Verified in a Rocky Linux 9 podman container: index.html has the password
field + Register link, register.html/options.html both resolve, POSTing to
register.html creates a SQLite user row, wrong password is rejected and the
correct one logs in, the same account's login still works identically after
a full container restart with /app/data bind-mounted (persistence), and
unregistered guest login still works. Independently reproduced by a
fresh-context review agent, which also rebuilt + re-verified the whole flow
itself and caught one real (if harmless) issue - the generated src/configure
had the SQLite check in a different physical position than configure.ac's -
now fixed so both files agree on ordering.
Not deployed to f3s: the live cluster still runs the old no-DB image.
Rolling this out needs a persistent volume for /app/data and an updated
Helm chart - a deliberate follow-up, not done here.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
|
|
Adds a second database backend alongside the (already-broken/dormant) MySQL
one: --enable-sqlite (configure.ac/configure, mirrors --enable-mysql),
USE_SQLITE (glob.h, mirrors USE_MYSQL), and a con/data implementation using
sqlite3_prepare_v2/bind/step (parameterized queries - safer than the MySQL
path's hand-rolled character-transliteration escaping). chat.database.dbname
doubles as the SQLite file path; the "user" table is created on first
connect (CREATE TABLE IF NOT EXISTS) since SQLite has no separate schema-
provisioning step. New Dockerfile.sqlite (Mode B) builds and runs this in
Rocky Linux 9; DOCKER-SQLITE.md documents everything in detail.
DATABASE had never actually been compiled before this (Mode A always
disables it, and --enable-mysql doesn't work - configure.ac registers it as
AC_ARG_ENABLE(mysqlclient,...) but the gating check tests a third, never-set
$enable_mysql - left alone, this task is about moving away from MySQL, not
fixing it). Getting DATABASE to compile and actually run for the first time
surfaced two real, previously-undetectable bugs, both fixed:
- class data collided with std::data() (C++17) under "using namespace std"
("reference to 'data' is ambiguous") - renamed to ychatdb throughout
(data.h/cpp, wrap.h/cpp, yc_register.cpp). Same bug class as the
function->mod_func_t rename already made in glob.h.
- data_base.cpp's config-query parser used "unsigned i_pos" for a
string::npos comparison - truncating npos to 32-bit makes the
"no more tokens" check never true, and i_pos+1 wraps back to 0, so the
loop never advances: an infinite loop that OOM-killed the container within
seconds of startup. Fixed to size_t (same bug class already fixed
repeatedly in ../yhttpd).
Verified in Docker: register creates a row, wrong password is rejected,
correct password succeeds, and - the actual point of this task - a second
user's login still works identically after a full container restart with
the db file on a bind-mounted volume, proving persistence. Independently
reproduced by a fresh-context review agent, which also rebuilt +
re-verified the whole flow itself.
Not deployed to f3s - this is a local proof of concept alongside the live
Mode A (in-memory guest chat) deployment.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
|
|
ycurses shares no source files with ychat/yhttpd (it's an ncurses UI
toolkit, not part of the httpd/socket engine), so none of the sock/reqp/
html/logd/tool fixes apply here - only the same class of toolchain-gate
bugs did:
- Top-level configure's g++ 3.x version gate now accepts any GNU g++
(same fix as yhttpd/configure).
- scripts/config.pl silently BEGIN-failed on modern Perl (`use
scripts::modules::file` needs "." on @INC, dropped by Perl 5.26+); the
"yes" default answer was never actually read. Fixed with `perl -I.`
(also applied to yhttpd/configure, which had the same latent bug).
- src/configure's library search paths predate 64-bit multilib distros
(no /usr/lib64), so installed libpanel/libmenu/libncurses were reported
"NOT OK" on Rocky Linux 9 (also backported to yhttpd/src/configure).
- attributes.h declared `set<int> set_attr` (std::set, via `using
namespace std`) and separately two member functions literally named
`set` - GCC 11 treats that as ill-formed ("changes meaning of 'set'"),
not just a warning. Renamed both overloads to set_attr_flag; no
external caller used the bare set(...)/set(int) names.
Verified in a Rocky Linux 9 container: builds clean, links, and runs -
initializes curses, draws the demo screen using color/attributes
(exercising the fix above), exits cleanly. Added Dockerfile (build
verification only - it's an interactive demo, not a service) and
BUILD.md documenting the fixes and one pre-existing, deliberately
unfixed bug (unset() never actually clears an attribute).
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
|
|
Ports the ychat revival fixes (unsigned/size_t npos truncation, ofstream
== NULL, tool::trim OOB, CGI popen -> execve, www.yChat.org links, g++
version gate, config.pl -I., NCURSES/CLI-disabled build) so yhttpd builds
on modern GCC in a Rocky Linux 9 container, plus yhttpd-specific fixes
found while verifying it under concurrent load:
- listen() backlog was hardcoded to 1; bumped to SOMAXCONN.
- sock::_close() closed sockets with unread request bytes still in the
kernel receive buffer (read_http() only reads the GET line), so Linux
sent an abortive RST instead of a FIN, racing the client's read of the
response ("connection reset by peer" even though it was delivered).
Fixed with a non-blocking, bounded drain before close() - confirmed via
tcpdump: RSTs on every response before, zero after, across 140+
requests / concurrent bursts of 20.
- Removed a duplicate _make_server_socket() call in start() (wrap.cpp's
init_wrapper() already makes it before start() runs) that leaked a fd
and would have double-initialized SSL if OPENSSL is ever enabled;
caught by fresh-context review, documented honestly in DOCKER.md.
- src/configure's dependency-checker predates 64-bit multilib distros
(only checked /usr/lib, never /usr/lib64) and was missing an ncur
move-aside entry for the NCURSES-disabled build.
Added Dockerfile/.dockerignore/DOCKER.md documenting the build, the fixes,
and the one known-but-unfixed landmine (a SIGILL heap corruption in
sock::_close that reproduces on newer host GCC/glibc but not in the
container - latent, not fixed).
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
|
|
The Options menu (options.html + yc_options) manages the user's email and
password - account settings that only persist with a database. Without one
the menu is non-persistent and misleading (a guest who sets a password can't
log back in after being garbage-collected, since the recycled-user login
checks the password). Make it build-aware (#ifdef DATABASE):
- html/input.html: the Options link is now %%OPTIONS_LINK%%.
- src/reqp.cpp: set map_params[OPTIONS_LINK] to the link (built with the
session tmpid) under #ifdef DATABASE, else empty. So a DB build keeps the
Options link; the no-DB build hides it.
- Dockerfile (no-DB build): drop options.html and yc_options.so from the
image (the popup form and the options module are gone; an on-demand
get_module for the options event fails gracefully -> no-op).
Verified (no-DB): the input frame no longer shows an Options link (Colors,
Help, Users, Logout remain); options.html -> 'Page not found';
yc_options.so absent; normal chat still works.
|
|
The repo holds three legacy C++ subprojects (ychat, yhttpd, ycurses). Add a
root README that explains them, points at ./ychat (the revived/deployed
chat) and its DOCKER.md, and gives a detailed local Docker build/run/access
quickstart plus f3s deploy pointer.
Also fix the now-stale 'HTTP/0.9 responses' note in ychat/DOCKER.md: ychat
emits proper HTTP/1.1 responses since the reqp.cpp header fix.
|
|
The footer 'get it at' link on every served HTML page pointed at the dead
http://www.yChat.org. Point it (href + visible text) at the actual source
repo, https://codeberg.org/snonux/ychat, across all HTML templates, and
update the startup banner CONTACT URL likewise.
|
|
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).
|
|
|
|
|
|
|
|
|
|
|
|
|
