Tooling: Fix integer range checks

The packet protocol should check if the number of bytes to be read is
positive. Also, a Q_ASSERT on the return value of read() is too brutal.
The device can have failed for any number of reasons and we don't want
to crash the application because of that. Finally, the number of bytes
to be read includes the bytes read to determine the number. Make that
clearer by subtracting the actual count, not sizeof(qint32).

The check in QQmlProfilerTypedEvent is supposed to happen before we cast
the number to the more restrictive type. Furthermore, if subtype doesn't
fit the range constraint, we don't have to do anything at all as the
default rangeType is already set before.

Change-Id: I48c8c47e4207abae6e718eea97815d43e7f9d833
Reviewed-by: Simon Hausmann <simon.hausmann@qt.io>

2 files changed, 5 insertions, 10 deletions

diff --git a/src/plugins/qmltooling/packetprotocol/qpacketprotocol.cpp b/src/plugins/qmltooling/packetprotocol/qpacketprotocol.cpp
index 5460d617dd..1c73b9d7fb 100644
--- a/src/plugins/qmltooling/packetprotocol/qpacketprotocol.cpp
+++ b/src/plugins/qmltooling/packetprotocol/qpacketprotocol.cpp
@@ -45,8 +45,6 @@
 
 QT_BEGIN_NAMESPACE
 
-static const int MAX_PACKET_SIZE = 0x7FFFFFFF;
-
 /*!
   \class QPacketProtocol
   \internal
@@ -238,12 +236,10 @@ void QPacketProtocol::readyToRead()
                 return;
 
             // Read size header
-            int read = d->dev->read((char *)&d->inProgressSize, sizeof(qint32));
-            Q_ASSERT(read == sizeof(qint32));
-            Q_UNUSED(read);
+            const qint64 read = d->dev->read((char *)&d->inProgressSize, sizeof(qint32));
 
             // Check sizing constraints
-            if (d->inProgressSize > MAX_PACKET_SIZE) {
+            if (read != sizeof(qint32) || d->inProgressSize < read) {
                 disconnect(d->dev, &QIODevice::readyRead, this, &QPacketProtocol::readyToRead);
                 disconnect(d->dev, &QIODevice::aboutToClose, this, &QPacketProtocol::aboutToClose);
                 disconnect(d->dev, &QIODevice::bytesWritten, this, &QPacketProtocol::bytesWritten);
@@ -252,7 +248,7 @@ void QPacketProtocol::readyToRead()
                 return;
             }
 
-            d->inProgressSize -= sizeof(qint32);
+            d->inProgressSize -= read;
         } else {
             d->inProgress.append(d->dev->read(d->inProgressSize - d->inProgress.size()));
 
diff --git a/src/qmldebug/qqmlprofilertypedevent.cpp b/src/qmldebug/qqmlprofilertypedevent.cpp
index 31a36bd052..94591ba7e3 100644
--- a/src/qmldebug/qqmlprofilertypedevent.cpp
+++ b/src/qmldebug/qqmlprofilertypedevent.cpp
@@ -58,9 +58,8 @@ QDataStream &operator>>(QDataStream &stream, QQmlProfilerTypedEvent &event)
     RangeType rangeType = MaximumRangeType;
     if (!stream.atEnd()) {
         stream >> subtype;
-        rangeType = static_cast<RangeType>(subtype);
-        if (rangeType < 0 || rangeType > MaximumRangeType)
-            rangeType = MaximumRangeType;
+        if (subtype >= 0 && subtype < MaximumRangeType)
+            rangeType = static_cast<RangeType>(subtype);
     } else {
         subtype = -1;
     }