sc/inc/chartlis.hxx                      |    2 +-
 sc/inc/externalrefmgr.hxx                |    2 +-
 sc/source/core/tool/chartlis.cxx         |    9 ++++++---
 sc/source/ui/docshell/externalrefmgr.cxx |    8 ++++++++
 sc/source/ui/unoobj/chart2uno.cxx        |    3 +++
 5 files changed, 19 insertions(+), 5 deletions(-)

New commits:
commit c87bbaa87e2532c7601e8588d87de1dc4952f098
Author:     Michael Stahl <michael.st...@allotropia.de>
AuthorDate: Mon Mar 28 20:07:24 2022 +0200
Commit:     Michael Stahl <michael.st...@allotropia.de>
CommitDate: Tue Mar 29 10:49:40 2022 +0200

    sc: fix use after free in ScChart2DataSequence::ExternalRefListener
    
    UITest_chart: tdf122011.tdf122011.test_tdf122011
    
    ERROR: AddressSanitizer: heap-use-after-free on address 0x61e00007a13e at 
pc 0x7f9a88217e2b bp 0x7f9a901e7ab0 sp 0x7f9a901e7aa8
    READ of size 1 at 0x61e00007a13e thread T53
        #0 ScDocument::IsInDtorClear() const sc/inc/document.hxx:2421:56
        #1 ScChart2DataSequence::ExternalRefListener::~ExternalRefListener() 
sc/source/ui/unoobj/chart2uno.cxx:2897:26
        #4 ScChart2DataSequence::~ScChart2DataSequence() 
sc/source/ui/unoobj/chart2uno.cxx:2458:1
        #6 cppu::OWeakObject::release() cppuhelper/source/weak.cxx:230:9
        #8 
com::sun::star::uno::Reference<com::sun::star::chart2::data::XDataSequence>::~Reference()
 include/com/sun/star/uno/Reference.hxx:114:22
        #9 chart::LabeledDataSequence::~LabeledDataSequence() 
chart2/source/tools/LabeledDataSequence.cxx:89:1
        #11 cppu::OWeakObject::release() cppuhelper/source/weak.cxx:230:9
        #12 
cppu::WeakImplHelper<com::sun::star::chart2::data::XLabeledDataSequence2, 
com::sun::star::lang::XServiceInfo>::release() 
include/cppuhelper/implbase.hxx:115:66
        #13 
com::sun::star::uno::Reference<com::sun::star::chart2::data::XLabeledDataSequence>::~Reference()
 include/com/sun/star/uno/Reference.hxx:114:22
        #20 chart::DataSeries::~DataSeries() 
chart2/source/model/main/DataSeries.cxx:218:1
        #24 chart::DataSeries::release() 
chart2/source/model/main/DataSeries.cxx:537:1
        #25 rtl::Reference<chart::DataSeries>::~Reference() 
include/rtl/ref.hxx:129:22
        #32 std::__debug::vector<rtl::Reference<chart::DataSeries>, 
std::allocator<rtl::Reference<chart::DataSeries> > >::clear() 
/usr/bin/../lib/gcc/x86_64-redhat-linux/11/../../../../include/c++/11/debug/vector:720:9
        #33 chart::ChartType::~ChartType() 
chart2/source/model/template/ChartType.cxx:65:19
        #34 chart::ColumnChartType::~ColumnChartType() 
chart2/source/model/template/ColumnChartType.cxx:134:2
        #36 cppu::OWeakObject::release() cppuhelper/source/weak.cxx:230:9
        #38 chart::ChartType::release() 
chart2/source/model/template/ChartType.cxx:330:1
        #39 rtl::Reference<chart::ChartType>::~Reference() 
include/rtl/ref.hxx:129:22
        #46 chart::BaseCoordinateSystem::~BaseCoordinateSystem() 
chart2/source/model/main/BaseCoordinateSystem.cxx:185:1
        #47 chart::CartesianCoordinateSystem::~CartesianCoordinateSystem() 
chart2/source/model/main/CartesianCoordinateSystem.cxx:53:2
        #49 cppu::OWeakObject::release() cppuhelper/source/weak.cxx:230:9
        #51 chart::BaseCoordinateSystem::release() 
