Skip to content

fix(http): prevent use-after-free from synchronous abort()/close() #464

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
mathieucarbou wants to merge 1 commit into from
Closed

fix(http): prevent use-after-free from synchronous abort()/close() #464

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 17 additions & 0 deletions src/ESPAsyncWebServer.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -438,6 +438,15 @@ class AsyncWebServerRequest {
bool _paused = false; // request is paused (request continuation)
std::shared_ptr<AsyncWebServerRequest> _this; // shared pointer to this request

@willmmiles willmmiles Jul 21, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This (the shared_ptr) is the way out of locking and could substantially clean up the management logic. If we use the shared_ptr to take responsibility for doing object cleanup, then it's easy to extend the object lifetime over a function: just take a copy of the shared_ptr on the stack.

@willmmiles willmmiles Jul 21, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can sketch this later tonight.

@mathieucarbou mathieucarbou Jul 21, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!
Like I said above, the locking logic was isolated in a second commit since I also did not like it.
Feel free to drop it and complete the PR :-)
I will have a look tomorrow morning and merge the result if you are finished.

@mathieucarbou mathieucarbou Jul 21, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

update: removed the second commit with the lock, as I asked the user who got the issue to retest with these changes, and he will do so in the morning.


// Re-entrancy guard for AsyncTCP 3.5.0+ where abort()/close() fire the
// disconnect (discard) callback synchronously. If _onDisconnect() runs
// while we are executing inside the async_tcp task (_inAsyncTcpTask ==
// true), it must NOT `delete this` — the AsyncClient is still on the call
// stack. Instead it sets _disconnectPending and the top-level callback
// performs the delete after unwinding.
bool _inAsyncTcpTask = false;
bool _disconnectPending = false;

String _temp;
uint8_t _parseState;

