include/svx/svdedtv.hxx | 3 ++- svx/source/svdraw/svdedtv.cxx | 41 +++++++++++++++++++++++------------------ 2 files changed, 25 insertions(+), 19 deletions(-)
New commits: commit c6640f93273475b6d686f14820051cbfa4b1b6c2 Author: Michael Stahl <[email protected]> Date: Fri Jun 30 12:43:21 2017 +0200 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 <[email protected]> Reviewed-by: Miklos Vajna <[email protected]> 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() _______________________________________________ Libreoffice-commits mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/libreoffice-commits
