From bffbae47f13c4a925995446b79544307671063cf Mon Sep 17 00:00:00 2001
From: Ulf Hermann <ulf.hermann@qt.io>
Date: Fri, 8 Sep 2017 12:55:34 +0200
Subject: [PATCH] QmlProfiler: Don't write past end in QmlEvent

If the external data is quint16_max long, we need to stop writing at
quint16_max - 1. So we cannot rely on unsigned integer overflow to
enforce the boundary.

Also, use for (...) rather than foreach (...). This should easily offset
the extra overhead introduced by the bounds check.

Change-Id: I51d1aef1040fbaa8396ca80ec7e30b2fe7b7dd0b
Reviewed-by: Christian Kandeler <christian.kandeler@qt.io>
---
 src/plugins/qmlprofiler/qmlevent.h             |  7 +++++--
 .../qmlprofiler/tests/qmlevent_test.cpp        | 18 ++++++++++++++++++
 src/plugins/qmlprofiler/tests/qmlevent_test.h  |  1 +
 3 files changed, 24 insertions(+), 2 deletions(-)

diff --git a/src/plugins/qmlprofiler/qmlevent.h b/src/plugins/qmlprofiler/qmlevent.h
index fdd3039a892..9c680e95bb9 100644
--- a/src/plugins/qmlprofiler/qmlevent.h
+++ b/src/plugins/qmlprofiler/qmlevent.h
@@ -290,9 +290,12 @@ private:
             m_dataType = static_cast<Type>(sizeof(Number) * 8);
             data = static_cast<Number *>(m_dataType & External ? m_data.external : &m_data);
         }
-        quint16 i = 0; // If you really have more than 64k items, this will wrap. Too bad.
-        foreach (Number item, numbers)
+        quint16 i = 0;
+        for (Number item : numbers) {
+            if (i >= m_dataLength)
+                break;
             data[i++] = item;
+        }
     }
 
     void clearPointer()
diff --git a/src/plugins/qmlprofiler/tests/qmlevent_test.cpp b/src/plugins/qmlprofiler/tests/qmlevent_test.cpp
index 4f32ca4eced..f834579054d 100644
--- a/src/plugins/qmlprofiler/tests/qmlevent_test.cpp
+++ b/src/plugins/qmlprofiler/tests/qmlevent_test.cpp
@@ -28,6 +28,8 @@
 #include <QList>
 #include <QQueue>
 
+#include <cstring>
+
 namespace QmlProfiler {
 namespace Internal {
 
@@ -116,6 +118,22 @@ void QmlEventTest::testNumbers()
     QCOMPARE(event.number<qint64>(5), 0LL);
 }
 
+void QmlEventTest::testMaxSize()
+{
+    const qint8 marker1 = 0xee;
+    const qint8 marker2 = 0xbb;
+    QmlEvent event;
+    QVarLengthArray<qint8> numbers(1 << 17);
+    std::memset(numbers.data(), 0, (1 << 17));
+    numbers[0] = marker1;
+    numbers[(1 << 16) - 2] = marker2;
+    event.setNumbers<QVarLengthArray<qint8>, qint8>(numbers);
+    const auto result = event.numbers<QVarLengthArray<qint8>, qint8>();
+    QCOMPARE(result.size(), (1 << 16) - 1);
+    QCOMPARE(result[0], marker1);
+    QCOMPARE(result[(1 << 16) - 2], marker2);
+}
+
 void QmlEventTest::testStreamOps()
 {
     QQueue<QmlEvent> sentEvents;
diff --git a/src/plugins/qmlprofiler/tests/qmlevent_test.h b/src/plugins/qmlprofiler/tests/qmlevent_test.h
index 429d4e1d6ec..3108d132b92 100644
--- a/src/plugins/qmlprofiler/tests/qmlevent_test.h
+++ b/src/plugins/qmlprofiler/tests/qmlevent_test.h
@@ -39,6 +39,7 @@ public:
 private slots:
     void testCtors();
     void testNumbers();
+    void testMaxSize();
     void testStreamOps();
 
 };