Verify input buffer

So far, the implementation relied on the datagrams received to be
correct. This can cause buffer overwrites, including crashes or worse.
While unlikely, verify that requested read size matches the existing
buffer.

Change-Id: I695318142348e58b3b9568135896c52da75109e7
Reviewed-by: Alex Blasche <alexander.blasche@qt.io>

1 files changed, 15 insertions, 5 deletions

diff --git a/src/mqtt/qmqttconnection.cpp b/src/mqtt/qmqttconnection.cpp
index 150bbf4..61a6ef0 100644
--- a/src/mqtt/qmqttconnection.cpp
+++ b/src/mqtt/qmqttconnection.cpp
@@ -79,7 +79,8 @@ QString QMqttConnection::readBufferTyped(qint64 *dataSize)
     const quint16 size = readBufferTyped<quint16>(dataSize);
     if (dataSize)
         *dataSize -= size;
-    return QString::fromUtf8(reinterpret_cast<const char *>(readBuffer(size).constData()), size);
+    const QByteArray ba = readBuffer(size);
+    return QString::fromUtf8(reinterpret_cast<const char *>(ba.constData()), ba.size());
 }
 
 template<>
@@ -88,7 +89,7 @@ QByteArray QMqttConnection::readBufferTyped(qint64 *dataSize)
     const quint16 size = readBufferTyped<quint16>(dataSize);
     if (dataSize)
         *dataSize -= size;
-    return QByteArray(reinterpret_cast<const char *>(readBuffer(size).constData()), size);
+    return readBuffer(size);
 }
 
 QMqttConnection::QMqttConnection(QObject *parent) : QObject(parent)
@@ -303,13 +304,13 @@ bool QMqttConnection::sendControlConnect()
     if (m_clientPrivate->m_password.size())
         packet.append(m_clientPrivate->m_password.toUtf8());
 
+    m_internalState = BrokerWaitForConnectAck;
+    m_missingData = 0;
+
     if (!writePacketToTransport(packet)) {
         qCDebug(lcMqttConnection) << "Could not write CONNECT frame to transport.";
         return false;
     }
-
-    m_internalState = BrokerWaitForConnectAck;
-    m_missingData = 0;
     return true;
 }
 
@@ -695,6 +696,10 @@ void QMqttConnection::transportError(QAbstractSocket::SocketError e)
 
 void QMqttConnection::readBuffer(char *data, quint64 size)
 {
+    if (Q_UNLIKELY(quint64(m_readBuffer.size() - m_readPosition) < size)) {
+        qCDebug(lcMqttConnection) << "Reaching out of buffer, protocol violation";
+        closeConnection(QMqttClient::ProtocolViolation);
+    }
     memcpy(data, m_readBuffer.constData() + m_readPosition, size);
     m_readPosition += size;
 }
@@ -737,6 +742,11 @@ void QMqttConnection::closeConnection(QMqttClient::ClientError error)
 
 QByteArray QMqttConnection::readBuffer(quint64 size)
 {
+    if (Q_UNLIKELY(quint64(m_readBuffer.size() - m_readPosition) < size)) {
+        qCDebug(lcMqttConnection) << "Reaching out of buffer, protocol violation";
+        closeConnection(QMqttClient::ProtocolViolation);
+        return QByteArray();
+    }
     QByteArray res(m_readBuffer.constData() + m_readPosition, int(size));
     m_readPosition += size;
     return res;