chart2/source/model/main/BaseCoordinateSystem.cxx:406:1
        #52 rtl::Reference<chart::BaseCoordinateSystem>::~Reference() 
include/rtl/ref.hxx:129:22
        #53 chart::VCoordinateSystem::~VCoordinateSystem() 
chart2/source/view/axes/VCoordinateSystem.cxx:89:1
        #55 chart::VCartesianCoordinateSystem::~VCartesianCoordinateSystem() 
chart2/source/view/axes/VCartesianCoordinateSystem.cxx:69:1
        #65 chart::ChartView::impl_deleteCoordinateSystems() 
chart2/source/view/main/ChartView.cxx:1091:20
        #66 chart::ChartView::~ChartView() 
chart2/source/view/main/ChartView.cxx:1085:5
        #68 cppu::OWeakObject::release() cppuhelper/source/weak.cxx:230:9
        #70 rtl::Reference<chart::ChartView>::~Reference() 
include/rtl/ref.hxx:129:22
        #71 chart::ChartModel::~ChartModel() 
chart2/source/model/main/ChartModel.cxx:183:1
        #73 cppu::OWeakObject::release() cppuhelper/source/weak.cxx:230:9
        #75 rtl::Reference<chart::ChartModel>::clear() 
include/rtl/ref.hxx:196:19
        #76 chart::CreationWizardUnoDlg::disposing() 
chart2/source/controller/dialogs/dlg_CreationWizard_UNO.cxx:235:19
        #77 cppu::OComponentHelper::dispose() 
cppuhelper/source/component.cxx:161:17
        #78 
chart::CreationWizardUnoDlg::notifyTermination(com::sun::star::lang::EventObject
 const&) chart2/source/controller/dialogs/dlg_CreationWizard_UNO.cxx:140:5
        #79 framework::Desktop::impl_sendNotifyTerminationEvent() 
framework/source/services/desktop.cxx:1649:79
        #80 framework::Desktop::terminate() 
framework/source/services/desktop.cxx:282:13
    
    0x61e00007a13e is located 2238 bytes inside of 2712-byte region 
[0x61e000079880,0x61e00007a318)
    freed by thread T53 here:
        #0 0x4fe267 in operator delete(void*) 
(instdir/program/soffice.bin+0x4fe267)
        #1 ScDocShell::~ScDocShell() sc/source/ui/docshell/docsh.cxx:2899:1
        #2 SvRefBase::ReleaseRef() include/tools/ref.hxx:163:29
        #3 tools::SvRef<SfxObjectShell>::~SvRef() include/tools/ref.hxx:56:36
        #4 IMPL_SfxBaseModel_DataContainer::~IMPL_SfxBaseModel_DataContainer() 
sfx2/source/doc/sfxbasemodel.cxx:245:5
        #12 SfxBaseModel::dispose() sfx2/source/doc/sfxbasemodel.cxx:757:13
        #13 SfxBaseModel::close(unsigned char) 
sfx2/source/doc/sfxbasemodel.cxx:1482:5
        #14 SfxBaseModel::dispose() sfx2/source/doc/sfxbasemodel.cxx:718:13
        #15 gcc3::callVirtualMethod(void*, unsigned int, void*, 
_typelib_TypeDescriptionReference*, bool, unsigned long*, unsigned int, 
unsigned long*, double*) 
bridges/source/cpp_uno/gcc3_linux_x86-64/callvirtualmethod.cxx:77:5
    
    Change-Id: I4ac7a702c50f9519a0f982ece9776c2d449c43ad
    Reviewed-on: https://gerrit.libreoffice.org/c/core/+/132242
    Tested-by: Michael Stahl <michael.st...@allotropia.de>
    Reviewed-by: Michael Stahl <michael.st...@allotropia.de>

diff --git a/sc/inc/chartlis.hxx b/sc/inc/chartlis.hxx
index 3273a61a1da2..7e351082f4f7 100644
--- a/sc/inc/chartlis.hxx
+++ b/sc/inc/chartlis.hxx
@@ -56,7 +56,7 @@ public:
 
         ScChartListener& mrParent;
         std::unordered_set<sal_uInt16> maFileIds;
