QBindable: Fix out-of-bound access in non-bindable property support code

QObjectPrivate::getPropertyAdaptorSlotObject called
connectionsForSignal.
Calling this function is only safe after it has been ensured beforehand that the vector has size > signalIndex. As getPropertyAdaptorSlotObject
is not supposed to modify the vector, it does not resize the vector and it could consequently end up with an out-of-bounds read.

To avoid that issue, we instead first check if the vector can
potentially contain an entry for the signal. If not, we simply return
nullptr, and avoid the call to connectionsForSignal.

The issue and its fix can be verified by running the modified
tst_qproperty test with ASAN enabled. The test is modified in the
following way:
- We first create a signal connection to a dummy slot. Otherwise,
  connections.loadRelaxed() would return a nullptr, and the problematic
  code would never be reached.
- We add enough signals to ensure that the fooChanged signal will
  actually be out of reach (which means >= 8 signals, as the initial
  capacity of the vector is 8)
Running the test without ASAN will most likely not result in a failure,
as then the out-of-bounds read will simply read garbage, and the most
likely result is that the cast below will fail.

Pick-to: 6.6 6.5
Change-Id: I18a3c4f52769c2b6491a685abb84f6fcfb44e4d8
Reviewed-by: Ivan Solovev <ivan.solovev@qt.io>

2 files changed, 14 insertions, 2 deletions

diff --git a/src/corelib/kernel/qobject.cpp b/src/corelib/kernel/qobject.cpp
index b7b0ec390e..4304add49c 100644
--- a/src/corelib/kernel/qobject.cpp
+++ b/src/corelib/kernel/qobject.cpp
@@ -5385,7 +5385,9 @@ QObjectPrivate::getPropertyAdaptorSlotObject(const QMetaProperty &property)
         Q_Q(QObject);
         const QMetaObject *metaObject = q->metaObject();
         int signal_index = methodIndexToSignalIndex(&metaObject, property.notifySignalIndex());
-        auto connectionList = conns->connectionsForSignal(signal_index);
+        if (signal_index >= conns->signalVectorCount())
+            return nullptr;
+        const auto connectionList = conns->connectionsForSignal(signal_index);
         for (auto c = connectionList.first.loadRelaxed(); c;
              c = c->nextConnectionList.loadRelaxed()) {
             if (c->isSlotObject) {
diff --git a/tests/auto/corelib/kernel/qproperty/tst_qproperty.cpp b/tests/auto/corelib/kernel/qproperty/tst_qproperty.cpp
index 5ac7b5fb51..acbeea4730 100644
--- a/tests/auto/corelib/kernel/qproperty/tst_qproperty.cpp
+++ b/tests/auto/corelib/kernel/qproperty/tst_qproperty.cpp
@@ -1711,6 +1711,14 @@ class PropertyAdaptorTester : public QObject
     Q_PROPERTY(int foo1 READ foo WRITE setFoo)
 
 signals:
+    void dummySignal1();
+    void dummySignal2();
+    void dummySignal3();
+    void dummySignal4();
+    void dummySignal5();
+    void dummySignal6();
+    void dummySignal7();
+    void dummySignal8();
     void fooChanged(int newFoo);
 
 public slots:
@@ -1739,9 +1747,11 @@ void tst_QProperty::propertyAdaptorBinding()
 
     // Check binding of non BINDABLE property
     PropertyAdaptorTester object;
+    // set up a dummy connection (needed to verify that the QBindable avoids an out-of-bounds read)
+    QObject::connect(&object, &PropertyAdaptorTester::dummySignal1, [](){});
+    QBindable<int> binding(&object, "foo");
     QObject::connect(&object, &PropertyAdaptorTester::fooChanged, &object,
                      &PropertyAdaptorTester::fooHasChanged);
-    QBindable<int> binding(&object, "foo");
     binding.setBinding([&]() { return source + 1; });
     QCOMPARE(object.foo(), 6);
     QCOMPARE(object.fooChangedCount, 1);