Fix 162: closing websocket from ESP32 with Safari crashes the ESP32.

This happens when Safari sends a WS frame with masked bit set and length 2.
This commit is contained in:
Mathieu Carbou
2024-12-10 13:37:12 +01:00
parent 5b5efb96f5
commit ee9d406190
2 changed files with 35 additions and 11 deletions

View File

@@ -56,13 +56,23 @@ void setup() {
<head></head> <head></head>
<script> <script>
let ws = new WebSocket("ws://" + window.location.host + "/ws"); let ws = new WebSocket("ws://" + window.location.host + "/ws");
ws.addEventListener("open", (e) => {
console.log("WebSocket connected", e);
});
ws.addEventListener("error", (e) => {
console.log("WebSocket error", e);
});
ws.addEventListener("close", (e) => {
console.log("WebSocket close", e);
});
ws.addEventListener("message", (e) => {
console.log("WebSocket message", e);
});
document.addEventListener("DOMContentLoaded", function () {
ws.onopen = function () {
console.log("WebSocket connected");
};
});
function closeAllWsClients() { function closeAllWsClients() {
fetch("/close_all_ws_clients", { fetch("/close_all_ws_clients", {
method : "POST", method : "POST",
@@ -79,6 +89,11 @@ void setup() {
server.begin(); server.begin();
} }
uint32_t lastTime = 0;
void loop() { void loop() {
vTaskDelete(NULL); if (millis() - lastTime > 5000) {
lastTime = millis();
Serial.printf("Client count: %u\n", ws.count());
}
ws.cleanupClients();
} }

View File

@@ -451,6 +451,8 @@ void AsyncWebSocketClient::close(uint16_t code, const char* message) {
if (_status != WS_CONNECTED) if (_status != WS_CONNECTED)
return; return;
_status = WS_DISCONNECTING;
if (code) { if (code) {
uint8_t packetLen = 2; uint8_t packetLen = 2;
if (message != NULL) { if (message != NULL) {
@@ -496,30 +498,37 @@ void AsyncWebSocketClient::_onDisconnect() {
} }
void AsyncWebSocketClient::_onData(void* pbuf, size_t plen) { void AsyncWebSocketClient::_onData(void* pbuf, size_t plen) {
// Serial.println("onData");
_lastMessageTime = millis(); _lastMessageTime = millis();
uint8_t* data = (uint8_t*)pbuf; uint8_t* data = (uint8_t*)pbuf;
while (plen > 0) { while (plen > 0) {
if (!_pstate) { if (!_pstate) {
const uint8_t* fdata = data; const uint8_t* fdata = data;
_pinfo.index = 0; _pinfo.index = 0;
_pinfo.final = (fdata[0] & 0x80) != 0; _pinfo.final = (fdata[0] & 0x80) != 0;
_pinfo.opcode = fdata[0] & 0x0F; _pinfo.opcode = fdata[0] & 0x0F;
_pinfo.masked = (fdata[1] & 0x80) != 0; _pinfo.masked = (fdata[1] & 0x80) != 0;
_pinfo.len = fdata[1] & 0x7F; _pinfo.len = fdata[1] & 0x7F;
// log_d("WS[%" PRIu32 "]: _onData: %" PRIu32, _clientId, plen);
// log_d("WS[%" PRIu32 "]: _status = %" PRIu32, _clientId, _status);
// log_d("WS[%" PRIu32 "]: _pinfo: index: %" PRIu64 ", final: %" PRIu8 ", opcode: %" PRIu8 ", masked: %" PRIu8 ", len: %" PRIu64, _clientId, _pinfo.index, _pinfo.final, _pinfo.opcode, _pinfo.masked, _pinfo.len);
data += 2; data += 2;
plen -= 2; plen -= 2;
if (_pinfo.len == 126) {
if (_pinfo.len == 126 && plen >= 2) {
_pinfo.len = fdata[3] | (uint16_t)(fdata[2]) << 8; _pinfo.len = fdata[3] | (uint16_t)(fdata[2]) << 8;
data += 2; data += 2;
plen -= 2; plen -= 2;
} else if (_pinfo.len == 127) {
} else if (_pinfo.len == 127 && plen >= 8) {
_pinfo.len = fdata[9] | (uint16_t)(fdata[8]) << 8 | (uint32_t)(fdata[7]) << 16 | (uint32_t)(fdata[6]) << 24 | (uint64_t)(fdata[5]) << 32 | (uint64_t)(fdata[4]) << 40 | (uint64_t)(fdata[3]) << 48 | (uint64_t)(fdata[2]) << 56; _pinfo.len = fdata[9] | (uint16_t)(fdata[8]) << 8 | (uint32_t)(fdata[7]) << 16 | (uint32_t)(fdata[6]) << 24 | (uint64_t)(fdata[5]) << 32 | (uint64_t)(fdata[4]) << 40 | (uint64_t)(fdata[3]) << 48 | (uint64_t)(fdata[2]) << 56;
data += 8; data += 8;
plen -= 8; plen -= 8;
} }
if (_pinfo.masked) { if (_pinfo.masked && plen >= 4) { // if ws.close() is called, Safari sends a close frame with plen 2 and masked bit set. We must not decrement plen which is already 0.
memcpy(_pinfo.mask, data, 4); memcpy(_pinfo.mask, data, 4);
data += 4; data += 4;
plen -= 4; plen -= 4;