From a54ba50db2c341f0f0e47d77dbe64a6e588bc911 Mon Sep 17 00:00:00 2001
From: Michael Stahl <mstahl@redhat.com>
Date: Fri, 30 Jun 2017 12:43:21 +0200
Subject: 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
---
 svx/source/svdraw/svdedtv.cxx | 39 +++++++++++++++++++++++----------------
 1 file changed, 23 insertions(+), 16 deletions(-)

(limited to 'svx/source/svdraw/svdedtv.cxx')

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()
-- 
cgit