From c373e5f2f7580acbf0b8410c84f0b5f5325ad5b2 Mon Sep 17 00:00:00 2001 From: Mathieu Carbou Date: Wed, 28 Oct 2026 22:19:48 +0100 Subject: [PATCH 1/3] Fix request processing failures caused by failed allocations Abort the request instead of crashing, corrupting the wire format or silently mis-parsing it when a heap allocation fails. Arduino String leaves a failed string empty, and its c_str() is null on ESP32. Sites that copied header or body data and then passed c_str() to C string functions could crash (issue #500), and sites that stored the copy in request state silently lost or corrupted data. Header parsing: - Add AsyncWebHeader::tryParse() reporting ParseStatus::NoMemory so a failed name/value copy fails the request instead of dropping the header (a lost Content-Length desynchronizes the body parser). - Scan Accept and Transfer-Encoding in place: the lowercased copies are gone, so issue #500 cannot happen and chunked framing is no longer silently missed. - Abort on failed copies into _host, _contentType, _authorization and on failed header-line/body accumulation in the plain and multipart parsers. - Write the 100-continue reply from the literal: the String copy exceeded the small-buffer size and an OOM one silently stalled the client. Responses: - Check every concat() in _assembleHead(); callers now close the connection instead of writing a truncated head (basic, abstract, WebSocket and SSE responses). - AsyncWebSocketResponse flags RESPONSE_FAILED when it cannot build Sec-WebSocket-Accept, and _sourceValid() reflects it, so the upgrade is refused rather than completing with a garbage accept header. Allocation helpers: - Use std::nothrow for response objects created by beginResponse*(), AsyncStaticWebHandler and AsyncEventSource so failures are observable instead of aborting the device; send()/_send() handle a null response. - Replace plain new with malloc in generateBasicHash() and the WebSocket printf helpers, where the null checks were dead code. Static file handler: - Stop and fall through to 404 when the path cannot be built, rather than serving the default or a wrong file. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- src/AsyncEventSource.cpp | 16 +- src/AsyncWebHeader.cpp | 37 ++- src/AsyncWebSocket.cpp | 50 +++- src/AsyncWebSocket.h | 4 +- src/ESPAsyncWebServer.h | 28 +- src/WebAuthentication.cpp | 19 +- src/WebHandlers.cpp | 40 ++- src/WebRequest.cpp | 607 ++++++++++++++++++++++++-------------- src/WebResponses.cpp | 83 +++++- 9 files changed, 611 insertions(+), 273 deletions(-) diff --git a/src/AsyncEventSource.cpp b/src/AsyncEventSource.cpp index 797cf71a8..30ba1c5d7 100644 --- a/src/AsyncEventSource.cpp +++ b/src/AsyncEventSource.cpp @@ -386,7 +386,13 @@ bool AsyncEventSource::canHandle(AsyncWebServerRequest *request) const { } void AsyncEventSource::handleRequest(AsyncWebServerRequest *request) { - request->send(new AsyncEventSourceResponse(this)); + AsyncWebServerResponse *response = new (std::nothrow) AsyncEventSourceResponse(this); + if (!response) { + async_ws_log_e("Failed to allocate"); + request->abort(); + return; + } + request->send(response); } // list iteration protected by caller's lock @@ -416,6 +422,14 @@ AsyncEventSourceResponse::AsyncEventSourceResponse(AsyncEventSource *server) : _ void AsyncEventSourceResponse::_respond(AsyncWebServerRequest *request) { String out; _assembleHead(out, request->version()); + if (!_headLength) { + // Head assembly ran out of memory: fail the stream instead of sending a + // corrupted head. + async_ws_log_e("Failed to allocate"); + _state = RESPONSE_FAILED; + request->client()->close(); + return; + } uint32_t lastId = 0; if (request->hasHeader(T_Last_Event_ID)) { lastId = strtoul(request->getHeader(T_Last_Event_ID)->value().c_str(), nullptr, 10); diff --git a/src/AsyncWebHeader.cpp b/src/AsyncWebHeader.cpp index 04041fbf6..c9fce64d5 100644 --- a/src/AsyncWebHeader.cpp +++ b/src/AsyncWebHeader.cpp @@ -3,24 +3,33 @@ #include +#include + const AsyncWebHeader AsyncWebHeader::parse(const char *data) { + AsyncWebHeader header; + // Both a malformed line and an allocation failure yield an empty (invalid) header + (void)tryParse(data, header); + return header; +} + +AsyncWebHeader::ParseStatus AsyncWebHeader::tryParse(const char *data, AsyncWebHeader &out) { // https://developer.mozilla.org/en-US/docs/Web/HTTP/Reference/Headers // In HTTP/1.X, a header is a case-insensitive name followed by a colon, then optional whitespace which will be ignored, and finally by its value if (!data) { - return AsyncWebHeader(); // nullptr + return ParseStatus::Invalid; // nullptr } if (data[0] == '\0') { - return AsyncWebHeader(); // empty string + return ParseStatus::Invalid; // empty string } if (strchr(data, '\n') || strchr(data, '\r')) { - return AsyncWebHeader(); // Invalid header format + return ParseStatus::Invalid; // Invalid header format } const char *colon = strchr(data, ':'); if (!colon) { - return AsyncWebHeader(); // separator not found + return ParseStatus::Invalid; // separator not found } if (colon == data) { - return AsyncWebHeader(); // Header name cannot be empty + return ParseStatus::Invalid; // Header name cannot be empty } const char *startOfValue = colon + 1; // Skip the colon // RFC 7230 §3.2.3: OWS (optional whitespace) = *( SP / HTAB ) @@ -29,7 +38,19 @@ const AsyncWebHeader AsyncWebHeader::parse(const char *data) { startOfValue++; } String name; - name.reserve(colon - data); - name.concat(data, colon - data); - return AsyncWebHeader(name, String(startOfValue)); + if (!name.reserve(colon - data) || !name.concat(data, colon - data)) { + return ParseStatus::NoMemory; + } + String value(startOfValue); + // A copy of a non-empty value can never be empty: Arduino String leaves + // length() == 0 only when the heap buffer could not be allocated (its + // c_str() is then null on ESP32, which downstream header consumers such as + // atoi() and strncasecmp() would dereference). Reject the header so the + // caller can fail the request. + if (*startOfValue && !value.length()) { + return ParseStatus::NoMemory; + } + out._name = std::move(name); + out._value = std::move(value); + return ParseStatus::Ok; } diff --git a/src/AsyncWebSocket.cpp b/src/AsyncWebSocket.cpp index 8c159bc02..badd62bf9 100644 --- a/src/AsyncWebSocket.cpp +++ b/src/AsyncWebSocket.cpp @@ -859,9 +859,13 @@ size_t AsyncWebSocketClient::printf(const char *format, ...) { return 0; } - char *buffer = new char[len + 1]; + // malloc (not new) so an allocation failure is observable: plain new aborts + // the device on ESP32/ESP8266 when exceptions are disabled, leaving the + // NULL-check below dead code. + char *buffer = (char *)malloc(len + 1); if (!buffer) { + async_ws_log_e("Failed to allocate"); return 0; } @@ -870,7 +874,7 @@ size_t AsyncWebSocketClient::printf(const char *format, ...) { va_end(arg); bool enqueued = text(buffer, len); - delete[] buffer; + free(buffer); return enqueued ? len : 0; } @@ -885,9 +889,10 @@ size_t AsyncWebSocketClient::printf_P(PGM_P formatP, ...) { return 0; } - char *buffer = new char[len + 1]; + char *buffer = (char *)malloc(len + 1); if (!buffer) { + async_ws_log_e("Failed to allocate"); return 0; } @@ -896,7 +901,7 @@ size_t AsyncWebSocketClient::printf_P(PGM_P formatP, ...) { va_end(arg); bool enqueued = text(buffer, len); - delete[] buffer; + free(buffer); return enqueued ? len : 0; } #endif @@ -1368,9 +1373,13 @@ size_t AsyncWebSocket::printfAll(const char *format, ...) { return 0; } - char *buffer = new char[len + 1]; + // malloc (not new) so an allocation failure is observable: plain new aborts + // the device on ESP32/ESP8266 when exceptions are disabled, leaving the + // NULL-check below dead code. + char *buffer = (char *)malloc(len + 1); if (!buffer) { + async_ws_log_e("Failed to allocate"); return 0; } @@ -1379,7 +1388,7 @@ size_t AsyncWebSocket::printfAll(const char *format, ...) { va_end(arg); AsyncWebSocket::SendStatus status = textAll(buffer, len); - delete[] buffer; + free(buffer); return status == DISCARDED ? 0 : len; } @@ -1406,9 +1415,10 @@ size_t AsyncWebSocket::printfAll_P(PGM_P formatP, ...) { return 0; } - char *buffer = new char[len + 1]; + char *buffer = (char *)malloc(len + 1); if (!buffer) { + async_ws_log_e("Failed to allocate"); return 0; } @@ -1417,7 +1427,7 @@ size_t AsyncWebSocket::printfAll_P(PGM_P formatP, ...) { va_end(arg); AsyncWebSocket::SendStatus status = textAll(buffer, len); - delete[] buffer; + free(buffer); return status == DISCARDED ? 0 : len; } #endif @@ -1462,17 +1472,30 @@ void AsyncWebSocket::handleRequest(AsyncWebServerRequest *request) { const AsyncWebHeader *version = request->getHeader(WS_STR_VERSION); if (version->value().toInt() != 13) { AsyncWebServerResponse *response = request->beginResponse(400); + if (!response) { + async_ws_log_e("Failed to allocate"); + request->abort(); + return; + } response->addHeader(WS_STR_VERSION, T_13); request->send(response); return; } const AsyncWebHeader *key = request->getHeader(WS_STR_KEY); - AsyncWebServerResponse *response = new AsyncWebSocketResponse(key->value(), this); + AsyncWebServerResponse *response = new (std::nothrow) AsyncWebSocketResponse(key->value(), this); if (response == NULL) { async_ws_log_e("Failed to allocate"); request->abort(); return; } + if (!response->_sourceValid()) { + // The response could not build the Sec-WebSocket-Accept header (out of + // memory): fail the upgrade instead of sending an invalid handshake. + async_ws_log_e("Failed to allocate"); + delete response; + request->abort(); + return; + } if (request->hasHeader(WS_STR_PROTOCOL)) { const AsyncWebHeader *protocol = request->getHeader(WS_STR_PROTOCOL); // ToDo: check protocol @@ -1507,6 +1530,7 @@ AsyncWebSocketResponse::AsyncWebSocketResponse(const String &key, AsyncWebSocket String k; if (!k.reserve(key.length() + WS_STR_UUID_LEN)) { async_ws_log_e("Failed to allocate"); + _state = RESPONSE_FAILED; return; } k.concat(key); @@ -1554,6 +1578,14 @@ void AsyncWebSocketResponse::_respond(AsyncWebServerRequest *request) { ); String out; _assembleHead(out, request->version()); + if (!_headLength) { + // Head assembly ran out of memory: fail the upgrade instead of sending a + // corrupted handshake. + async_ws_log_e("Failed to allocate"); + _state = RESPONSE_FAILED; + request->client()->close(); + return; + } request->client()->write(out.c_str(), _headLength); _state = RESPONSE_WAIT_ACK; } diff --git a/src/AsyncWebSocket.h b/src/AsyncWebSocket.h index 07420b2c5..6224b8779 100644 --- a/src/AsyncWebSocket.h +++ b/src/AsyncWebSocket.h @@ -468,7 +468,9 @@ class AsyncWebSocketResponse : public AsyncWebServerResponse { return 0; }; bool _sourceValid() const override { - return true; + // _state is RESPONSE_FAILED when the Sec-WebSocket-Accept computation + // could not allocate (see the constructor). + return _state != RESPONSE_FAILED; } }; diff --git a/src/ESPAsyncWebServer.h b/src/ESPAsyncWebServer.h index 9e616eba9..85ede40ed 100644 --- a/src/ESPAsyncWebServer.h +++ b/src/ESPAsyncWebServer.h @@ -488,8 +488,33 @@ class AsyncWebHeader { return _name.length(); } + // Result of AsyncWebHeader::tryParse() + enum class ParseStatus { + Ok, // header parsed, output valid + Invalid, // malformed header line + NoMemory, // allocation failure: request must be failed, not skipped + }; + + /** + * @brief Parse a header line into an existing AsyncWebHeader. + * + * Unlike parse(), the output parameter is only overwritten on success, so a + * NoMemory result cannot leave a half-built header behind. A NoMemory + * result must abort the request (see issue #500): a String whose internal + * allocation failed has a null c_str() on ESP32 and an empty one on + * ESP8266, and downstream header consumers would either dereference it or + * silently misparse the request. + * + * @param data raw header line "Name: value" (no CR/LF) + * @param out header to fill on success; left untouched otherwise + * @return ParseStatus Ok, Invalid or NoMemory + */ + static ParseStatus tryParse(const char *data, AsyncWebHeader &out); + static const AsyncWebHeader parse(const String &data) { - return parse(data.c_str()); + AsyncWebHeader header; + (void)tryParse(data.c_str(), header); + return header; } static const AsyncWebHeader parse(const char *data); }; @@ -595,6 +620,7 @@ class AsyncWebServerRequest { bool _parseReqHead(); bool _parseReqHeader(); + void _parseHeader(AsyncWebHeader &&header); void _parseLine(); void _parsePlainPostChar(uint8_t data); void _parseMultipartPostByte(uint8_t data, bool last); diff --git a/src/WebAuthentication.cpp b/src/WebAuthentication.cpp index a7805dc8b..e589f9fc3 100644 --- a/src/WebAuthentication.cpp +++ b/src/WebAuthentication.cpp @@ -31,24 +31,29 @@ String generateBasicHash(const char *username, const char *password) { size_t toencodeLen = strlen(username) + strlen(password) + 1; - char *toencode = new char[toencodeLen + 1]; + // malloc (not new) so an allocation failure is observable: plain new + // aborts the device on ESP32/ESP8266 when exceptions are disabled, leaving + // the NULL-check below dead code. + char *toencode = (char *)malloc(toencodeLen + 1); if (toencode == NULL) { + async_ws_log_e("Failed to allocate"); return asyncsrv::emptyString; } - char *encoded = new char[base64_encode_expected_len(toencodeLen) + 1]; + char *encoded = (char *)malloc(base64_encode_expected_len(toencodeLen) + 1); if (encoded == NULL) { - delete[] toencode; + async_ws_log_e("Failed to allocate"); + free(toencode); return asyncsrv::emptyString; } sprintf_P(toencode, PSTR("%s:%s"), username, password); if (base64_encode_chars(toencode, toencodeLen, encoded) > 0) { String res = String(encoded); - delete[] toencode; - delete[] encoded; + free(toencode); + free(encoded); return res; } - delete[] toencode; - delete[] encoded; + free(toencode); + free(encoded); return asyncsrv::emptyString; } diff --git a/src/WebHandlers.cpp b/src/WebHandlers.cpp index 5a0977b2e..409f22f6f 100644 --- a/src/WebHandlers.cpp +++ b/src/WebHandlers.cpp @@ -109,11 +109,26 @@ bool AsyncStaticWebHandler::canHandle(AsyncWebServerRequest *request) const { bool AsyncStaticWebHandler::_getFile(AsyncWebServerRequest *request) const { // Remove the found uri String path = request->url().substring(_uri.length()); + // substring() can fail to allocate: an empty result for a longer URL means + // the heap buffer could not be allocated, and continuing would look up the + // wrong file (or the default one). Bail out so the request falls through + // to another handler / 404 instead of serving wrong content. + if (!path.length() && request->url().length() > _uri.length()) { + async_ws_log_e("Failed to allocate"); + return false; + } // We can skip the file check and look for default if request is to the root of a directory or that request path ends with '/' bool canSkipFileCheck = (_isDir && path.length() == 0) || (path.length() && path[path.length() - 1] == '/'); path = _path + path; + // The concatenation above can fail to allocate: _path is never empty (the + // constructor guarantees a leading '/'), so an empty result can only mean + // the heap buffer could not be allocated. + if (!path.length()) { + async_ws_log_e("Failed to allocate"); + return false; + } // Do we have a file or .gz file if (!canSkipFileCheck && const_cast(this)->_searchFile(request, path)) { @@ -200,9 +215,22 @@ bool AsyncStaticWebHandler::_searchFile(AsyncWebServerRequest *request, const St */ void AsyncStaticWebHandler::handleRequest(AsyncWebServerRequest *request) { // Get the filename from request->_tempObject and free it - String filename((char *)request->_tempObject); - free(request->_tempObject); - request->_tempObject = nullptr; + String filename; + if (request->_tempObject != nullptr) { + const char *tempPath = (const char *)request->_tempObject; + const size_t tempPathLen = strlen(tempPath); + filename = String(tempPath); + free(request->_tempObject); + request->_tempObject = nullptr; + // A copy of a non-empty path can only be empty when its allocation + // failed (c_str() is then null on ESP32); abort rather than serve the + // wrong file. + if (tempPathLen && !filename.length()) { + async_ws_log_e("Failed to allocate"); + request->abort(); + return; + } + } if (request->_tempFile != true) { request->send(404); @@ -258,9 +286,11 @@ void AsyncStaticWebHandler::handleRequest(AsyncWebServerRequest *request) { if (notModified) { request->_tempFile.close(); - response = new AsyncBasicResponse(304); // Not modified + // nothrow: plain new aborts the device on ESP32/ESP8266 when out of + // memory, making a NULL-check dead code. + response = new (std::nothrow) AsyncBasicResponse(304); // Not modified } else { - response = new AsyncFileResponse(request->_tempFile, filename, asyncsrv::emptyString, false, _callback); + response = new (std::nothrow) AsyncFileResponse(request->_tempFile, filename, asyncsrv::emptyString, false, _callback); } if (!response) { diff --git a/src/WebRequest.cpp b/src/WebRequest.cpp index 15e3692ad..033a9082d 100644 --- a/src/WebRequest.cpp +++ b/src/WebRequest.cpp @@ -163,7 +163,14 @@ void AsyncWebServerRequest::_onData(void *buf, size_t len) { _temp.concat(ch); } else { // Found new line - extract it and parse str[i] = 0; // Terminate the string at the end of the line. - _temp.concat(str); + // concat() can fail to allocate; a failed _temp would silently + // truncate the header line and desync the parser, so abort instead. + if (!_temp.concat(str)) { + async_ws_log_e("Failed to allocate"); + _parseState = PARSE_REQ_FAIL; + abort(); + return; + } _temp.trim(); _parseLine(); if (++i < len) { @@ -191,6 +198,11 @@ void AsyncWebServerRequest::_onData(void *buf, size_t len) { if (needParse) { size_t idx; for (idx = 0; idx < len; idx++) { + if (_parseState != PARSE_REQ_BODY) { + // the parser aborted this request (e.g. out of memory): stop + // processing the remaining bytes + break; + } _parseMultipartPostByte(((uint8_t *)buf)[idx], idx == len - 1); _parsedLength++; } @@ -221,6 +233,11 @@ void AsyncWebServerRequest::_onData(void *buf, size_t len) { } else if (needParse) { size_t idx; for (idx = 0; idx < len; idx++) { + if (_parseState != PARSE_REQ_BODY) { + // the parser aborted this request (e.g. out of memory): stop + // processing the remaining bytes + break; + } _parsedLength++; _parsePlainPostChar(((uint8_t *)buf)[idx]); } @@ -487,246 +504,335 @@ static inline const char *rtrim_ows(const char *start, const char *end) { } bool AsyncWebServerRequest::_parseReqHeader() { - AsyncWebHeader header = AsyncWebHeader::parse(_temp); - if (header) { - const String &name = header.name(); - const String &value = header.value(); - if (name.equalsIgnoreCase(T_Host)) { - _host = value; - } else if (name.equalsIgnoreCase(T_Content_Type)) { - _contentType = value.substring(0, value.indexOf(';')); - // Trim _contentType defensively; AsyncWebHeader::parse now strips all - // leading OWS per RFC 7230, but trim() guards against any future change. - _contentType.trim(); - if (strncasecmp(value.c_str(), T_MULTIPART_, sizeof(T_MULTIPART_) - 1) == 0) { - // Delimiter-structured, quote-aware search for the boundary parameter. - // We scan parameter by parameter (';'-delimited) so that 'boundary=' - // inside a quoted value (e.g. foo="x; boundary=y") is never mistaken - // for the real parameter. A ';' inside a quoted-string is not a - // parameter separator per RFC 2045 §5.1. - const char *p = value.c_str(); - const char *end = p + value.length(); - - // Advance past the media-type to the first ';' - while (p < end && *p != ';') { - p++; - } + AsyncWebHeader header; + switch (AsyncWebHeader::tryParse(_temp.c_str(), header)) { + case AsyncWebHeader::ParseStatus::NoMemory: + // Allocation failure while parsing a header must abort the request: + // a failed String copy has a null c_str() on ESP32, and a silent skip + // would misparse the request (e.g. a lost Content-Length or an + // undetected chunked framing). See issue #500. + async_ws_log_e("Failed to allocate while parsing header"); + _parseState = PARSE_REQ_FAIL; + abort(); + return true; + case AsyncWebHeader::ParseStatus::Invalid: break; // ignore malformed header line + case AsyncWebHeader::ParseStatus::Ok: _parseHeader(std::move(header)); break; + } +#if defined(TARGET_RP2040) || defined(TARGET_RP2350) || defined(PICO_RP2040) || defined(PICO_RP2350) || defined(LIBRETINY) || defined(HOST) + // ArduinoCore-API does not have String::clear() method 8-() + _temp = asyncsrv::emptyString; +#else + _temp.clear(); +#endif + return true; +} +// Process one successfully parsed header line. Any allocation failure +// detected here aborts the request (see _parseReqHeader() and issue #500): +// a String whose allocation failed is empty with a null c_str() on ESP32, +// and continuing would misparse or crash. +void AsyncWebServerRequest::_parseHeader(AsyncWebHeader &&header) { + const String &name = header.name(); + const String &value = header.value(); + // The copies below can fail to allocate; a member field left empty (its + // c_str() is then null on ESP32) would mislead handlers, so abort instead. + if (name.equalsIgnoreCase(T_Host)) { + _host = value; + if (value.length() && !_host.length()) { + async_ws_log_e("Failed to allocate"); + _parseState = PARSE_REQ_FAIL; + abort(); + return; + } + } else if (name.equalsIgnoreCase(T_Content_Type)) { + _contentType = value.substring(0, value.indexOf(';')); + // The media type slice is non-empty when the value does not start with + // ';': an empty result then means the allocation failed. + if (value.length() && value[0] != ';' && !_contentType.length()) { + async_ws_log_e("Failed to allocate"); + _parseState = PARSE_REQ_FAIL; + abort(); + return; + } + // Trim _contentType defensively; AsyncWebHeader::tryParse now strips all + // leading OWS per RFC 7230, but trim() guards against any future change. + _contentType.trim(); + if (strncasecmp(value.c_str(), T_MULTIPART_, sizeof(T_MULTIPART_) - 1) == 0) { + // Delimiter-structured, quote-aware search for the boundary parameter. + // We scan parameter by parameter (';'-delimited) so that 'boundary=' + // inside a quoted value (e.g. foo="x; boundary=y") is never mistaken + // for the real parameter. A ';' inside a quoted-string is not a + // parameter separator per RFC 2045 §5.1. + const char *p = value.c_str(); + const char *end = p + value.length(); + + // Advance past the media-type to the first ';' + while (p < end && *p != ';') { + p++; + } - const char *bvdata = nullptr; - while (p < end) { - p++; // consume ';' - p = skip_ows(p, end); - if ((size_t)(end - p) > T_BOUNDARY_LEN && strncasecmp(p, T_BOUNDARY, T_BOUNDARY_LEN) == 0) { - bvdata = p + T_BOUNDARY_LEN; - break; - } - // Advance to the next ';', skipping over any quoted-string value - while (p < end && *p != ';') { - if (*p++ == '"') { - while (p < end && *p != '"') { - if (*p == '\\' && p + 1 < end) { - p++; // skip quoted-pair - } - p++; - } - if (p < end) { - p++; // skip closing '"' + const char *bvdata = nullptr; + while (p < end) { + p++; // consume ';' + p = skip_ows(p, end); + if ((size_t)(end - p) > T_BOUNDARY_LEN && strncasecmp(p, T_BOUNDARY, T_BOUNDARY_LEN) == 0) { + bvdata = p + T_BOUNDARY_LEN; + break; + } + // Advance to the next ';', skipping over any quoted-string value + while (p < end && *p != ';') { + if (*p++ == '"') { + while (p < end && *p != '"') { + if (*p == '\\' && p + 1 < end) { + p++; // skip quoted-pair } + p++; + } + if (p < end) { + p++; // skip closing '"' } } } + } - if (bvdata == nullptr) { - // No valid boundary parameter found — cannot parse multipart body. - async_ws_log_d("Missing multipart boundary parameter, aborting"); - _parseState = PARSE_REQ_FAIL; - abort(); - return true; - } + if (bvdata == nullptr) { + // No valid boundary parameter found — cannot parse multipart body. + async_ws_log_d("Missing multipart boundary parameter, aborting"); + _parseState = PARSE_REQ_FAIL; + abort(); + return; + } - // Strip leading OWS after 'boundary=' — non-RFC but seen in the wild. - bvdata = skip_ows(bvdata, end); + // Strip leading OWS after 'boundary=' — non-RFC but seen in the wild. + bvdata = skip_ows(bvdata, end); - // Clear any previous value — duplicate Content-Type headers must not - // concatenate into the boundary string. - _boundary = String(); + // Clear any previous value — duplicate Content-Type headers must not + // concatenate into the boundary string. + _boundary = String(); - if (bvdata < end && *bvdata == '"') { - // Quoted-string: unescape quoted-pairs into _boundary directly. - bvdata++; // skip opening '"' - // Reserve at most 70 chars — the RFC 2046 §5.1 maximum — rather than - // the full (attacker-controlled) remaining suffix length. - if (!_boundary.reserve(70)) { - async_ws_log_e("Failed to allocate"); - _parseState = PARSE_REQ_FAIL; - abort(); - return true; - } - bool closed = false; - while (bvdata < end) { - char c = *bvdata++; - if (c == '\\' && bvdata < end) { - _boundary += *bvdata++; // quoted-pair: keep only the escaped char - } else if (c == '"') { - closed = true; - break; - } else { - _boundary += c; - } - // CWE-190 / DoS fix: RFC 2046 §5.1 limits boundary strings to 70 characters. - if (_boundary.length() > 70) { - async_ws_log_d("Invalid multipart boundary length (%u), aborting", _boundary.length()); - _parseState = PARSE_REQ_FAIL; - abort(); - return true; - } - } - if (!closed) { - // Opening quote was never closed — malformed header. - async_ws_log_d("Invalid multipart boundary (unterminated quote), aborting"); - _parseState = PARSE_REQ_FAIL; - abort(); - return true; - } - } else { - // Token form: value ends at the next ';' or end of string. - const char *tok_end = bvdata; - while (tok_end < end && *tok_end != ';') { - tok_end++; + if (bvdata < end && *bvdata == '"') { + // Quoted-string: unescape quoted-pairs into _boundary directly. + bvdata++; // skip opening '"' + // Reserve at most 70 chars — the RFC 2046 §5.1 maximum — rather than + // the full (attacker-controlled) remaining suffix length. + if (!_boundary.reserve(70)) { + async_ws_log_e("Failed to allocate"); + _parseState = PARSE_REQ_FAIL; + abort(); + return; + } + bool closed = false; + while (bvdata < end) { + char c = *bvdata++; + if (c == '\\' && bvdata < end) { + _boundary += *bvdata++; // quoted-pair: keep only the escaped char + } else if (c == '"') { + closed = true; + break; + } else { + _boundary += c; } - tok_end = rtrim_ows(bvdata, tok_end); - size_t bvlen = (size_t)(tok_end - bvdata); // CWE-190 / DoS fix: RFC 2046 §5.1 limits boundary strings to 70 characters. - if (bvlen == 0 || bvlen > 70) { - async_ws_log_d("Invalid multipart boundary length (%u), aborting", (unsigned)bvlen); + if (_boundary.length() > 70) { + async_ws_log_d("Invalid multipart boundary length (%u), aborting", _boundary.length()); _parseState = PARSE_REQ_FAIL; abort(); - return true; - } -#ifdef ESP8266 - { - // ESP8266 Arduino String lacks String(const char*, size_t). - // bvlen <= 70 is guaranteed above, so a stack buffer is safe. - char buf[71]; - memcpy(buf, bvdata, bvlen); - buf[bvlen] = '\0'; - _boundary = String(buf); + return; } -#else - _boundary = String(bvdata, (unsigned int)bvlen); -#endif } - - // CWE-190 / DoS fix: final safety net. The per-branch checks above - // cover the token path (bvlen == 0 || bvlen > 70) and the quoted-string - // path (> 70 inside the loop), but an empty quoted-string (boundary="") - // closes immediately with _boundary still empty and is not rejected - // by either branch. Reject it here before _isMultipart is set, so an - // empty boundary never reaches the multipart body parser (where it - // would cause --\r\n to be accepted as a delimiter). - if (_boundary.length() == 0 || _boundary.length() > 70) { - async_ws_log_d("Invalid multipart boundary length (%u), aborting", _boundary.length()); + if (!closed) { + // Opening quote was never closed — malformed header. + async_ws_log_d("Invalid multipart boundary (unterminated quote), aborting"); _parseState = PARSE_REQ_FAIL; abort(); - return true; + return; + } + } else { + // Token form: value ends at the next ';' or end of string. + const char *tok_end = bvdata; + while (tok_end < end && *tok_end != ';') { + tok_end++; + } + tok_end = rtrim_ows(bvdata, tok_end); + size_t bvlen = (size_t)(tok_end - bvdata); + // CWE-190 / DoS fix: RFC 2046 §5.1 limits boundary strings to 70 characters. + if (bvlen == 0 || bvlen > 70) { + async_ws_log_d("Invalid multipart boundary length (%u), aborting", (unsigned)bvlen); + _parseState = PARSE_REQ_FAIL; + abort(); + return; } +#ifdef ESP8266 + { + // ESP8266 Arduino String lacks String(const char*, size_t). + // bvlen <= 70 is guaranteed above, so a stack buffer is safe. + char buf[71]; + memcpy(buf, bvdata, bvlen); + buf[bvlen] = '\0'; + _boundary = String(buf); + } +#else + _boundary = String(bvdata, (unsigned int)bvlen); +#endif + } - _isMultipart = true; + // CWE-190 / DoS fix: final safety net. The per-branch checks above + // cover the token path (bvlen == 0 || bvlen > 70) and the quoted-string + // path (> 70 inside the loop), but an empty quoted-string (boundary="") + // closes immediately with _boundary still empty and is not rejected + // by either branch. Reject it here before _isMultipart is set, so an + // empty boundary never reaches the multipart body parser (where it + // would cause --\r\n to be accepted as a delimiter). + if (_boundary.length() == 0 || _boundary.length() > 70) { + async_ws_log_d("Invalid multipart boundary length (%u), aborting", _boundary.length()); + _parseState = PARSE_REQ_FAIL; + abort(); + return; } - } else if (name.equalsIgnoreCase(T_Content_Length) || name.equalsIgnoreCase(T_X_Expected_Entity_Length)) { - // MacOS WebDAVFS uses X-Expected-Entity-Length to indicate the - // total length of a chunked request body. It is useful to - // determine if a PUT can possibly fit in the available space. - _contentLength = atoi(value.c_str()); - } else if (name.equalsIgnoreCase(T_EXPECT) && value.equalsIgnoreCase(T_100_CONTINUE)) { - _expectingContinue = true; - } else if (name.equalsIgnoreCase(T_AUTH)) { - int space = value.indexOf(' '); - if (space == -1) { - _authorization = value; - _authMethod = AsyncAuthType::AUTH_OTHER; + + _isMultipart = true; + } + } else if (name.equalsIgnoreCase(T_Content_Length) || name.equalsIgnoreCase(T_X_Expected_Entity_Length)) { + // MacOS WebDAVFS uses X-Expected-Entity-Length to indicate the + // total length of a chunked request body. It is useful to + // determine if a PUT can possibly fit in the available space. + _contentLength = atoi(value.c_str()); + } else if (name.equalsIgnoreCase(T_EXPECT) && value.equalsIgnoreCase(T_100_CONTINUE)) { + _expectingContinue = true; + } else if (name.equalsIgnoreCase(T_AUTH)) { + int space = value.indexOf(' '); + if (space == -1) { + _authorization = value; + // A copy of a non-empty value can only be empty when its allocation + // failed; abort rather than authenticate against a corrupt token. + if (value.length() && !_authorization.length()) { + async_ws_log_e("Failed to allocate"); + _parseState = PARSE_REQ_FAIL; + abort(); + return; + } + _authMethod = AsyncAuthType::AUTH_OTHER; + } else { + String method = value.substring(0, space); + if (space && !method.length()) { + async_ws_log_e("Failed to allocate"); + _parseState = PARSE_REQ_FAIL; + abort(); + return; + } + if (method.equalsIgnoreCase(T_BASIC)) { + _authMethod = AsyncAuthType::AUTH_BASIC; + } else if (method.equalsIgnoreCase(T_DIGEST)) { + _authMethod = AsyncAuthType::AUTH_DIGEST; + } else if (method.equalsIgnoreCase(T_BEARER)) { + _authMethod = AsyncAuthType::AUTH_BEARER; } else { - String method = value.substring(0, space); - if (method.equalsIgnoreCase(T_BASIC)) { - _authMethod = AsyncAuthType::AUTH_BASIC; - } else if (method.equalsIgnoreCase(T_DIGEST)) { - _authMethod = AsyncAuthType::AUTH_DIGEST; - } else if (method.equalsIgnoreCase(T_BEARER)) { - _authMethod = AsyncAuthType::AUTH_BEARER; - } else { - _authMethod = AsyncAuthType::AUTH_OTHER; - } - _authorization = value.substring(space + 1); + _authMethod = AsyncAuthType::AUTH_OTHER; } - } else if (name.equalsIgnoreCase(T_UPGRADE) && value.equalsIgnoreCase(T_WS)) { - // WebSocket request can be uniquely identified by header: [Upgrade: websocket] - // Per RFC 6455 §4.1 the handshake is a GET. Only classify when the - // connection is still a plain HTTP connection so a previously detected - // SSE request (or any other classified type) cannot be clobbered by - // header ordering. - if (_method == AsyncWebRequestMethod::HTTP_GET && _reqconntype == RCT_HTTP) { - _reqconntype = RCT_WS; + _authorization = value.substring(space + 1); + // The credentials slice is non-empty when the value has characters + // after the space: an empty result then means the allocation failed. + if (value.length() > (size_t)(space + 1) && !_authorization.length()) { + async_ws_log_e("Failed to allocate"); + _parseState = PARSE_REQ_FAIL; + abort(); + return; } - } else if (name.equalsIgnoreCase(T_ACCEPT)) { - String lowcase(value); - lowcase.toLowerCase(); -#ifndef ESP8266 - const char *substr = std::strstr(lowcase.c_str(), T_text_event_stream); -#else - const char *substr = std::strstr(lowcase.c_str(), String(T_text_event_stream).c_str()); -#endif - // Server-Sent Events (HTML §9.2) are GET-only connections negotiated via - // Accept: text/event-stream. Only classify when the connection is still - // a plain HTTP connection so a previously detected WebSocket upgrade - // cannot be clobbered by header ordering. - if (substr != NULL && _method == AsyncWebRequestMethod::HTTP_GET && _reqconntype == RCT_HTTP) { - // WebEvent request can be uniquely identified by header: [Accept: text/event-stream] - _reqconntype = RCT_EVENT; + } + } else if (name.equalsIgnoreCase(T_UPGRADE) && value.equalsIgnoreCase(T_WS)) { + // WebSocket request can be uniquely identified by header: [Upgrade: websocket] + // Per RFC 6455 §4.1 the handshake is a GET. Only classify when the + // connection is still a plain HTTP connection so a previously detected + // SSE request (or any other classified type) cannot be clobbered by + // header ordering. + if (_method == AsyncWebRequestMethod::HTTP_GET && _reqconntype == RCT_HTTP) { + _reqconntype = RCT_WS; + } + } else if (name.equalsIgnoreCase(T_ACCEPT)) { + // Scan the value in place instead of allocating a lowercased copy: a copy + // that fails to allocate is empty with a null c_str() on ESP32, and + // strstr() on it crashed the device (issue #500). value.c_str() is valid + // here because tryParse() rejects OOM copies before we get here. + const char *v = value.c_str(); + const size_t vlen = value.length(); + const size_t nlen = sizeof(T_text_event_stream) - 1; + bool isSSE = false; + if (vlen >= nlen) { + for (size_t i = 0; i <= vlen - nlen && !isSSE; i++) { + isSSE = strncasecmp(v + i, T_text_event_stream, nlen) == 0; } - } else if (name.equalsIgnoreCase(T_Transfer_Encoding)) { - String lowcase(value); - lowcase.toLowerCase(); - String key; - - while (lowcase.length()) { - auto pos = lowcase.indexOf(','); - if (pos >= 0) { - key = lowcase.substring(0, pos); - lowcase = lowcase.substring(pos + 1); - } else { - key = lowcase; - lowcase = ""; - } - key.trim(); - if (key == "chunked") { - _chunkSize = 0; - _chunkStartIndex = 0; - _chunkedParseState = CHUNK_LENGTH; - break; - } + } + // Server-Sent Events (HTML §9.2) are GET-only connections negotiated via + // Accept: text/event-stream. Only classify when the connection is still + // a plain HTTP connection so a previously detected WebSocket upgrade + // cannot be clobbered by header ordering. + if (isSSE && _method == AsyncWebRequestMethod::HTTP_GET && _reqconntype == RCT_HTTP) { + // WebEvent request can be uniquely identified by header: [Accept: text/event-stream] + _reqconntype = RCT_EVENT; + } + } else if (name.equalsIgnoreCase(T_Transfer_Encoding)) { + // Scan the value in place, token by token, instead of substring copies: + // a copy that fails to allocate is empty (null c_str() on ESP32), which + // silently dropped the "chunked" token and desynchronized the body parser + // (issue #500). value.c_str() is valid here because tryParse() rejects + // OOM copies before we get here. + const char *v = value.c_str(); + const size_t vlen = value.length(); + const size_t tlen = sizeof(T_chunked) - 1; + size_t i = 0; + while (i < vlen) { + // skip OWS and list separators + while (i < vlen && (v[i] == ' ' || v[i] == '\t' || v[i] == ',')) { + i++; + } + size_t start = i; + while (i < vlen && v[i] != ',' && v[i] != ' ' && v[i] != '\t') { + i++; + } + if (i - start == tlen && strncasecmp(v + start, T_chunked, tlen) == 0) { + _chunkSize = 0; + _chunkStartIndex = 0; + _chunkedParseState = CHUNK_LENGTH; + break; } } - _headers.emplace_back(std::move(header)); } -#if defined(TARGET_RP2040) || defined(TARGET_RP2350) || defined(PICO_RP2040) || defined(PICO_RP2350) || defined(LIBRETINY) || defined(HOST) - // ArduinoCore-API does not have String::clear() method 8-() - _temp = asyncsrv::emptyString; -#else - _temp.clear(); -#endif - return true; + _headers.emplace_back(std::move(header)); } void AsyncWebServerRequest::_parsePlainPostChar(uint8_t data) { if (data && (char)data != '&') { - _temp += (char)data; + // concat() can fail to allocate; a truncated value would silently corrupt + // the parameter being parsed, so abort the request instead. + if (!_temp.concat((char)data)) { + async_ws_log_e("Failed to allocate"); + _parseState = PARSE_REQ_FAIL; + abort(); + return; + } } if (!data || (char)data == '&' || _parsedLength == _contentLength) { String name(T_BODY); String value(_temp); + // A copy of a non-empty _temp can only be empty when its allocation + // failed (c_str() is then null on ESP32); abort instead of storing a + // corrupt parameter (see issue #500). + if (_temp.length() && !value.length()) { + async_ws_log_e("Failed to allocate"); + _parseState = PARSE_REQ_FAIL; + abort(); + return; + } if (!(_temp.charAt(0) == '{') && !(_temp.charAt(0) == '[') && _temp.indexOf('=') > 0) { name = _temp.substring(0, _temp.indexOf('=')); value = _temp.substring(_temp.indexOf('=') + 1); + // substring() can fail to allocate: a non-empty slice can never come + // back empty. Abort the request instead of storing a corrupt pair. + if ((_temp.indexOf('=') && !name.length()) || (_temp.length() > (size_t)(_temp.indexOf('=') + 1) && !value.length())) { + async_ws_log_e("Failed to allocate"); + _parseState = PARSE_REQ_FAIL; + abort(); + return; + } } name = urlDecode(name); if (name.length()) { @@ -776,13 +882,18 @@ enum { }; void AsyncWebServerRequest::_parseMultipartPostByte(uint8_t data, bool last) { -#define itemWriteByte(b) \ - do { \ - _itemSize++; \ - if (_itemIsFile) \ - _handleUploadByte(b, last); \ - else \ - _itemValue += (char)(b); \ +#define itemWriteByte(b) \ + do { \ + _itemSize++; \ + if (_itemIsFile) \ + _handleUploadByte(b, last); \ + else if (!_itemValue.concat((char)(b))) { \ + /* truncated item value: abort */ \ + async_ws_log_e("Failed to allocate"); \ + _multiParseState = PARSE_ERROR; \ + abort(); \ + return; \ + } \ } while (0) if (!_parsedLength) { @@ -824,7 +935,14 @@ void AsyncWebServerRequest::_parseMultipartPostByte(uint8_t data, bool last) { } } else if (_multiParseState == PARSE_HEADERS) { if ((char)data != '\r' && (char)data != '\n') { - _temp += (char)data; + // A truncated item header line would misidentify the part; abort on + // allocation failure rather than continue with a corrupt line. + if (!_temp.concat((char)data)) { + async_ws_log_e("Failed to allocate"); + _multiParseState = PARSE_ERROR; + abort(); + return; + } } if ((char)data == '\n') { if (_temp.length()) { @@ -1014,8 +1132,13 @@ void AsyncWebServerRequest::_parseLine() { _server->_rewriteRequest(this); _server->_attachHandler(this); if (_expectingContinue) { - String response(T_HTTP_100_CONT); - _client->write(response.c_str(), response.length()); + // Write the literal directly: wrapping it in a String allocates (it + // exceeds String's small-buffer optimization), and an OOM'd String + // would be empty and silently swallow the 100 Continue (the client + // would then stall until its own continue timeout). A failed write + // here is harmless for the same reason: the client recovers on its + // own, and the request can still be processed normally. + _client->write(T_HTTP_100_CONT, sizeof(T_HTTP_100_CONT) - 1); } if (_contentLength || _chunkedParseState != CHUNK_NONE) { _parseState = PARSE_REQ_BODY; @@ -1056,10 +1179,18 @@ void AsyncWebServerRequest::_send() { } // response is not valid ? - if (!_response->_sourceValid()) { + if (_response && !_response->_sourceValid()) { send(500, T_text_plain, "Invalid data in handler"); } + // no response could be built at all (out of memory): give up on this + // connection instead of dereferencing a null response + if (!_response) { + async_ws_log_e("Failed to allocate"); + abort(); + return; + } + // here, we either have a response given from user or one of the two above _client->setRxTimeout(0); _response->_respond(this); @@ -1208,20 +1339,20 @@ double AsyncWebServerRequest::getAttribute(const char *name, double defaultValue AsyncWebServerResponse *AsyncWebServerRequest::beginResponse(int code, const char *contentType, const char *content, AwsTemplateProcessor callback) { if (callback) { - return new AsyncProgmemResponse(code, contentType, (const uint8_t *)content, strlen(content), callback); + return new (std::nothrow) AsyncProgmemResponse(code, contentType, (const uint8_t *)content, strlen(content), callback); } - return new AsyncBasicResponse(code, contentType, content); + return new (std::nothrow) AsyncBasicResponse(code, contentType, content); } AsyncWebServerResponse * AsyncWebServerRequest::beginResponse(int code, const char *contentType, const uint8_t *content, size_t len, AwsTemplateProcessor callback) { - return new AsyncProgmemResponse(code, contentType, content, len, callback); + return new (std::nothrow) AsyncProgmemResponse(code, contentType, content, len, callback); } AsyncWebServerResponse * AsyncWebServerRequest::beginResponse(FS &fs, const String &path, const char *contentType, bool download, AwsTemplateProcessor callback) { if (fs.exists(path) || (!download && fs.exists(path + T__gz))) { - return new AsyncFileResponse(fs, path, contentType, download, callback); + return new (std::nothrow) AsyncFileResponse(fs, path, contentType, download, callback); } return NULL; } @@ -1229,34 +1360,34 @@ AsyncWebServerResponse * AsyncWebServerResponse * AsyncWebServerRequest::beginResponse(File content, const String &path, const char *contentType, bool download, AwsTemplateProcessor callback) { if (content == true) { - return new AsyncFileResponse(content, path, contentType, download, callback); + return new (std::nothrow) AsyncFileResponse(content, path, contentType, download, callback); } return NULL; } AsyncWebServerResponse *AsyncWebServerRequest::beginResponse(Stream &stream, const char *contentType, size_t len, AwsTemplateProcessor callback) { - return new AsyncStreamResponse(stream, contentType, len, callback); + return new (std::nothrow) AsyncStreamResponse(stream, contentType, len, callback); } AsyncWebServerResponse * AsyncWebServerRequest::beginResponse(const char *contentType, size_t len, AwsResponseFiller callback, AwsTemplateProcessor templateCallback) { - return new AsyncCallbackResponse(contentType, len, callback, templateCallback); + return new (std::nothrow) AsyncCallbackResponse(contentType, len, callback, templateCallback); } AsyncWebServerResponse * AsyncWebServerRequest::beginChunkedResponse(const char *contentType, AwsResponseFiller callback, AwsTemplateProcessor templateCallback) { if (_version) { - return new AsyncChunkedResponse(contentType, callback, templateCallback); + return new (std::nothrow) AsyncChunkedResponse(contentType, callback, templateCallback); } - return new AsyncCallbackResponse(contentType, 0, callback, templateCallback); + return new (std::nothrow) AsyncCallbackResponse(contentType, 0, callback, templateCallback); } AsyncResponseStream *AsyncWebServerRequest::beginResponseStream(const char *contentType, size_t bufferSize) { - return new AsyncResponseStream(contentType, bufferSize); + return new (std::nothrow) AsyncResponseStream(contentType, bufferSize); } AsyncWebServerResponse *AsyncWebServerRequest::beginResponse_P(int code, const String &contentType, PGM_P content, AwsTemplateProcessor callback) { - return new AsyncProgmemResponse(code, contentType, (const uint8_t *)content, strlen_P(content), callback); + return new (std::nothrow) AsyncProgmemResponse(code, contentType, (const uint8_t *)content, strlen_P(content), callback); } void AsyncWebServerRequest::send(AsyncWebServerResponse *response) { @@ -1271,6 +1402,18 @@ void AsyncWebServerRequest::send(AsyncWebServerResponse *response) { } _response = response; + // a null response means the caller could not allocate it: fall back to a + // plain 500 so _send() never dereferences a null _response + if (_response == NULL && !_paused) { + _response = new (std::nothrow) AsyncBasicResponse(500, T_text_plain, "Insufficient memory"); + if (_response == NULL) { + // not even the fallback could be allocated: give up on this connection + async_ws_log_e("Failed to allocate"); + abort(); + return; + } + } + // if request was paused, we need to send the response now if (_paused) { _paused = false; @@ -1280,6 +1423,13 @@ void AsyncWebServerRequest::send(AsyncWebServerResponse *response) { void AsyncWebServerRequest::redirect(const char *url, int code) { AsyncWebServerResponse *response = beginResponse(code); + if (!response) { + // beginResponse could not allocate: send() would dereference a null + // response, so abort the request instead. + async_ws_log_e("Failed to allocate"); + abort(); + return; + } response->addHeader(T_LOCATION, url); send(response); } @@ -1329,6 +1479,13 @@ void AsyncWebServerRequest::requestAuthentication(AsyncAuthType method, const ch } AsyncWebServerResponse *r = _authFailMsg ? beginResponse(401, T_text_html, _authFailMsg) : beginResponse(401); + if (!r) { + // beginResponse could not allocate: the switch below would dereference + // a null response, so abort the request instead. + async_ws_log_e("Failed to allocate"); + abort(); + return; + } switch (method) { case AsyncAuthType::AUTH_BASIC: diff --git a/src/WebResponses.cpp b/src/WebResponses.cpp index 4280f494b..8317f0082 100644 --- a/src/WebResponses.cpp +++ b/src/WebResponses.cpp @@ -206,35 +206,72 @@ void AsyncWebServerResponse::_assembleHead(String &buffer, uint8_t version) { len += header.name().length() + header.value().length() + 4; } - // prepare buffer - buffer.reserve(len); + // prepare buffer. Every concat() below is fallible: if one fails, the head + // would be truncated and the wire format corrupted, so stop appending at + // the first failure and let the caller observe buffer.length() == 0. + bool oom = !buffer.reserve(len); // HTTP header + if (!oom) { #ifdef ESP8266 - buffer.concat(PSTR("HTTP/1.")); + oom = !buffer.concat(PSTR("HTTP/1.")); #else - buffer.concat("HTTP/1."); + oom = !buffer.concat("HTTP/1."); #endif - buffer.concat(version); - buffer.concat(' '); - buffer.concat(_code); - buffer.concat(' '); - buffer.concat(responseCodeToString(_code)); - buffer.concat(T_rn); + } + if (!oom) { + oom = !buffer.concat(version); + } + if (!oom) { + oom = !buffer.concat(' '); + } + if (!oom) { + oom = !buffer.concat(_code); + } + if (!oom) { + oom = !buffer.concat(' '); + } + if (!oom) { + oom = !buffer.concat(responseCodeToString(_code)); + } + if (!oom) { + oom = !buffer.concat(T_rn); + } // Add headers for (const auto &header : _headers) { - buffer.concat(header.name()); + if (!oom) { + oom = !buffer.concat(header.name()); + } + if (!oom) { #ifdef ESP8266 - buffer.concat(PSTR(": ")); + oom = !buffer.concat(PSTR(": ")); #else - buffer.concat(": "); + oom = !buffer.concat(": "); #endif - buffer.concat(header.value()); - buffer.concat(T_rn); + } + if (!oom) { + oom = !buffer.concat(header.value()); + } + if (!oom) { + oom = !buffer.concat(T_rn); + } + if (oom) { + break; + } } - buffer.concat(T_rn); + if (!oom) { + oom = !buffer.concat(T_rn); + } + + if (oom) { + // Out of memory while assembling the head: truncate to an empty buffer so + // callers treat it as a failure (writing a partial head would corrupt the + // wire format or dereference a null c_str()). + async_ws_log_e("Failed to allocate"); + buffer = String(); + } _headLength = buffer.length(); } @@ -273,6 +310,13 @@ AsyncBasicResponse::AsyncBasicResponse(int code, const char *contentType, const void AsyncBasicResponse::_respond(AsyncWebServerRequest *request) { _state = RESPONSE_HEADERS; _assembleHead(_assembled_headers, request->version()); + if (!_headLength) { + // Head assembly ran out of memory: fail the response instead of sending + // a corrupted (partial or missing) head. + _state = RESPONSE_FAILED; + request->client()->close(); + return; + } write_send_buffs(request, 0, 0); } @@ -334,6 +378,13 @@ AsyncAbstractResponse::AsyncAbstractResponse(AwsTemplateProcessor callback) : _c void AsyncAbstractResponse::_respond(AsyncWebServerRequest *request) { addHeader(T_Connection, T_close, false); _assembleHead(_assembled_headers, request->version()); + if (!_headLength) { + // Head assembly ran out of memory: fail the response instead of sending + // a corrupted (partial or missing) head. + _state = RESPONSE_FAILED; + request->client()->close(); + return; + } _state = RESPONSE_HEADERS; write_send_buffs(request, 0, 0); } From b4f4bced6530c2b0c460a2af29877a5167f913a8 Mon Sep 17 00:00:00 2001 From: Mathieu Carbou Date: Wed, 28 Oct 2026 22:28:28 +0100 Subject: [PATCH 2/3] Remove explanatory comments around malloc/nothrow changes Keep the code self-explanatory: the allocation change does not need an inline comment in each of the call sites. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- src/AsyncWebSocket.cpp | 6 ------ src/WebAuthentication.cpp | 3 --- src/WebHandlers.cpp | 2 -- 3 files changed, 11 deletions(-) diff --git a/src/AsyncWebSocket.cpp b/src/AsyncWebSocket.cpp index badd62bf9..c70cd338d 100644 --- a/src/AsyncWebSocket.cpp +++ b/src/AsyncWebSocket.cpp @@ -859,9 +859,6 @@ size_t AsyncWebSocketClient::printf(const char *format, ...) { return 0; } - // malloc (not new) so an allocation failure is observable: plain new aborts - // the device on ESP32/ESP8266 when exceptions are disabled, leaving the - // NULL-check below dead code. char *buffer = (char *)malloc(len + 1); if (!buffer) { @@ -1373,9 +1370,6 @@ size_t AsyncWebSocket::printfAll(const char *format, ...) { return 0; } - // malloc (not new) so an allocation failure is observable: plain new aborts - // the device on ESP32/ESP8266 when exceptions are disabled, leaving the - // NULL-check below dead code. char *buffer = (char *)malloc(len + 1); if (!buffer) { diff --git a/src/WebAuthentication.cpp b/src/WebAuthentication.cpp index e589f9fc3..21d461108 100644 --- a/src/WebAuthentication.cpp +++ b/src/WebAuthentication.cpp @@ -31,9 +31,6 @@ String generateBasicHash(const char *username, const char *password) { size_t toencodeLen = strlen(username) + strlen(password) + 1; - // malloc (not new) so an allocation failure is observable: plain new - // aborts the device on ESP32/ESP8266 when exceptions are disabled, leaving - // the NULL-check below dead code. char *toencode = (char *)malloc(toencodeLen + 1); if (toencode == NULL) { async_ws_log_e("Failed to allocate"); diff --git a/src/WebHandlers.cpp b/src/WebHandlers.cpp index 409f22f6f..00caa6bc9 100644 --- a/src/WebHandlers.cpp +++ b/src/WebHandlers.cpp @@ -286,8 +286,6 @@ void AsyncStaticWebHandler::handleRequest(AsyncWebServerRequest *request) { if (notModified) { request->_tempFile.close(); - // nothrow: plain new aborts the device on ESP32/ESP8266 when out of - // memory, making a NULL-check dead code. response = new (std::nothrow) AsyncBasicResponse(304); // Not modified } else { response = new (std::nothrow) AsyncFileResponse(request->_tempFile, filename, asyncsrv::emptyString, false, _callback); From c839e63f4c5a7455cad5dbb49f6a04659ac58a25 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sat, 31 Oct 2026 03:18:30 +0000 Subject: [PATCH 3/3] Use nothrow new with unique_ptr instead of malloc in printf helpers and generateBasicHash Co-authored-by: willmmiles <6540455+willmmiles@users.noreply.github.com> --- src/AsyncWebSocket.cpp | 29 +++++++++++++---------------- src/WebAuthentication.cpp | 23 ++++++++++------------- 2 files changed, 23 insertions(+), 29 deletions(-) diff --git a/src/AsyncWebSocket.cpp b/src/AsyncWebSocket.cpp index c70cd338d..56bb5ce8b 100644 --- a/src/AsyncWebSocket.cpp +++ b/src/AsyncWebSocket.cpp @@ -28,6 +28,7 @@ #include #include #include +#include #include #include @@ -859,7 +860,7 @@ size_t AsyncWebSocketClient::printf(const char *format, ...) { return 0; } - char *buffer = (char *)malloc(len + 1); + std::unique_ptr buffer(new (std::nothrow) char[len + 1]); if (!buffer) { async_ws_log_e("Failed to allocate"); @@ -867,11 +868,10 @@ size_t AsyncWebSocketClient::printf(const char *format, ...) { } va_start(arg, format); - len = vsnprintf(buffer, len + 1, format, arg); + len = vsnprintf(buffer.get(), len + 1, format, arg); va_end(arg); - bool enqueued = text(buffer, len); - free(buffer); + bool enqueued = text(buffer.get(), len); return enqueued ? len : 0; } @@ -886,7 +886,7 @@ size_t AsyncWebSocketClient::printf_P(PGM_P formatP, ...) { return 0; } - char *buffer = (char *)malloc(len + 1); + std::unique_ptr buffer(new (std::nothrow) char[len + 1]); if (!buffer) { async_ws_log_e("Failed to allocate"); @@ -894,11 +894,10 @@ size_t AsyncWebSocketClient::printf_P(PGM_P formatP, ...) { } va_start(arg, formatP); - len = vsnprintf_P(buffer, len + 1, formatP, arg); + len = vsnprintf_P(buffer.get(), len + 1, formatP, arg); va_end(arg); - bool enqueued = text(buffer, len); - free(buffer); + bool enqueued = text(buffer.get(), len); return enqueued ? len : 0; } #endif @@ -1370,7 +1369,7 @@ size_t AsyncWebSocket::printfAll(const char *format, ...) { return 0; } - char *buffer = (char *)malloc(len + 1); + std::unique_ptr buffer(new (std::nothrow) char[len + 1]); if (!buffer) { async_ws_log_e("Failed to allocate"); @@ -1378,11 +1377,10 @@ size_t AsyncWebSocket::printfAll(const char *format, ...) { } va_start(arg, format); - len = vsnprintf(buffer, len + 1, format, arg); + len = vsnprintf(buffer.get(), len + 1, format, arg); va_end(arg); - AsyncWebSocket::SendStatus status = textAll(buffer, len); - free(buffer); + AsyncWebSocket::SendStatus status = textAll(buffer.get(), len); return status == DISCARDED ? 0 : len; } @@ -1409,7 +1407,7 @@ size_t AsyncWebSocket::printfAll_P(PGM_P formatP, ...) { return 0; } - char *buffer = (char *)malloc(len + 1); + std::unique_ptr buffer(new (std::nothrow) char[len + 1]); if (!buffer) { async_ws_log_e("Failed to allocate"); @@ -1417,11 +1415,10 @@ size_t AsyncWebSocket::printfAll_P(PGM_P formatP, ...) { } va_start(arg, formatP); - len = vsnprintf_P(buffer, len + 1, formatP, arg); + len = vsnprintf_P(buffer.get(), len + 1, formatP, arg); va_end(arg); - AsyncWebSocket::SendStatus status = textAll(buffer, len); - free(buffer); + AsyncWebSocket::SendStatus status = textAll(buffer.get(), len); return status == DISCARDED ? 0 : len; } #endif diff --git a/src/WebAuthentication.cpp b/src/WebAuthentication.cpp index 21d461108..76199afb7 100644 --- a/src/WebAuthentication.cpp +++ b/src/WebAuthentication.cpp @@ -5,6 +5,9 @@ #include "AsyncWebServerLogging.h" #include +#include +#include + #if defined(ESP32) || defined(TARGET_RP2040) || defined(TARGET_RP2350) || defined(PICO_RP2040) || defined(PICO_RP2350) || defined(HOST) #include #else @@ -31,26 +34,20 @@ String generateBasicHash(const char *username, const char *password) { size_t toencodeLen = strlen(username) + strlen(password) + 1; - char *toencode = (char *)malloc(toencodeLen + 1); - if (toencode == NULL) { + std::unique_ptr toencode(new (std::nothrow) char[toencodeLen + 1]); + if (!toencode) { async_ws_log_e("Failed to allocate"); return asyncsrv::emptyString; } - char *encoded = (char *)malloc(base64_encode_expected_len(toencodeLen) + 1); - if (encoded == NULL) { + std::unique_ptr encoded(new (std::nothrow) char[base64_encode_expected_len(toencodeLen) + 1]); + if (!encoded) { async_ws_log_e("Failed to allocate"); - free(toencode); return asyncsrv::emptyString; } - sprintf_P(toencode, PSTR("%s:%s"), username, password); - if (base64_encode_chars(toencode, toencodeLen, encoded) > 0) { - String res = String(encoded); - free(toencode); - free(encoded); - return res; + sprintf_P(toencode.get(), PSTR("%s:%s"), username, password); + if (base64_encode_chars(toencode.get(), toencodeLen, encoded.get()) > 0) { + return String(encoded.get()); } - free(toencode); - free(encoded); return asyncsrv::emptyString; }