Skip to content

Commit c7114c0

Browse files
committed
Fix CWE-190: integer overflow in multipart boundary parser (DoS)
_boundaryPosition was declared as uint8_t. A remote attacker could send a Content-Type header with a boundary string of exactly 256 bytes, causing the increment in _parseMultipartPostByte() to overflow back to 0. The parser would then loop indefinitely in the BOUNDARY_OR_DATA state, consuming 100% CPU and triggering the FreeRTOS watchdog on ESP32/ESP8266. Fixes: - ESPAsyncWebServer.h: change _boundaryPosition from uint8_t to size_t, eliminating the overflow entirely. - WebRequest.cpp (_parseMultipartPostByte): change the rewind loop variable from uint8_t to size_t for consistency. - WebRequest.cpp (_parseReqHeader): validate the boundary value extracted from the Content-Type header before any body parsing begins: * Require the 'boundary=' token to be a proper RFC 2045 parameter (immediately preceded by ';'), preventing substring false-matches. * Strip optional surrounding whitespace, quotes and trailing parameters (RFC 2046 quoted-string form). * Reject boundaries that are empty or longer than 70 characters (RFC 2046 §5.1 hard limit); drop the connection immediately with PARSE_REQ_FAIL + abort() instead of continuing.
1 parent a87fcdb commit c7114c0

3 files changed

Lines changed: 68 additions & 4 deletions

File tree

src/ESPAsyncWebServer.h

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -465,7 +465,9 @@ class AsyncWebServerRequest {
465465
std::unordered_map<const char *, String, std::hash<const char *>, std::equal_to<const char *>> _attributes;
466466

467467
uint8_t _multiParseState;
468-
uint8_t _boundaryPosition;
468+
// CWE-190 fix: was uint8_t, which would overflow to 0 for boundaries >= 256
469+
// bytes and cause the BOUNDARY_OR_DATA parser loop to never terminate (DoS).
470+
size_t _boundaryPosition;
469471
size_t _itemStartIndex;
470472
size_t _itemSize;
471473
String _itemName;

src/WebRequest.cpp

Lines changed: 63 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -474,8 +474,67 @@ bool AsyncWebServerRequest::_parseReqHeader() {
474474
} else if (name.equalsIgnoreCase(T_Content_Type)) {
475475
_contentType = value.substring(0, value.indexOf(';'));
476476
if (value.startsWith(T_MULTIPART_)) {
477-
_boundary = value.substring(value.indexOf('=') + 1);
477+
// Case-insensitive search for the boundary parameter.
478+
// A simple indexOf("boundary=") would incorrectly match a token whose
479+
// name merely ends in "boundary=" (e.g. "x-boundary="). We require
480+
// the match to be immediately preceded by a ';' (with optional
481+
// whitespace) to ensure it is a proper RFC 2045 parameter.
482+
String lowcase(value);
483+
lowcase.toLowerCase();
484+
485+
int bpos = -1;
486+
int searchFrom = 0;
487+
while (true) {
488+
int pos = lowcase.indexOf(T_BOUNDARY, searchFrom);
489+
if (pos < 0) {
490+
break;
491+
}
492+
493+
// Only match a real parameter (must be preceded by ';' with optional whitespace)
494+
int p = pos - 1;
495+
while (p >= 0 && (lowcase.charAt(p) == ' ' || lowcase.charAt(p) == '\t')) {
496+
p--;
497+
}
498+
if (p >= 0 && lowcase.charAt(p) == ';') {
499+
bpos = pos;
500+
break;
501+
}
502+
503+
searchFrom = pos + 1;
504+
}
505+
506+
if (bpos < 0) {
507+
// No valid boundary parameter found — cannot parse multipart body.
508+
async_ws_log_d("Missing multipart boundary parameter, aborting");
509+
_parseState = PARSE_REQ_FAIL;
510+
abort();
511+
return true;
512+
}
513+
514+
// Extract the boundary value; strip any following parameters, optional
515+
// surrounding whitespace and quotes (RFC 2046 allows quoted-string).
516+
_boundary = value.substring(bpos + (int)T_BOUNDARY_LEN);
517+
int semi = _boundary.indexOf(';');
518+
if (semi >= 0) {
519+
_boundary = _boundary.substring(0, semi);
520+
}
521+
522+
_boundary.trim();
478523
_boundary.replace(String('"'), String());
524+
525+
// CWE-190 / DoS fix: RFC 2046 §5.1 limits boundary strings to 70
526+
// characters. _boundaryPosition was formerly uint8_t, so a boundary
527+
// of exactly 256 bytes would overflow the counter to 0, preventing the
528+
// BOUNDARY_OR_DATA loop from ever reaching the end of the boundary and
529+
// causing unbounded CPU consumption (watchdog reset on ESP32/ESP8266).
530+
// Reject boundaries outside the valid range before any parsing begins.
531+
if (_boundary.length() == 0 || _boundary.length() > 70) {
532+
async_ws_log_d("Invalid multipart boundary length (%u), aborting", _boundary.length());
533+
_parseState = PARSE_REQ_FAIL;
534+
abort();
535+
return true;
536+
}
537+
479538
_isMultipart = true;
480539
}
481540
} else if (name.equalsIgnoreCase(T_Content_Length) || name.equalsIgnoreCase(T_X_Expected_Entity_Length)) {
@@ -743,8 +802,9 @@ void AsyncWebServerRequest::_parseMultipartPostByte(uint8_t data, bool last) {
743802
itemWriteByte('\n');
744803
itemWriteByte('-');
745804
itemWriteByte('-');
746-
uint8_t i;
747-
for (i = 0; i < _boundaryPosition; i++) {
805+
// CWE-190 fix: loop variable was uint8_t, matching the old type of
806+
// _boundaryPosition. Changed to size_t for consistency.
807+
for (size_t i = 0; i < _boundaryPosition; i++) {
748808
itemWriteByte(_boundary.c_str()[i]);
749809
}
750810
_parseMultipartPostByte(data, last);

src/literals.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -108,6 +108,8 @@ static constexpr const char T_username[] = "username";
108108
static constexpr const char T_WS[] = "websocket";
109109
static constexpr const char T_WWW_AUTH[] = "WWW-Authenticate";
110110
static constexpr const char T_X_Expected_Entity_Length[] = "X-Expected-Entity-Length";
111+
static constexpr const char T_BOUNDARY[] = "boundary=";
112+
static constexpr size_t T_BOUNDARY_LEN = sizeof(T_BOUNDARY) - 1;
111113

112114
// HTTP Methods
113115
static constexpr const char T_GET[] = "GET";

0 commit comments

Comments
 (0)