Fix use after free crash in QQmlDelegateModel

When iterating over the cache in QQmlDelegateModel::_q_itemsInserted(),
_q_itemsRemoved, _q_itemsMoved, _q_modelReset() and _q_itemsMoved,
updating some of the item's modelIndex can trigger layout change in
the view, which might in turn remove a QQmlDelegateModelItem from
the cache, causing us to dereference an already deleted pointer.

To prevent a crash, we always check whether the item is still valid in
the original cache and skip it if it has been removed in the meanwhile.

This fix is similar to 5df747fc but reduces runtime impact by performing
the lookup only when d->m_cache has detached from the loop's copy.

Fixes: QTBUG-76254
Change-Id: I9d7e0118e64e9ec7d8efae04e6ae319804f31981
Reviewed-by: Richard Moe Gustavsen <richard.gustavsen@qt.io>
 

1 files changed, 16 insertions, 1 deletions

diff --git a/src/qml/types/qqmldelegatemodel.cpp b/src/qml/types/qqmldelegatemodel.cpp
index 572f58339f..63bc64d5e6 100644
--- a/src/qml/types/qqmldelegatemodel.cpp
+++ b/src/qml/types/qqmldelegatemodel.cpp
@@ -1344,6 +1344,11 @@ void QQmlDelegateModel::_q_itemsInserted(int index, int count)
     const QList<QQmlDelegateModelItem *> cache = d->m_cache;
     for (int i = 0, c = cache.count();  i < c; ++i) {
         QQmlDelegateModelItem *item = cache.at(i);
+        // layout change triggered by changing the modelIndex might have
+        // already invalidated this item in d->m_cache and deleted it.
+        if (!d->m_cache.isSharedWith(cache) && !d->m_cache.contains(item))
+            continue;
+
         if (item->modelIndex() >= index) {
             const int newIndex = item->modelIndex() + count;
             const int row = newIndex;
@@ -1487,7 +1492,7 @@ void QQmlDelegateModel::_q_itemsRemoved(int index, int count)
         QQmlDelegateModelItem *item = cache.at(i);
         // layout change triggered by removal of a previous item might have
         // already invalidated this item in d->m_cache and deleted it
-        if (!d->m_cache.contains(item))
+        if (!d->m_cache.isSharedWith(cache) && !d->m_cache.contains(item))
             continue;
 
         if (item->modelIndex() >= index + count) {
@@ -1542,6 +1547,11 @@ void QQmlDelegateModel::_q_itemsMoved(int from, int to, int count)
     const QList<QQmlDelegateModelItem *> cache = d->m_cache;
     for (int i = 0, c = cache.count();  i < c; ++i) {
         QQmlDelegateModelItem *item = cache.at(i);
+        // layout change triggered by changing the modelIndex might have
+        // already invalidated this item in d->m_cache and deleted it.
+        if (!d->m_cache.isSharedWith(cache) && !d->m_cache.contains(item))
+            continue;
+
         if (item->modelIndex() >= from && item->modelIndex() < from + count) {
             const int newIndex = item->modelIndex() - from + to;
             const int row = newIndex;
@@ -1634,6 +1644,11 @@ void QQmlDelegateModel::_q_modelReset()
         const QList<QQmlDelegateModelItem *> cache = d->m_cache;
         for (int i = 0, c = cache.count();  i < c; ++i) {
             QQmlDelegateModelItem *item = cache.at(i);
+            // layout change triggered by changing the modelIndex might have
+            // already invalidated this item in d->m_cache and deleted it.
+            if (!d->m_cache.isSharedWith(cache) && !d->m_cache.contains(item))
+                continue;
+
             if (item->modelIndex() != -1)
                 item->setModelIndex(-1, -1, -1);
         }