-        ScDocument&                 mrDoc;
+        ScDocument* m_pDoc;
     };
 
 private:
diff --git a/sc/inc/externalrefmgr.hxx b/sc/inc/externalrefmgr.hxx
index 20931043bfd1..d31145046d52 100644
--- a/sc/inc/externalrefmgr.hxx
+++ b/sc/inc/externalrefmgr.hxx
@@ -374,7 +374,7 @@ public:
     typedef std::set<ScFormulaCell*>                      RefCellSet;
     typedef std::unordered_map<sal_uInt16, RefCellSet>         RefCellMap;
 
-    enum LinkUpdateType { LINK_MODIFIED, LINK_BROKEN };
+    enum LinkUpdateType { LINK_MODIFIED, LINK_BROKEN, 
OH_NO_WE_ARE_GOING_TO_DIE };
 
     /**
      * Base class for objects that need to listen to link updates.  When a
diff --git a/sc/source/core/tool/chartlis.cxx b/sc/source/core/tool/chartlis.cxx
index 18b69b12d92b..3566e357f67b 100644
--- a/sc/source/core/tool/chartlis.cxx
+++ b/sc/source/core/tool/chartlis.cxx
@@ -50,18 +50,18 @@ public:
 
 // ScChartListener
 ScChartListener::ExternalRefListener::ExternalRefListener(ScChartListener& 
rParent, ScDocument& rDoc) :
-    mrParent(rParent), mrDoc(rDoc)
+    mrParent(rParent), m_pDoc(&rDoc)
 {
 }
 
 ScChartListener::ExternalRefListener::~ExternalRefListener()
 {
-    if (mrDoc.IsInDtorClear())
+    if (!m_pDoc || m_pDoc->IsInDtorClear())
         // The document is being destroyed.  Do nothing.
         return;
 
     // Make sure to remove all pointers to this object.
-    mrDoc.GetExternalRefManager()->removeLinkListener(this);
+    m_pDoc->GetExternalRefManager()->removeLinkListener(this);
 }
 
 void ScChartListener::ExternalRefListener::notify(sal_uInt16 nFileId, 
ScExternalRefManager::LinkUpdateType eType)
@@ -79,6 +79,9 @@ void ScChartListener::ExternalRefListener::notify(sal_uInt16 
nFileId, ScExternal
         case ScExternalRefManager::LINK_BROKEN:
             removeFileId(nFileId);
         break;
+        case ScExternalRefManager::OH_NO_WE_ARE_GOING_TO_DIE:
+            m_pDoc = nullptr;
+        break;
     }
 }
 
diff --git a/sc/source/ui/docshell/externalrefmgr.cxx 
b/sc/source/ui/docshell/externalrefmgr.cxx
index 3d1845cbeacb..f03815fdaa16 100644
--- a/sc/source/ui/docshell/externalrefmgr.cxx
+++ b/sc/source/ui/docshell/externalrefmgr.cxx
@@ -3106,6 +3106,14 @@ void ScExternalRefManager::setFilterData(sal_uInt16 
nFileId, const OUString& rFi
 
 void ScExternalRefManager::clear()
 {
+    for (auto& rEntry : maLinkListeners)
+    {
+        for (auto& it : rEntry.second)
+        {
+            it->notify(0, OH_NO_WE_ARE_GOING_TO_DIE);
+        }
+    }
+
     for (auto& rEntry : maDocShells)
         rEntry.second.maShell->DoClose();
 
diff --git a/sc/source/ui/unoobj/chart2uno.cxx 
b/sc/source/ui/unoobj/chart2uno.cxx
index 447e3b2ae6d0..adc244bacc78 100644
--- a/sc/source/ui/unoobj/chart2uno.cxx
+++ b/sc/source/ui/unoobj/chart2uno.cxx
@@ -2916,6 +2916,9 @@ void 
ScChart2DataSequence::ExternalRefListener::notify(sal_uInt16 nFileId, ScExt
         case ScExternalRefManager::LINK_BROKEN:
             maFileIds.erase(nFileId);
         break;
+        case ScExternalRefManager::OH_NO_WE_ARE_GOING_TO_DIE:
+            mpDoc = nullptr;
+        break;
     }
 }
 

Reply via email to