From 4822af4f41a8b39bf631f9f02e1a35f40a76ecc5 Mon Sep 17 00:00:00 2001
From: Fabian Kosmale <fabian.kosmale@qt.io>
Date: Mon, 20 Feb 2023 11:23:09 +0100
Subject: [PATCH] QObjectBindableProperty: Avoid use-after-free in
 notifyObservers

We so far refetched the first observer after evaluating bindings, as
binding evaluating might change the list of observers.
However, that approach did not take into account that the 'this' pointer
might no longer be valid after binding evaluation: In case of a
QObjectBindableProperty (or a QObjectCompatProperty), binding evaluation
might cause a reallocation of the binding storage, and consequently the
invalidation of the QPropertyBindingData.
Fix this by refetching the QPropertyBindingData from the storage (if a
storage has been provided, which is always the case for the affected
classes).

Fixes: QTBUG-111268
Change-Id: Ie7e143a0bbb18f1c3f88a81dd9b31e6af463584f
Reviewed-by: Ivan Solovev <ivan.solovev@qt.io>
Reviewed-by: Ulf Hermann <ulf.hermann@qt.io>
(cherry picked from commit b124171309b259d429bd064fe3bfdec148869ef4)
Reviewed-by: Qt Cherry-pick Bot <cherrypick_bot@qt-project.org>
---
 src/corelib/kernel/qproperty.cpp               | 10 +++++++++-
 .../corelib/kernel/qproperty/tst_qproperty.cpp | 18 ++++++++++++++++++
 2 files changed, 27 insertions(+), 1 deletion(-)

diff --git a/src/corelib/kernel/qproperty.cpp b/src/corelib/kernel/qproperty.cpp
index 9ae78669b9e..26a18545da9 100644
--- a/src/corelib/kernel/qproperty.cpp
+++ b/src/corelib/kernel/qproperty.cpp
@@ -603,7 +603,15 @@ void QPropertyBindingData::notifyObservers(QUntypedPropertyData *propertyDataPtr
     PendingBindingObserverList bindingObservers;
     if (QPropertyObserverPointer observer = d.firstObserver()) {
         if (notifyObserver_helper(propertyDataPtr, storage, observer, bindingObservers) == Evaluated) {
-            // evaluateBindings() can trash the observers. We need to re-fetch here.
+            /* evaluateBindings() can trash the observers. We need to re-fetch here.
+             "this" might also no longer be valid in case we have a QObjectBindableProperty
+             and consequently d isn't either (this happens when binding evaluation has
+             caused the binding storage to resize.
+             If storage is nullptr, then there is no dynamically resizable storage,
+             and we cannot run into the issue.
+            */
+            if (storage)
+                d = QPropertyBindingDataPointer {storage->bindingData(propertyDataPtr)};
             if (QPropertyObserverPointer observer = d.firstObserver())
                 observer.notifyOnlyChangeHandler(propertyDataPtr);
             for (auto &&bindingObserver: bindingObservers)
diff --git a/tests/auto/corelib/kernel/qproperty/tst_qproperty.cpp b/tests/auto/corelib/kernel/qproperty/tst_qproperty.cpp
index 18bdcb9dc75..56da58c2914 100644
--- a/tests/auto/corelib/kernel/qproperty/tst_qproperty.cpp
+++ b/tests/auto/corelib/kernel/qproperty/tst_qproperty.cpp
@@ -67,6 +67,7 @@ private slots:
     void quntypedBindableApi();
     void readonlyConstQBindable();
     void qobjectBindableManualNotify();
+    void qobjectBindableReallocatedBindingStorage();
     void qobjectBindableSignalTakingNewValue();
 
     void testNewStuff();
@@ -1184,6 +1185,23 @@ void tst_QProperty::qobjectBindableManualNotify()
     QCOMPARE(object.foo(), 1);
 }
 
+
+struct ReallocObject : QObject {
+    ReallocObject()
+    { v.setBinding([this] { return x.value() + y.value() + z.value(); }); }
+    Q_OBJECT_BINDABLE_PROPERTY(ReallocObject, int, v)
+    Q_OBJECT_BINDABLE_PROPERTY(ReallocObject, int, x)
+    Q_OBJECT_BINDABLE_PROPERTY(ReallocObject, int, y)
+    Q_OBJECT_BINDABLE_PROPERTY(ReallocObject, int, z)
+};
+
+void tst_QProperty::qobjectBindableReallocatedBindingStorage()
+{
+    ReallocObject object;
+    object.x = 1;
+    QCOMPARE(object.v.value(), 1);
+}
+
 void tst_QProperty::qobjectBindableSignalTakingNewValue()
 {
     // Given an object of type MyQObject,