HTTPRequest::parse() passed the parsed Content-Length straight into a
bounds check and a buffer read with no validation. ZString::asInteger()
accumulates digits into a plain 32-bit int with no overflow check, so a
request claiming an absurd Content-Length (confirmed live: a classic
SSTP vulnerability-scanner probe sending "Content-Length:
18446744073709551615") silently overflowed to -1. That negative value
passed the "buffer.getLength() < contentLength" guard for any real
buffer size, then got used as a length in ZString(cursor,
contentLength) - reinterpreted as an enormous size_t, this read far
past the real buffer and corrupted the heap, crashing the process
(sometimes immediately, sometimes much later in an unrelated objects
destructor) every time this scanner traffic reached it.
This single bug was silently crash-looping barant.com (3x in ~30
hours) and the ClockPlanet server (3x over the same window) before
systemds auto-restart each time masked the outage - confirmed via
live core dump analysis (gdb bt full showing double free or
corruption (out) inside MString::~MString, called from
HTTPRequest::parse, with contentLength = -1 in the frame). Fixed by
rejecting a negative Content-Length immediately, before it is ever
used - the same throw-on-malformed-input pattern this function already
uses for a bad request line and unsupported Transfer-Encoding.
Verified live against the exact attack string on both barant.com and
the ClockPlanet server: the connection is now cleanly rejected and the
service stays up with no restart.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
- MString::read() now gives up after repeated I/O errors instead of
looping forever and crashing via realloc() failure (root cause of a
recurring barant.com production crash).
- HTTPRequest::parse() no longer leaks lineParser/headerParser on a
malformed request.
- Log gained size-based rotation (config-driven) and a non-static-mutex
fix; a separate fix stopped rotation from destroying the logs symlink
on its first trigger.
- File.cpp: symlink-preserving rotation support.
- JString: fixed an array-index out-of-bounds and a setString issue.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Was an unchecked std::vector index (return list[index];). Any
malformed/short line - a port scanner, a bot probing the port, or any
client sending fewer colon-delimited fields than a given handler
indexes via request[N] - reached this with no length check anywhere
upstream, across every server built on this class. Depending on build
flags that was either undefined behavior or, confirmed live (a real
crash log from Zephora's game server), a hard process abort via
libstdc++'s own vector bounds assertion:
Assertion '__n < this->size()' failed.
Throwing instead lets this be caught: every caller that dispatches a
parsed request already wraps that dispatch in a try/catch (see
CommandList::processRequest()), so a single malformed line is now
cleanly rejected instead of taking down the entire server process for
every connected player.
Invisible until this session's ServerCore tls/master merge brought
Subscription/SubscriptionManager (which calls it) into the same
dependency chain as a real executable for the first time - a static
library archive (ar) never checks for unresolved symbols, only the
final link of a real binary does, so this went unnoticed for however
long it sat declared-only in MString.h. Stores the raw pointer value
as the string bytes (pointer-identity comparison, matching how void*
aliases are compared everywhere else in this codebase) rather than
dereferencing it, since some callers pass arbitrary non-string
pointers through this constructor.
File-upload parts (Content-Disposition with filename=) were
unconditionally flagged currentSkip and discarded at PART_BOUNDARY,
so every <field type="file"> upload site (track audio, presets,
project icons) only ever received an empty value via $[fieldname],
never the real uploaded bytes.
- File: stat() only after a successful open() (previously ran first with
its result ignored, feeding a missing file's garbage st_size into a
realloc - could crash the whole APIServer on a request for a
nonexistent static asset)
- JString::setString(): always quote+escape the value instead of going
through set()'s literal-value guess, which misclassified any string
starting with digits (e.g. a UUID) as a bare numeric literal and wrote
it unquoted, corrupting later edits to the same JSON document
- MString::remove() (both overloads): fixed an off-by-one that read one
byte past the buffer on a completely full string (found via ASan under
concurrent load)
- CGIFormData: added fields() so callers can iterate every parsed field
without knowing names in advance (used to merge POST body into
jet::Global::variables the same way query-string params already are)
