sw/source/uibase/dbui/dbmgr.cxx | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-)
New commits: commit c400c36018e8d89bbf8e551258a480c8f6571efe Author: Miklos Vajna <[email protected]> AuthorDate: Fri Nov 4 18:07:13 2022 +0100 Commit: Miklos Vajna <[email protected]> CommitDate: Fri Nov 4 21:07:03 2022 +0100 sw: fix heap-use-after-free in SwDBManager::MergeMailFiles() This went wrong in commit b1d13c48b57633c156e7d76fe81fc5828d1e3e20 (sw: fix merge mail multiple files, 2022-10-18). There were 3 problems here: 1) SwDBManager::MergeMailFiles() called SetDBManager() on an SwDoc, but that SwDoc has been already deleted by SfxObjectShellLock::operator=() that owns it. Fix the problem by switching our reference of that SwDoc to rtl::Reference<SwDoc> and then clearing that reference before the owning SwDocShell would be deleted. 2) Now that the SwDoc is either alive or is properly a nullptr, need to check if our SwDoc is nullptr before calling SetDBManager() on it. 3) Now that the doc shell is a SfxObjectShellRef and the doc is a rtl::Reference<SwDoc>, need to make sure that the doc is declared after the doc shell, since the SwDoc dtor will access its doc shell, but not the other way around. Change-Id: Ie10bac422a6d39b4401c90ecd67637b511e5b705 Reviewed-on: https://gerrit.libreoffice.org/c/core/+/142282 Tested-by: Jenkins Reviewed-by: Miklos Vajna <[email protected]> diff --git a/sw/source/uibase/dbui/dbmgr.cxx b/sw/source/uibase/dbui/dbmgr.cxx index d089d2bf3ab8..a62bd62b869b 100644 --- a/sw/source/uibase/dbui/dbmgr.cxx +++ b/sw/source/uibase/dbui/dbmgr.cxx @@ -173,7 +173,7 @@ static SfxObjectShell* lcl_CreateWorkingDocument( const WorkingDocType aType, const SwWrtShell &rSourceWrtShell, const vcl::Window *pSourceWindow, SwDBManager** const ppDBManager, - SwView** const pView, SwWrtShell** const pWrtShell, SwDoc** const pDoc ); + SwView** const pView, SwWrtShell** const pWrtShell, rtl::Reference<SwDoc>* const pDoc ); static bool lcl_getCountFromResultSet( sal_Int32& rCount, const SwDSParam* pParam ) { @@ -434,7 +434,7 @@ bool SwDBManager::Merge( const SwMergeDescriptor& rMergeDesc ) SfxObjectShellLock xWorkObjSh; SwWrtShell *pWorkShell = nullptr; - SwDoc *pWorkDoc = nullptr; + rtl::Reference<SwDoc> pWorkDoc; SwDBManager *pWorkDocOrigDBManager = nullptr; switch( rMergeDesc.nMergeType ) @@ -905,7 +905,7 @@ static SfxObjectShell* lcl_CreateWorkingDocument( // optional in and output to swap the DB manager SwDBManager** const ppDBManager, // optional output - SwView** const pView, SwWrtShell** const pWrtShell, SwDoc** const pDoc ) + SwView** const pView, SwWrtShell** const pWrtShell, rtl::Reference<SwDoc>* const pDoc ) { const SwDoc *pSourceDoc = rSourceWrtShell.GetDoc(); SfxObjectShellRef xWorkObjectShell = pSourceDoc->CreateCopy( true, (aType == WorkingDocType::TARGET) ); @@ -1178,8 +1178,8 @@ bool SwDBManager::MergeMailFiles(SwWrtShell* pSourceShell, SwView* pTargetView = rMergeDescriptor.pMailMergeConfigItem ? rMergeDescriptor.pMailMergeConfigItem->GetTargetView() : nullptr; SwWrtShell* pTargetShell = nullptr; - SwDoc* pTargetDoc = nullptr; SfxObjectShellRef xTargetDocShell; + rtl::Reference<SwDoc> pTargetDoc; std::unique_ptr< utl::TempFileNamed > aTempFile; sal_uInt16 nStartingPageNo = 0; @@ -1283,7 +1283,7 @@ bool SwDBManager::MergeMailFiles(SwWrtShell* pSourceShell, // it is more safe to use SfxObjectShellLock here SfxObjectShellLock xWorkDocSh; SwView* pWorkView = nullptr; - SwDoc* pWorkDoc = nullptr; + rtl::Reference<SwDoc> pWorkDoc; SwDBManager* pWorkDocOrigDBManager = nullptr; SwWrtShell* pWorkShell = nullptr; bool bWorkDocInitialized = false; @@ -1511,6 +1511,7 @@ bool SwDBManager::MergeMailFiles(SwWrtShell* pSourceShell, if( bCreateSingleFile || bIsPDFexport || bIsMultiFile) { pWorkDoc->SetDBManager( pWorkDocOrigDBManager ); + pWorkDoc.clear(); xWorkDocSh->DoClose(); xWorkDocSh = nullptr; } @@ -1543,7 +1544,8 @@ bool SwDBManager::MergeMailFiles(SwWrtShell* pSourceShell, Printer::FinishPrintJob( pWorkView->GetPrinterController()); if( !bIsPDFexport ) { - pWorkDoc->SetDBManager( pWorkDocOrigDBManager ); + if (pWorkDoc) + pWorkDoc->SetDBManager(pWorkDocOrigDBManager); if (xWorkDocSh.Is()) xWorkDocSh->DoClose(); }
