mmo-ref/tinymux
1
0
Fork 0
mirror of https://github.com/brazilofmux/tinymux synced 2026-08-13 00:23:11 -04:00
Code Issues Projects Releases Packages Wiki Activity

harden: bound two remaining DB-load overflow paths

Browse source 
Two follow-ups to a056f67d0, same malicious/corrupt-database threat model,
found by review of code paths analogous to the ones fixed there. Verified by
full rebuild + smoke suite (1264/1264, 0 crashes).

1. getstring_noalloc legacy path: static buffer overflow (lib/dbutil.cpp)
   a056f67d0 fixed the escaped-string (new_strings=true) branch, but the
   legacy (new_strings=false) continued-line branch advanced `p += nLine`
   across '\r'-terminated lines with no capacity guard against the static
   buf[2*LBUF_SIZE+20]. A crafted legacy string with enough continued lines
   marches p past the buffer and the next fgets writes out of bounds. This
   branch is reachable from v1/v2 flatfile import (db_rw.cpp lock/field reads
   pass new_strings=false). Fix: track nBufferLeft and cap each fgets request
   to the space remaining, mirroring the escaped branch.

2. make_numlist direct-recipient path: stack buffer overflow (modules/engine/mail.cpp)
   a056f67d0 bounded the *alias-expansion copy loop into aRecip[(LBUF_SIZE+1)/2]
   but left the direct-recipient append (aRecip[nRecip++] = target) unbounded.
   Because the malias_read clamp lets a single alias fill aRecip to capacity,
   a crafted mail.db alias followed by additional recipients in the same @mail
   still overflows the stack array. Fix: bound the direct-recipient write with
   the same cap, dropping recipients past it (matching the alias loop) rather
   than overflowing.

Also assessed but left as-is: malias_read's `new malias_t*[getref()]` takes an
unbounded count, but an oversized/overflowing count throws (bad_alloc /
bad_array_new_length), is caught, and the read loop truncates at EOF -- a
bounded memory-DoS with no natural clamp value, not an overwrite.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
Stephen Dennis 2026-07-07 20:31:15 -06:00
parent a056f67d0a
commit 315cfb0786
2 changed files with 20 additions and 4 deletions
Download patch file Download diff file Expand all files Collapse all files

14
mux/lib/dbutil.cpp
View file

@ -310,13 +310,20 @@ void *getstring_noalloc(FILE *f, bool new_strings, size_t *pnBuffer)
ungetc(c, f);
UTF8 *p = buf;
size_t nBufferLeft = sizeof(buf);
for (;;)
{
// Fetch up to and including the next LF.
// Fetch up to and including the next LF. Cap the request at the
// space remaining in buf: a crafted legacy string with enough
// '\r'-continued lines would otherwise march p past the static
// buffer and overflow it on the next fgets.
//
if (fgets(reinterpret_cast<char *>(p), LBUF_SIZE, f) == nullptr)
int nRead = (nBufferLeft < LBUF_SIZE)
? static_cast<int>(nBufferLeft) : LBUF_SIZE;
if ( nRead <= 1
|| fgets(reinterpret_cast<char *>(p), nRead, f) == nullptr)
{
// EOF or ERROR.
// EOF, ERROR, or no room left.
//
p[0] = '\0';
}
@ -332,6 +339,7 @@ void *getstring_noalloc(FILE *f, bool new_strings, size_t *pnBuffer)
// Line is continued on the next line.
//
p += nLine;
nBufferLeft -= nLine;
continue;
}

10
mux/modules/engine/mail.cpp
View file

@ -2324,7 +2324,15 @@ static UTF8 *make_numlist(dbref player, UTF8 *arg, bool bBlind)
target = lookup_player(player, head, true);
if (Good_obj(target))
{
aRecip[nRecip++] = target;
// Bound the write like the alias-copy loop above: aRecip[] is
// sized (LBUF_SIZE+1)/2, and a single clamped malias can fill
// it, so a valid recipient past the cap is dropped rather than
// overflowing the stack array.
//
if (nRecip < static_cast<int>(sizeof(aRecip)/sizeof(aRecip[0])))
{
aRecip[nRecip++] = target;
}
}
else
{