Expand Down Expand Up @@ -485,6 +494,14 @@ class AsyncWebServerRequest {
uint8_t _chunkedLastChar;
bool _parseChunkedBytes(uint8_t *data, size_t len);

// Called at every exit point of a top-level async_tcp task callback
// (_onData, _onAck, _onPoll, _onTimeout). Returns true if the request is
// being disconnected (abort()/close() ran _onDisconnect() re-entrantly
// during the callback), in which case the caller must `return` immediately
// without touching `this` — this method has already performed `delete this`
// via _handleDisconnect().
bool _onAsyncTcpTaskExit();

void _onPoll();
void _onAck(size_t len, uint32_t time);
void _onError(int8_t error);
Expand Down
82 changes: 78 additions & 4 deletions src/WebRequest.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -124,12 +124,17 @@ AsyncWebServerRequest::~AsyncWebServerRequest() {
}

void AsyncWebServerRequest::_onData(void *buf, size_t len) {
_inAsyncTcpTask = true;

// SSL/TLS handshake detection
#ifndef ASYNC_TCP_SSL_ENABLED
if (_parseState == PARSE_REQ_START && len && ((uint8_t *)buf)[0] == 0x16) { // 0x16 indicates a Handshake message (SSL/TLS).
async_ws_log_d("SSL/TLS handshake detected: resetting connection");
_parseState = PARSE_REQ_FAIL;
abort();
if (_onAsyncTcpTaskExit()) {
return;
}
return;
}
#endif
Expand All @@ -145,6 +150,9 @@ void AsyncWebServerRequest::_onData(void *buf, size_t len) {
if (!str[i]) {
_parseState = PARSE_REQ_FAIL;
abort();
if (_onAsyncTcpTaskExit()) {
return;
}
return;
}
if (str[i] == '\n') {
Expand All @@ -158,6 +166,9 @@ void AsyncWebServerRequest::_onData(void *buf, size_t len) {
async_ws_log_e("Failed to allocate");
_parseState = PARSE_REQ_FAIL;
abort();
if (_onAsyncTcpTaskExit()) {
return;
}
return;
}
_temp.concat(str);
Expand All @@ -167,6 +178,11 @@ void AsyncWebServerRequest::_onData(void *buf, size_t len) {
_temp.concat(str);
_temp.trim();
_parseLine();
// _parseLine() may call abort() on a parse error; bail out before
// touching any member if the request is being disconnected.
if (_disconnectPending) {
break;
}
if (++i < len) {
// Still have more buffer to process
buf = str + i;
Expand All @@ -177,9 +193,13 @@ void AsyncWebServerRequest::_onData(void *buf, size_t len) {
} else if (_parseState == PARSE_REQ_BODY) {
if (_chunkedParseState != CHUNK_NONE) {
if (_parseChunkedBytes((uint8_t *)buf, len)) {
_parseState = PARSE_REQ_END;
_runMiddlewareChain();
_send();
// _parseChunkedBytes may have called abort() on a parse error;
// if so, _send()/_runMiddlewareChain() must be skipped.
if (!_disconnectPending) {
_parseState = PARSE_REQ_END;
_runMiddlewareChain();
_send();
}
}
break;
}
Expand All @@ -193,6 +213,10 @@ void AsyncWebServerRequest::_onData(void *buf, size_t len) {
size_t i;
for (i = 0; i < len; i++) {
_parseMultipartPostByte(((uint8_t *)buf)[i], i == len - 1);
// _parseMultipartPostByte may call abort() on a parse error.
if (_disconnectPending) {
break;
}
_parsedLength++;
}
} else {
Expand Down Expand Up @@ -229,6 +253,9 @@ void AsyncWebServerRequest::_onData(void *buf, size_t len) {
_parsedLength += len;
}
}
if (_disconnectPending) {
break;
}
if (_parsedLength == _contentLength) {
_parseState = PARSE_REQ_END;
_runMiddlewareChain();
Expand All @@ -237,18 +264,30 @@ void AsyncWebServerRequest::_onData(void *buf, size_t len) {
}
break;
}

if (_onAsyncTcpTaskExit()) {
return;
}
}

void AsyncWebServerRequest::_onPoll() {
_inAsyncTcpTask = true;
// os_printf("p\n");
if (_response && _client && _client->canSend()) {
_response->_ack(this, 0, 0);
}
if (_onAsyncTcpTaskExit()) {
return;
}
}

void AsyncWebServerRequest::_onAck(size_t len, uint32_t time) {
_inAsyncTcpTask = true;
// os_printf("a:%u:%u\n", len, time);
if (!_response) {
if (_onAsyncTcpTaskExit()) {
return;
}
return;
}

Expand All @@ -262,28 +301,59 @@ void AsyncWebServerRequest::_onAck(size_t len, uint32_t time) {
// this will close responses that were complete via a single _send() call
_client->close(); // this will trigger _onDisconnect() and object destruction
}
if (_onAsyncTcpTaskExit()) {
return;
}
}

void AsyncWebServerRequest::_onError(int8_t error) {
(void)error;
}

void AsyncWebServerRequest::_onTimeout(uint32_t time) {
_inAsyncTcpTask = true;
(void)time;
// os_printf("TIMEOUT: %u, state: %s\n", time, _client->stateToString());
_client->close();
if (_onAsyncTcpTaskExit()) {
return;
}
}

void AsyncWebServerRequest::onDisconnect(ArDisconnectHandler fn) {
_onDisconnectfn = fn;
}

bool AsyncWebServerRequest::_onAsyncTcpTaskExit() {
_inAsyncTcpTask = false;
// AsyncTCP 3.5.0+ fires the discard (onDisconnect) callback synchronously
// from abort()/close(). If that happened during the callback we just ran,
// _onDisconnect() set _disconnectPending instead of deleting `this` (because
// the AsyncClient was still on the call stack). Now that we've unwound, it
// is safe to perform the deletion. Returns true if `this` was deleted — the
// caller must not touch any member afterward.
if (_disconnectPending) {
_server->_handleDisconnect(this); // delete this
return true;
}
return false;
}

void AsyncWebServerRequest::_onDisconnect() {
async_ws_log_v("onDisconnect() cb for request: %s", _url.c_str());
if (_onDisconnectfn) {
_onDisconnectfn();
}
_server->_handleDisconnect(this);
// AsyncTCP 3.5.0+ fires this callback synchronously from abort()/close().
// If we are executing inside the async_tcp task (_inAsyncTcpTask), the
// AsyncClient is still on the call stack and `delete this` here would
// destroy it mid-call (use-after-free). Defer the deletion to
// _onAsyncTcpTaskExit().
if (_inAsyncTcpTask) {
_disconnectPending = true;
} else {
_server->_handleDisconnect(this);
}
}

void AsyncWebServerRequest::_addGetParams(const String &params) {
Expand Down Expand Up @@ -1327,7 +1397,9 @@ void AsyncWebServerRequest::requestAuthentication(AsyncAuthType method, const ch
r->addHeader(T_WWW_AUTH, header.c_str());
} else {
async_ws_log_e("Failed to allocate");
delete r;
abort();
return;
}

break;
Expand All @@ -1351,7 +1423,9 @@ void AsyncWebServerRequest::requestAuthentication(AsyncAuthType method, const ch
r->addHeader(T_WWW_AUTH, header.c_str());
} else {
async_ws_log_e("Failed to allocate");
delete r;
abort();
return;
}
}
break;
Expand Down
Loading