From 576a78fc997bb3b742f5accba96b9f39a11acedc Mon Sep 17 00:00:00 2001 From: Daniel Manser Date: Fri, 23 May 2025 13:43:16 +0200 Subject: [PATCH 1/4] Keep parsing if FIN was not reached but bytes are available still --- lib/src/WebSocketConnectionImpl.cc | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/lib/src/WebSocketConnectionImpl.cc b/lib/src/WebSocketConnectionImpl.cc index c3395fd7d9..73e875f415 100644 --- a/lib/src/WebSocketConnectionImpl.cc +++ b/lib/src/WebSocketConnectionImpl.cc @@ -275,7 +275,7 @@ bool WebSocketMessageParser::parse(trantor::MsgBuffer *buffer) switch (opcode) { case 0: - // continuation frame + LOG_TRACE << "continuation frame"; break; case 1: type_ = WebSocketMessageType::Text; @@ -383,7 +383,16 @@ bool WebSocketMessageParser::parse(trantor::MsgBuffer *buffer) if (isFin) gotAll_ = true; buffer->retrieve(indexFirstMask + 4 + length); - return true; + if (!isFin && buffer->readableBytes() >= 2) + { + // Keep parsing if FIN was not reached but bytes are available still. + // It might be a continuation frame. + return parse(buffer); + } + else + { + return true; + } } } else From 2472bb5151020d848ec9a4ab0170d071152ad589 Mon Sep 17 00:00:00 2001 From: antao Date: Fri, 23 May 2025 23:07:34 +0800 Subject: [PATCH 2/4] Use loops instead of recursion --- lib/src/WebSocketConnectionImpl.cc | 19 +++++++------------ 1 file changed, 7 insertions(+), 12 deletions(-) diff --git a/lib/src/WebSocketConnectionImpl.cc b/lib/src/WebSocketConnectionImpl.cc index 73e875f415..64f6d002a4 100644 --- a/lib/src/WebSocketConnectionImpl.cc +++ b/lib/src/WebSocketConnectionImpl.cc @@ -268,7 +268,7 @@ bool WebSocketMessageParser::parse(trantor::MsgBuffer *buffer) { // According to the rfc6455 gotAll_ = false; - if (buffer->readableBytes() >= 2) + while (buffer->readableBytes() >= 2) { unsigned char opcode = (*buffer)[0] & 0x0f; bool isControlFrame = false; @@ -380,17 +380,10 @@ bool WebSocketMessageParser::parse(trantor::MsgBuffer *buffer) { message_[oldLen + i] = (rawData[i] ^ masks[i % 4]); } - if (isFin) - gotAll_ = true; buffer->retrieve(indexFirstMask + 4 + length); - if (!isFin && buffer->readableBytes() >= 2) - { - // Keep parsing if FIN was not reached but bytes are available still. - // It might be a continuation frame. - return parse(buffer); - } - else + if (isFin) { + gotAll_ = true; return true; } } @@ -401,10 +394,12 @@ bool WebSocketMessageParser::parse(trantor::MsgBuffer *buffer) { auto rawData = buffer->peek() + indexFirstMask; message_.append(rawData, length); + buffer->retrieve(indexFirstMask + 4 + length); if (isFin) + { gotAll_ = true; - buffer->retrieve(indexFirstMask + length); - return true; + return true; + } } } } From 0bc4b224d16fbe87a74144046de05985631595fb Mon Sep 17 00:00:00 2001 From: antao Date: Fri, 23 May 2025 23:23:40 +0800 Subject: [PATCH 3/4] Fix --- lib/src/WebSocketConnectionImpl.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/src/WebSocketConnectionImpl.cc b/lib/src/WebSocketConnectionImpl.cc index 64f6d002a4..c5853208f2 100644 --- a/lib/src/WebSocketConnectionImpl.cc +++ b/lib/src/WebSocketConnectionImpl.cc @@ -394,7 +394,7 @@ bool WebSocketMessageParser::parse(trantor::MsgBuffer *buffer) { auto rawData = buffer->peek() + indexFirstMask; message_.append(rawData, length); - buffer->retrieve(indexFirstMask + 4 + length); + buffer->retrieve(indexFirstMask + length); if (isFin) { gotAll_ = true; From 42cc36623361ebdfe9ca58afabc7f7bffe6e6fb3 Mon Sep 17 00:00:00 2001 From: antao Date: Sat, 24 May 2025 23:28:39 +0800 Subject: [PATCH 4/4] Fix infinite loop --- lib/src/WebSocketConnectionImpl.cc | 37 ++++++++++++++++++++++-------- 1 file changed, 28 insertions(+), 9 deletions(-) diff --git a/lib/src/WebSocketConnectionImpl.cc b/lib/src/WebSocketConnectionImpl.cc index c5853208f2..cd589b6aa8 100644 --- a/lib/src/WebSocketConnectionImpl.cc +++ b/lib/src/WebSocketConnectionImpl.cc @@ -17,6 +17,7 @@ #include #include #include +#include using namespace drogon; @@ -327,8 +328,13 @@ bool WebSocketMessageParser::parse(trantor::MsgBuffer *buffer) { indexFirstMask = 10; } - if (indexFirstMask > 2 && buffer->readableBytes() >= indexFirstMask) + if (indexFirstMask > 2) { + if (buffer->readableBytes() < indexFirstMask) + { + // Not enough data yet, wait for more. + return true; + } if (isControlFrame) { // rfc6455-5.5 @@ -344,14 +350,17 @@ bool WebSocketMessageParser::parse(trantor::MsgBuffer *buffer) } else if (indexFirstMask == 10) { - length = (unsigned char)(*buffer)[2]; - length = (length << 8) + (unsigned char)(*buffer)[3]; - length = (length << 8) + (unsigned char)(*buffer)[4]; - length = (length << 8) + (unsigned char)(*buffer)[5]; - length = (length << 8) + (unsigned char)(*buffer)[6]; - length = (length << 8) + (unsigned char)(*buffer)[7]; - length = (length << 8) + (unsigned char)(*buffer)[8]; - length = (length << 8) + (unsigned char)(*buffer)[9]; + length = 0; + for (int i = 2; i <= 9; ++i) + { + if (length > ((std::numeric_limits::max)() >> 8)) + { + LOG_ERROR + << "Payload length too large to handle safely"; + return false; + } + length = (length << 8) + (unsigned char)(*buffer)[i]; + } } else { @@ -387,6 +396,11 @@ bool WebSocketMessageParser::parse(trantor::MsgBuffer *buffer) return true; } } + else + { + // Not enough data yet, wait for more. + return true; + } } else { @@ -401,6 +415,11 @@ bool WebSocketMessageParser::parse(trantor::MsgBuffer *buffer) return true; } } + else + { + // Not enough data yet, wait for more. + return true; + } } } return true;