Fix use-after-free when removing elements from a ListModel

Detaching delegate instances from model items is done after the
destruction of said model items. The problem is that after the model
item is destroyed, it will emit a change/destroyed signal. As the
delegate is still referencing the item, this will result in a
use-after-free. To provent that, the items are kept around until after
everyone (notably the delegate model) has been notified of the removal.

[ChangeLog][Qt][Qml] Fix possible use-after-free when removing items from a ListModel through JavaScript.

Task-number: QTBUG-59256
Change-Id: Iee182e2cf0b50d3dda2181fed95e38f1a60f22a9
Reviewed-by: Simon Hausmann <simon.hausmann@qt.io>

1 files changed, 19 insertions, 6 deletions

diff --git a/src/qml/types/qqmllistmodel.cpp b/src/qml/types/qqmllistmodel.cpp
index 9d0f1afb32..b33be2b4fb 100644
--- a/src/qml/types/qqmllistmodel.cpp
+++ b/src/qml/types/qqmllistmodel.cpp
@@ -567,14 +567,20 @@ void ListModel::clear()
     elements.clear();
 }
 
-void ListModel::remove(int index, int count)
+QVector<std::function<void()>> ListModel::remove(int index, int count)
 {
+    QVector<std::function<void()>> toDestroy;
+    auto layout = m_layout;
     for (int i=0 ; i < count ; ++i) {
-        elements[index+i]->destroy(m_layout);
-        delete elements[index+i];
+        auto element = elements[index+i];
+        toDestroy.append([element, layout](){
+            element->destroy(layout);
+            delete element;
+        });
     }
     elements.remove(index, count);
     updateCacheIndices();
+    return toDestroy;
 }
 
 void ListModel::insert(int elementIndex, QV4::Object *object)
@@ -2053,15 +2059,22 @@ void QQmlListModel::remove(QQmlV4Function *args)
 
         emitItemsAboutToBeRemoved(index, removeCount);
 
+        QVector<std::function<void()>> toDestroy;
         if (m_dynamicRoles) {
-            for (int i=0 ; i < removeCount ; ++i)
-                delete m_modelObjects[index+i];
+            for (int i=0 ; i < removeCount ; ++i) {
+                auto modelObject = m_modelObjects[index+i];
+                toDestroy.append([modelObject](){
+                    delete modelObject;
+                });
+            }
             m_modelObjects.remove(index, removeCount);
         } else {
-            m_listModel->remove(index, removeCount);
+            toDestroy = m_listModel->remove(index, removeCount);
         }
 
         emitItemsRemoved(index, removeCount);
+        for (const auto &destroyer : toDestroy)
+            destroyer();
     } else {
         qmlWarning(this) << tr("remove: incorrect number of arguments");
     }