commit c400c36018e8d89bbf8e551258a480c8f6571efe
author Miklos Vajna <vmiklos@collabora.com> Fri Nov 04 18:07:13 2022 +0100
committer Miklos Vajna <vmiklos@collabora.com> Fri Nov 04 21:07:03 2022 +0100
tree ab174914aedf936c679560b6e652140526812cbc
parent 6e4336bcd26911fdb20051b58050d24c141ce372

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 <vmiklos@collabora.com>

sw/source/uibase/dbui/dbmgr.cxx

diff --git a/sw/source/uibase/dbui/dbmgr.cxx b/sw/source/uibase/dbui/dbmgr.cxx
index d089d2b..a62bd62 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();
            }