diff options
author | Michael Stahl <mstahl@redhat.com> | 2017-06-30 12:43:21 +0200 |
---|---|---|
committer | Miklos Vajna <vmiklos@collabora.co.uk> | 2017-07-04 12:31:46 +0200 |
commit | c6640f93273475b6d686f14820051cbfa4b1b6c2 (patch) | |
tree | de37779f8d0d8fd680e7aa2743b30d0c63467711 | |
parent | 4ce1abe8e375dfcbcbaf4454b6de20231373ab40 (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.hxx | 3 | ||||
-rw-r--r-- | svx/source/svdraw/svdedtv.cxx | 41 |
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() |