[Libreoffice-commits] core.git: include/xmloff sc/source xmloff/source

[Caolán McNamara]

 include/xmloff/xmlimp.hxx         |    2 ++
 sc/source/filter/xml/xmlimprt.cxx |    7 +++++--
 xmloff/source/core/xmlimp.cxx     |    7 ++++++-
 3 files changed, 13 insertions(+), 3 deletions(-)

New commits:
commit 81c7311609d5c9b5ebf7348e805276a8864dadcf
Author: Caolán McNamara <caolanm at redhat.com>
Date:   Tue Sep 19 20:41:28 2017 +0100

    ScXMLImport dtor deleted pSolarMutex before SvXMLImport dtor is called
    
    so if there are still ScXMLDatabaseRangesContext on the import stack,
    i.e. exception thrown during ScXMLDatabaseRanges import, then
    stuff crashes
    
    =================================================================
    ==23249==ERROR: AddressSanitizer: heap-use-after-free
    READ of size 8 at 0x6020001c3190 thread T0
        SolarMutexGuard::~SolarMutexGuard() /src/libreoffice/include/vcl/svapp.hxx:1420:9
        ScXMLImport::UnlockSolarMutex() /src/libreoffice/sc/source/filter/xml/xmlimprt.cxx:1916:13
        ScXMLDatabaseRangesContext::~ScXMLDatabaseRangesContext() /src/libreoffice/sc/source/filter/xml/xmldrani.cxx:64:19
        ScXMLDatabaseRangesContext::~ScXMLDatabaseRangesContext() /src/libreoffice/sc/source/filter/xml/xmldrani.cxx:63:1
        destroy /usr/local/bin/../include/c++/v1/memory:1838:64
        __destroy<rtl::Reference<SvXMLImportContext> > /usr/local/bin/../include/c++/v1/memory:1706
        destroy<rtl::Reference<SvXMLImportContext> > /usr/local/bin/../include/c++/v1/memory:1574
        std::__1::deque<rtl::Reference<SvXMLImportContext>, std::__1::allocator<rtl::Reference<SvXMLImportContext> > >::pop_back() /usr/local/bin/../include/c++/v1/deque:2604
        pop /usr/local/bin/../include/c++/v1/stack:212:19
        SvXMLImport::~SvXMLImport() /src/libreoffice/xmloff/source/core/xmlimp.cxx:418
        ScXMLImport::~ScXMLImport() /src/libreoffice/sc/source/filter/xml/xmlimprt.cxx:769:1
    
    freed by thread T0 here:
        #0 0x6164a0 in operator delete(void*) /src/llvm/projects/compiler-rt/lib/asan/asan_new_delete.cc:126
        #1 0x6254e3 in ScXMLImport::~ScXMLImport() /src/libreoffice/sc/source/filter/xml/xmlimprt.cxx:787:5
        #2 0x6259cc in ScXMLImport::~ScXMLImport() /src/libreoffice/sc/source/filter/xml/xmlimprt.cxx:769:1
    
    Change-Id: Ia72284f99e2e2efe4f1f9e57b1483c0f1333274d
    Reviewed-on: https://gerrit.libreoffice.org/42502
    Tested-by: Jenkins <ci at libreoffice.org>
    Reviewed-by: Caolán McNamara <caolanm at redhat.com>
    Tested-by: Caolán McNamara <caolanm at redhat.com>

diff --git a/include/xmloff/xmlimp.hxx b/include/xmloff/xmlimp.hxx
index 390c3d511349..b6552a297843 100644
--- a/include/xmloff/xmlimp.hxx
+++ b/include/xmloff/xmlimp.hxx
@@ -281,6 +281,8 @@ public:
         OUString const & implementationName,
         SvXMLImportFlags nImportFlags = SvXMLImportFlags::ALL );
 
+    void cleanup() throw();
+
     virtual ~SvXMLImport() throw() override;
 
     // css::xml::sax::XDocumentHandler
diff --git a/sc/source/filter/xml/xmlimprt.cxx b/sc/source/filter/xml/xmlimprt.cxx
index 188a687d20cf..7bc07d66abb4 100644
--- a/sc/source/filter/xml/xmlimprt.cxx
+++ b/sc/source/filter/xml/xmlimprt.cxx
@@ -782,12 +782,15 @@ ScXMLImport::~ScXMLImport() throw()
     delete pStyleNumberFormats;
     delete pStylesImportHelper;
 
-    delete pSolarMutexGuard;
-
     delete m_pMyNamedExpressions;
     delete pMyLabelRanges;
     delete pValidations;
     delete pDetectiveOpArray;
+
+    //call SvXMLImport dtor contents before deleting pSolarMutexGuard
+    cleanup();
+
+    delete pSolarMutexGuard;
 }
 
 void ScXMLImport::initialize( const css::uno::Sequence<css::uno::Any>& aArguments )
diff --git a/xmloff/source/core/xmlimp.cxx b/xmloff/source/core/xmlimp.cxx
index 724dd5c88e80..883e471b3d9a 100644
--- a/xmloff/source/core/xmlimp.cxx
+++ b/xmloff/source/core/xmlimp.cxx
@@ -408,7 +408,7 @@ SvXMLImport::SvXMLImport(
     registerNamespaces();
 }
 
-SvXMLImport::~SvXMLImport() throw ()
+void SvXMLImport::cleanup() throw ()
 {
     if (mxEventListener.is() && mxModel.is())
         mxModel->removeEventListener(mxEventListener);
@@ -419,6 +419,11 @@ SvXMLImport::~SvXMLImport() throw ()
     DisposingModel();
 }
 
+SvXMLImport::~SvXMLImport() throw ()
+{
+    cleanup();
+}
+
 namespace
 {
     class theSvXMLImportUnoTunnelId : public rtl::Static< UnoTunnelIdInit, theSvXMLImportUnoTunnelId> {};