tdf#108863 svx: fix use-after-free in SdrEditView::DeleteMarkedObj()

The sdr::ViewSelection has multiple vectors with pointers to the same
SdrObjects, and those are only cleared in
sdr::ViewSelection::SetEdgesOfMarkedNodesDirty(), so deleting SdrObjects
that are marked must be delayed until after that is called.

Change-Id: I7ab18cb2116164a71dce29bf10eca974061ab424

2 files changed, 25 insertions, 17 deletions

diff --git a/include/svx/svdedtv.hxx b/include/svx/svdedtv.hxx
index 81c680f1c085..039f56cae89b 100644
--- a/include/svx/svdedtv.hxx
+++ b/include/svx/svdedtv.hxx
@@ -154,7 +154,8 @@ protected:
 
     // Removes all objects of the MarkList from their ObjLists including Undo.
     // The entries in rMark remain.
-    void DeleteMarkedList(const SdrMarkList& rMark); // DeleteMarked -> DeleteMarkedList
+    // @return a list of objects that must be deleted after the outermost EndUndo
+    std::vector<SdrObject *> DeleteMarkedList(SdrMarkList const& rMark); // DeleteMarked -> DeleteMarkedList
 
     // Check possibilities of all marked objects
     virtual void CheckPossibilities();
diff --git a/svx/source/svdraw/svdedtv.cxx b/svx/source/svdraw/svdedtv.cxx
index 0382ffc77659..f3d5a046de14 100644
--- a/svx/source/svdraw/svdedtv.cxx
+++ b/svx/source/svdraw/svdedtv.cxx
@@ -678,8 +678,9 @@ void SdrEditView::ForceMarkedObjToAnotherPage()
     }
 }
 
-void SdrEditView::DeleteMarkedList(const SdrMarkList& rMark)
+std::vector<SdrObject*> SdrEditView::DeleteMarkedList(SdrMarkList const& rMark)
 {
+    std::vector<SdrObject*> ret;
     if (rMark.GetMarkCount()!=0)
     {
         rMark.ForceSort();
@@ -733,10 +734,8 @@ void SdrEditView::DeleteMarkedList(const SdrMarkList& rMark)
 
                 if( !bUndo )
                 {
-                    if( bIs3D )
-                        aRemoved3DObjects.push_back( pObj ); // may be needed later
-                    else
-                        SdrObject::Free(pObj);
+                    // tdf#108863 don't delete objects before EndUndo()
+                    ret.push_back(pObj);
                 }
             }
 
@@ -746,21 +745,22 @@ void SdrEditView::DeleteMarkedList(const SdrMarkList& rMark)
                 delete aUpdaters.back();
                 aUpdaters.pop_back();
             }
-
-            if( !bUndo )
-            {
-                // now delete removed scene objects
-                while(!aRemoved3DObjects.empty())
-                {
-                    SdrObject::Free( aRemoved3DObjects.back() );
-                    aRemoved3DObjects.pop_back();
-                }
-            }
         }
 
         if( bUndo )
             EndUndo();
     }
+    return ret;
+}
+
+static void lcl_LazyDelete(std::vector<SdrObject*> & rLazyDelete)
+{
+    // now delete removed scene objects
+    while (!rLazyDelete.empty())
+    {
+        SdrObject::Free( rLazyDelete.back() );
+        rLazyDelete.pop_back();
+    }
 }
 
 void SdrEditView::DeleteMarkedObj()
@@ -775,6 +775,7 @@ void SdrEditView::DeleteMarkedObj()
     BrkAction();
     BegUndo(ImpGetResStr(STR_EditDelete),GetDescriptionOfMarkedObjects(),SdrRepeatFunc::Delete);
 
+    std::vector<SdrObject*> lazyDeleteObjects;
     // remove as long as something is selected. This allows to schedule objects for
     // removal for a next run as needed
     while(GetMarkedObjectCount())
@@ -835,7 +836,11 @@ void SdrEditView::DeleteMarkedObj()
 
         // original stuff: remove selected objects. Handle clear will
         // do something only once
-        DeleteMarkedList(GetMarkedObjectList());
+        auto temp(DeleteMarkedList(GetMarkedObjectList()));
+        for (auto p : temp)
+        {
+            lazyDeleteObjects.push_back(p);
+        }
         GetMarkedObjectListWriteAccess().Clear();
         maHdlList.Clear();
 
@@ -865,6 +870,8 @@ void SdrEditView::DeleteMarkedObj()
     // end undo and change messaging moved at the end
     EndUndo();
     MarkListHasChanged();
+
+    lcl_LazyDelete(lazyDeleteObjects);
 }
 
 void SdrEditView::CopyMarkedObj()