Skip to content

Commit

Permalink
sc: fix use after free in ScChart2DataSequence::ExternalRefListener
Browse files Browse the repository at this point in the history 
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
    LibreOffice#1 ScChart2DataSequence::ExternalRefListener::~ExternalRefListener() sc/source/ui/unoobj/chart2uno.cxx:2897:26
    LibreOffice#4 ScChart2DataSequence::~ScChart2DataSequence() sc/source/ui/unoobj/chart2uno.cxx:2458:1
    LibreOffice#6 cppu::OWeakObject::release() cppuhelper/source/weak.cxx:230:9
    LibreOffice#8 com::sun::star::uno::Reference<com::sun::star::chart2::data::XDataSequence>::~Reference() include/com/sun/star/uno/Reference.hxx:114:22
    LibreOffice#9 chart::LabeledDataSequence::~LabeledDataSequence() chart2/source/tools/LabeledDataSequence.cxx:89:1
    LibreOffice#11 cppu::OWeakObject::release() cppuhelper/source/weak.cxx:230:9
    LibreOffice#12 cppu::WeakImplHelper<com::sun::star::chart2::data::XLabeledDataSequence2, com::sun::star::lang::XServiceInfo>::release() include/cppuhelper/implbase.hxx:115:66
    LibreOffice#13 com::sun::star::uno::Reference<com::sun::star::chart2::data::XLabeledDataSequence>::~Reference() include/com/sun/star/uno/Reference.hxx:114:22
    LibreOffice#20 chart::DataSeries::~DataSeries() chart2/source/model/main/DataSeries.cxx:218:1
    LibreOffice#24 chart::DataSeries::release() chart2/source/model/main/DataSeries.cxx:537:1
    LibreOffice#25 rtl::Reference<chart::DataSeries>::~Reference() include/rtl/ref.hxx:129:22
    LibreOffice#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
    LibreOffice#33 chart::ChartType::~ChartType() chart2/source/model/template/ChartType.cxx:65:19
    LibreOffice#34 chart::ColumnChartType::~ColumnChartType() chart2/source/model/template/ColumnChartType.cxx:134:2
    LibreOffice#36 cppu::OWeakObject::release() cppuhelper/source/weak.cxx:230:9
    LibreOffice#38 chart::ChartType::release() chart2/source/model/template/ChartType.cxx:330:1
    LibreOffice#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)
    LibreOffice#1 ScDocShell::~ScDocShell() sc/source/ui/docshell/docsh.cxx:2899:1
    LibreOffice#2 SvRefBase::ReleaseRef() include/tools/ref.hxx:163:29
    LibreOffice#3 tools::SvRef<SfxObjectShell>::~SvRef() include/tools/ref.hxx:56:36
    LibreOffice#4 IMPL_SfxBaseModel_DataContainer::~IMPL_SfxBaseModel_DataContainer() sfx2/source/doc/sfxbasemodel.cxx:245:5
    LibreOffice#12 SfxBaseModel::dispose() sfx2/source/doc/sfxbasemodel.cxx:757:13
    LibreOffice#13 SfxBaseModel::close(unsigned char) sfx2/source/doc/sfxbasemodel.cxx:1482:5
    LibreOffice#14 SfxBaseModel::dispose() sfx2/source/doc/sfxbasemodel.cxx:718:13
    LibreOffice#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.stahl@allotropia.de>
Reviewed-by: Michael Stahl <michael.stahl@allotropia.de>
(cherry picked from commit c87bbaa)
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/132183
Tested-by: Jenkins
Reviewed-by: Caolán McNamara <caolanm@redhat.com>
  • Loading branch information
@mistmist
mistmist authored and Caolán McNamara committed Mar 30, 2022
1 parent de78f74 commit 1d741aa
Show file tree
Hide file tree
Showing 5 changed files with 19 additions and 5 deletions.
2 changes: 1 addition & 1 deletion sc/inc/chartlis.hxx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ public:

ScChartListener& mrParent;
std::unordered_set<sal_uInt16> maFileIds;
ScDocument& mrDoc;
ScDocument* m_pDoc;
};

private:
Expand Down
2 changes: 1 addition & 1 deletion sc/inc/externalrefmgr.hxx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -368,7 +368,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
Expand Down
9 changes: 6 additions & 3 deletions sc/source/core/tool/chartlis.cxx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -50,18 +50,18 @@ class ScChartUnoData

// 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)
Expand All @@ -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;
}
}

Expand Down
8 changes: 8 additions & 0 deletions sc/source/ui/docshell/externalrefmgr.cxx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3080,6 +3080,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();

Expand Down
3 changes: 3 additions & 0 deletions sc/source/ui/unoobj/chart2uno.cxx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2915,6 +2915,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;
}
}

Expand Down

0 comments on commit 1d741aa

Please sign in to comment.