summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorMichael Stahl <mstahl@redhat.com>2017-06-30 12:43:21 +0200
committerMiklos Vajna <vmiklos@collabora.co.uk>2017-07-04 12:31:46 +0200
commitc6640f93273475b6d686f14820051cbfa4b1b6c2 (patch)
treede37779f8d0d8fd680e7aa2743b30d0c63467711
parent4ce1abe8e375dfcbcbaf4454b6de20231373ab40 (diff)
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. (cherry picked from commit a54ba50db2c341f0f0e47d77dbe64a6e588bc911) loplugin:unusedvariablecheck leftover from a54ba50db2c341f0f0e47d77dbe64a6e588bc911 "tdf#108863 svx: fix use- after-free in SdrEditView::DeleteMarkedObj()" (cherry picked from commit 77f98204ba2fbb51b0a0bb2ac94b839249eec9a4) Change-Id: I7ab18cb2116164a71dce29bf10eca974061ab424 Reviewed-on: https://gerrit.libreoffice.org/39418 Tested-by: Jenkins <ci@libreoffice.org> Reviewed-by: Miklos Vajna <vmiklos@collabora.co.uk>
-rw-r--r--include/svx/svdedtv.hxx3
-rw-r--r--svx/source/svdraw/svdedtv.cxx41
2 files changed, 25 insertions, 19 deletions
diff --git a/include/svx/svdedtv.hxx b/include/svx/svdedtv.hxx
index f849846f720a..51045e0b0856 100644
--- a/include/svx/svdedtv.hxx
+++ b/include/svx/svdedtv.hxx
@@ -160,7 +160,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 cdb208e571eb..521ebcfd3ef2 100644
--- a/svx/source/svdraw/svdedtv.cxx
+++ b/svx/source/svdraw/svdedtv.cxx
@@ -687,8 +687,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();
@@ -721,8 +722,6 @@ void SdrEditView::DeleteMarkedList(const SdrMarkList& rMark)
// make sure, OrderNums are correct:
rMark.GetMark(0)->GetMarkedSdrObj()->GetOrdNum();
- std::vector< SdrObject* > aRemoved3DObjects;
-
for(size_t nm = nMarkCount; nm > 0;)
{
--nm;
@@ -742,10 +741,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);
}
}
@@ -755,21 +752,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()
@@ -784,6 +782,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())
@@ -844,7 +843,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();
@@ -874,6 +877,8 @@ void SdrEditView::DeleteMarkedObj()
// end undo and change messaging moved at the end
EndUndo();
MarkListHasChanged();
+
+ lcl_LazyDelete(lazyDeleteObjects);
}
void SdrEditView::CopyMarkedObj()