2.9 KiB
Survey: comsys channel system (comsys.cpp)
Audit of mux/modules/engine/comsys.cpp (5354 lines) — the channel/chat system:
channel data load/save, channel commands (addcom/delcom/create/destroy),
message formatting and broadcast (do_processcom, BuildChannelMessage,
SendChannelMessage, do_cemit). Threat surfaces: the persisted-file
deserializers (same class as flatfile DB #806 / Lua undumper #833) and the
player-facing message path. Methodology matches the parser/JIT/wild campaign.
Result: memory-safe and well-hardened. One minor legacy correctness fix (#841); one noted robustness design choice (mux_assert-abort on malformed file).
Deserializers (load_comsystem_V5/V4/V0123, load_channels_*) — safe
- Channel name/header/title are each clamped to
MAX_CHANNEL_LEN(50)/MAX_HEADER_LEN(100)/MAX_TITLE_LEN(200) beforememcpyinto the correctly sizedchannel::name[51]/header[101]/std::stringtitle. No OOB. GetLineTruncreturns a minimum of 1 (substitutes"\n"on EOF/empty), so the ubiquitousif (temp[nChannel-1] == '\n')is nevertemp[-1]— no underflow OOB read.ReadListOfNumbers(fp, cnt, anum)reads into a fixedbuffer[200]via boundedfgetsand writesanum[0..cnt); every caller sizesanum[] >= cnt(anum[10]with cnt ≤ 10,anum[2]with cnt 2). No fixed-array OOB.MakeCanonicalComAlias/ParseChannelLine: alias clamped toMAX_ALIAS_LEN, nameStringCloned (heap).
#841 (132c197e9): the V0123 channel-name UTF-8 boundary backoff read temp
(the pre-conversion line) instead of pBufferUnicode (the converted bytes
actually copied) — a copy-paste slip vs. the adjacent header path. Not OOB
(in-bounds, count never exceeds source), but could truncate a legacy channel
name mid-character → invalid UTF-8. Fixed to read pBufferUnicode.
Noted (not changed): the loaders use mux_assert(ReadListOfNumbers(...)),
i.e. a controlled abort on a malformed/truncated comsys file. Safe (no UB),
fail-fast — a defensible design choice; a graceful skip-and-log would be friendlier
but is invasive to retrofit across V5/V4/V0123.
Message path — safe
BuildChannelMessagebuildsmessNormal/messNoComtitle(alloc_lbuf) entirely viasafe_str/safe_chr(LBUF-bounded) andmux_exec(..., LBUF_SIZE-1, ...). The eval-comtitle path runs asuser->who(their own code — the documentedeval_comtitlefeature, not a privilege issue).do_processcomcaps the message at 3500 chars; small fixed buffers (chattype[2],sdrBuf[32]) use boundedmux_sprintf(sizeof, …).do_cemitmux_strncpys into an alloc_lbuf (LBUF-bounded) and is permission- gated (Controls/Comm_All). It passes one buffer as bothmsgNormalandmsgNoComtitle, butSendChannelMessageguardsfree_lbuf(msgNoComtitle)withmsgNoComtitle != msgNormal— no double-free.