Skip to content

Commit 300d91c

Browse files
committed
Fix CWE-190 - DoS could happen with a specially crafted boundary parameter in a multipart request
1 parent a87fcdb commit 300d91c

3 files changed

Lines changed: 47 additions & 4 deletions

File tree

src/ESPAsyncWebServer.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -465,7 +465,7 @@ 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+
size_t _boundaryPosition;
469469
size_t _itemStartIndex;
470470
size_t _itemSize;
471471
String _itemName;

src/WebRequest.cpp

Lines changed: 44 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -474,8 +474,50 @@ 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+
String lowcase(value);
478+
lowcase.toLowerCase();
479+
480+
int bpos = -1;
481+
int searchFrom = 0;
482+
while (true) {
483+
int pos = lowcase.indexOf(T_BOUNDARY, searchFrom);
484+
if (pos < 0) {
485+
break;
486+
}
487+
488+
// Only match a real parameter (must be preceded by ';' with optional whitespace)
489+
int p = pos - 1;
490+
while (p >= 0 && (lowcase.charAt(p) == ' ' || lowcase.charAt(p) == '\t')) {
491+
p--;
492+
}
493+
if (p >= 0 && lowcase.charAt(p) == ';') {
494+
bpos = pos;
495+
break;
496+
}
497+
498+
searchFrom = pos + 1;
499+
}
500+
if (bpos < 0) {
501+
async_ws_log_d("Missing multipart boundary parameter, aborting");
502+
_parseState = PARSE_REQ_FAIL;
503+
abort();
504+
return true;
505+
}
506+
_boundary = value.substring(bpos + (int)T_BOUNDARY_LEN);
507+
int semi = _boundary.indexOf(';');
508+
if (semi >= 0) {
509+
_boundary = _boundary.substring(0, semi);
510+
}
511+
_boundary.trim();
478512
_boundary.replace(String('"'), String());
513+
// RFC 2046 §5.1 limits boundary strings to 70 characters.
514+
// Reject invalid boundaries to prevent integer overflow in the parser.
515+
if (_boundary.length() == 0 || _boundary.length() > 70) {
516+
async_ws_log_d("Invalid multipart boundary length (%u), aborting", _boundary.length());
517+
_parseState = PARSE_REQ_FAIL;
518+
abort();
519+
return true;
520+
}
479521
_isMultipart = true;
480522
}
481523
} else if (name.equalsIgnoreCase(T_Content_Length) || name.equalsIgnoreCase(T_X_Expected_Entity_Length)) {
@@ -743,8 +785,7 @@ void AsyncWebServerRequest::_parseMultipartPostByte(uint8_t data, bool last) {
743785
itemWriteByte('\n');
744786
itemWriteByte('-');
745787
itemWriteByte('-');
746-
uint8_t i;
747-
for (i = 0; i < _boundaryPosition; i++) {
788+
for (size_t i = 0; i < _boundaryPosition; i++) {
748789
itemWriteByte(_boundary.c_str()[i]);
749790
}
750791
_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)