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>
Stage 2 of the APIServer plan. This codebase has only ever sat behind a
CGI gateway - IMFRequest/IMFResponse/IMFMessage predate the
StreamReader/Parser rebuild and are stubs (IMFResponse's header-emission
calls addHeader()/output(), neither of which exist on the class;
IMFRequest is a fragile non-resumable 3-token line split). Nothing in
this repo has ever actually parsed a raw HTTP request off a socket.
HTTPRequest : public Parser fills that gap the same way every other
Parser here is built: REQUEST_LINE -> HEADERS -> BODY -> DONE, chaining
LineParser (request line) and IMFHeader (headers, reused as-is - HTTP
header syntax is IMF-derived, nothing email-specific in IMFHeader
itself) via checkParse() over one shared StreamReader, exactly the
pattern CGIFormData already established for chaining IMFHeader/
IMFBoundary. The BODY state dispatches on Content-Type as CGIFormData
itself does for CGI bodies - application/x-www-form-urlencoded and
multipart/form-data delegate directly to CGIFormData (reused, not
reimplemented), anything else is captured as a raw body (e.g. JSON).
Query string is split off the path before anything else sees it.
getHeader() does a case-insensitive lookup (HTTP header names are
case-insensitive per RFC 7230, unlike IMFHeader's own std::map, which
compares byte-for-byte) since a route handler constantly needs this and
getting it wrong would be a silent framing bug, not a cosmetic one.
The boundary-extraction code mirrors __jet.cpp's own fix exactly (same
cursor-mutation trap: ZString::find() leaves the searched string's
cursor at the match, and CGIFormData's constructor copy-constructs its
own contentType member from whatever's passed in, inheriting that same
moved cursor unless reset() first - would otherwise silently miss its
own startsWith() dispatch checks).
HTTPResponse is a small, new, output-only builder (status line + a real
headers container + body, Content-Length auto-computed unless set
explicitly) - deliberately not patched from IMFResponse, which has
nothing safe to extend.
v1 limitations, flagged not fixed (matching the standard already set by
other Parser subclasses here for known-but-out-of-scope gaps): chunked
Transfer-Encoding isn't supported, and repeated header names last-wins
(inherited from IMFHeader's own std::map).
Verified live via testing/http_test.cpp (34 assertions, new - now wired
into testing/compile): byte-at-a-time GET with a query string and
case-insensitive header lookup; a GET with no query string; a POST with
an urlencoded body deliberately split mid-body (not just mid-header)
across two feed() calls, confirming the CGIFormData delegation resumes
correctly; a raw JSON POST body; testing/http_request.data (a real
fixture, previously unused - normalized from its stored LF line endings
to real CRLF, since HTTP requires CRLF and IMFHeader enforces that
strictly) fed byte-at-a-time; and HTTPResponse::serialize() both with an
auto-computed and an explicitly-set Content-Length (no duplicate
header). Also reran the full existing testing/compile suite
(zstring/mstring/jstring/mfile/jfile) against the rebuilt libCoreUtils.a
to confirm no regression from the two new files.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
