From a39269e11e7c2d1843a4a4bf67749ae0224e4830 Mon Sep 17 00:00:00 2001 From: Simon Rettberg Date: Fri, 9 Sep 2016 16:31:04 +0200 Subject: Fix infinite loop when receiving incomplete headers --- src/client/net/serverconnection.cpp | 6 ++++-- src/client/net/serverdiscovery.cpp | 2 +- src/server/net/client.cpp | 6 ++++-- src/server/net/discoverylistener.cpp | 2 +- src/shared/networkmessage.cpp | 28 ++++++++++++++-------------- src/shared/networkmessage.h | 8 ++++++-- 6 files changed, 30 insertions(+), 22 deletions(-) diff --git a/src/client/net/serverconnection.cpp b/src/client/net/serverconnection.cpp index c6ffd8c..54d95bc 100644 --- a/src/client/net/serverconnection.cpp +++ b/src/client/net/serverconnection.cpp @@ -379,15 +379,17 @@ void ServerConnection::sock_dataArrival() return; } - while (_socket->bytesAvailable()) + while (_socket->bytesAvailable() > 0) { bool retval; retval = _fromServer.readMessage(_socket); // let the message read data from socket - if (!retval) // error parsing msg, disconnect client! + if (retval == NM_READ_FAILED) // error parsing msg, disconnect client! { this->disconnectFromServer(); return; } + if (retval == NM_READ_INCOMPLETE) + return; if (_fromServer.readComplete()) // message is complete { this->handleMsg(); diff --git a/src/client/net/serverdiscovery.cpp b/src/client/net/serverdiscovery.cpp index 5dba327..1d1e891 100644 --- a/src/client/net/serverdiscovery.cpp +++ b/src/client/net/serverdiscovery.cpp @@ -161,7 +161,7 @@ void ServerDiscovery::onUdpReadyRead() continue; _packet.reset(); - if (!_packet.readMessage(data, (quint32)size)) + if (_packet.readMessage(data, (quint32)size) != NM_READ_OK) continue; // Valid packet, process it: diff --git a/src/server/net/client.cpp b/src/server/net/client.cpp index e55fb3b..5086ea1 100644 --- a/src/server/net/client.cpp +++ b/src/server/net/client.cpp @@ -122,14 +122,16 @@ void Client::onDataArrival() } bool ret; - while (_socket->bytesAvailable()) + while (_socket->bytesAvailable() > 0) { ret = _fromClient.readMessage(_socket); // let the message read data from socket - if (!ret) // error parsing msg, disconnect client! + if (ret == NM_READ_FAILED) // error parsing msg, disconnect client! { this->disconnect(); return; } + if (ret == NM_READ_INCOMPLETE) + return; if (_fromClient.readComplete()) // message is complete { this->handleMsg(); diff --git a/src/server/net/discoverylistener.cpp b/src/server/net/discoverylistener.cpp index e205c77..892ca53 100644 --- a/src/server/net/discoverylistener.cpp +++ b/src/server/net/discoverylistener.cpp @@ -145,7 +145,7 @@ void DiscoveryListener::onReadyRead() } ++_packetCounter[bucket]; _packet.reset(); - if (!_packet.readMessage(data, (quint32)size)) + if (_packet.readMessage(data, (quint32)size) != NM_READ_OK) continue; // Valid packet, process it: const QByteArray iplist(_packet.getFieldBytes(_IPLIST)); diff --git a/src/shared/networkmessage.cpp b/src/shared/networkmessage.cpp index b0b82cd..6ab5d78 100644 --- a/src/shared/networkmessage.cpp +++ b/src/shared/networkmessage.cpp @@ -78,7 +78,7 @@ inline void NetworkMessage::allocBuffer() } } -bool NetworkMessage::readMessage(QAbstractSocket* socket) +int NetworkMessage::readMessage(QAbstractSocket* socket) { // Check/Set the _mode variable, so read and write calls are not mixed if (_mode != 1) @@ -86,7 +86,7 @@ bool NetworkMessage::readMessage(QAbstractSocket* socket) if (_mode != 0) { qDebug("NetworkMessage::readMessage(TCP) called when class was in mode %d!", _mode); - return false; + return NM_READ_FAILED; } _mode = 1; } @@ -94,15 +94,15 @@ bool NetworkMessage::readMessage(QAbstractSocket* socket) if (_bufferSize == 0) { if (socket->bytesAvailable() < HEADER_LEN) - return true; + return NM_READ_INCOMPLETE; char header[HEADER_LEN]; if (socket->read(header, HEADER_LEN) != HEADER_LEN) { qDebug("FIXME: Socket said 8 bytes available, but could not read 8..."); - return false; + return NM_READ_FAILED; } if (!this->parseHeader(header)) - return false; + return NM_READ_FAILED; //qDebug() << "Expecting message of " << _bufferSize << " bytes"; allocBuffer(); } @@ -115,7 +115,7 @@ bool NetworkMessage::readMessage(QAbstractSocket* socket) if (ret < 0) { qDebug("Socket read failed (TCP), return code %d", (int)ret); - return false; + return NM_READ_FAILED; } _bufferPos += ret; //qDebug() << "Buffer has now " << _bufferPos << " of " << _bufferSize << " bytes"; @@ -123,13 +123,13 @@ bool NetworkMessage::readMessage(QAbstractSocket* socket) if (_bufferSize == _bufferPos) { if (!this->parseMessage(_buffer)) - return false; + return NM_READ_FAILED; } } - return true; + return NM_READ_OK; } -bool NetworkMessage::readMessage(char* data, quint32 len) +int NetworkMessage::readMessage(char* data, quint32 len) { // Check/Set the _mode variable, so read and write calls are not mixed if (_mode != 1) @@ -137,23 +137,23 @@ bool NetworkMessage::readMessage(char* data, quint32 len) if (_mode != 0) { qDebug("NetworkMessage::readMessage(UDP) called when class was in mode %d!", _mode); - return false; + return NM_READ_FAILED; } _mode = 1; } if (len < HEADER_LEN) { qDebug("UDP message shorter than 8 bytes. ignored."); - return false; + return NM_READ_FAILED; } if (!this->parseHeader(data)) - return false; + return NM_READ_FAILED; if (len != _bufferSize + HEADER_LEN) { qDebug("UDP packet has wrong size. Is %d, expected %d", (int)_bufferSize, len - HEADER_LEN); - return false; + return NM_READ_FAILED; } - return this->parseMessage(data + HEADER_LEN); + return this->parseMessage(data + HEADER_LEN) ? NM_READ_OK : NM_READ_FAILED; } bool NetworkMessage::parseHeader(char *header) diff --git a/src/shared/networkmessage.h b/src/shared/networkmessage.h index 3742890..2e8c1c5 100644 --- a/src/shared/networkmessage.h +++ b/src/shared/networkmessage.h @@ -8,6 +8,10 @@ #ifndef NETWORKMESSAGE_H_ #define NETWORKMESSAGE_H_ +#define NM_READ_OK (1) +#define NM_READ_INCOMPLETE (2) +#define NM_READ_FAILED (0) + #include class QAbstractSocket; @@ -56,8 +60,8 @@ private: public: NetworkMessage(); virtual ~NetworkMessage(); - bool readMessage(QAbstractSocket* socket); - bool readMessage(char* data, quint32 len); + int readMessage(QAbstractSocket* socket); + int readMessage(char* data, quint32 len); bool writeMessage(QAbstractSocket * const socket); bool writeMessage(QUdpSocket* socket, const QHostAddress& address, quint16 port); void reset() { _fields.clear(); _bufferSize = 0; _mode = 0; } -- cgit v1.2.3-55-g7522