From 2cb54449b4fc9e908aae0a44f808d82b8e83acfc Mon Sep 17 00:00:00 2001 From: Michael Stahl Date: Mon, 12 Oct 2020 12:19:55 +0200 Subject: sdext: fix use-after-free on global AccessibleFocusManager The problem is that the destructor of the vector maFocusableObjects ends up dispose()-ing every element, which calls back into AccessibleFocusManager to remove the element from the vector, which invokes its destructor a 2nd time. Move it to the stack so it doesn't double-free itself. ERROR: AddressSanitizer: heap-use-after-free on address 0x612001571c00 at pc 0x7fc5e723ca72 bp 0x7fffbaa8d6d0 sp 0x7fffbaa8d6c8 READ of size 1 at 0x612001571c00 thread T0 #0 0x7fc5e723ca71 in cppu::WeakComponentImplHelperBase::release() cppuhelper/source/implbase.cxx:84:9 #1 0x7fc595211b27 in cppu::PartialWeakComponentImplHelper::release() include/cppuhelper/compbase.hxx:86:36 #2 0x7fc5952093e4 in rtl::Reference::~Reference() include/rtl/ref.hxx:113:22 #3 0x7fc59522acd4 in void std::_Destroy >(rtl::Reference*) /usr/bin/../lib/gcc/x86_64-redhat-linux/10/../../../../include/c++/10/bits/stl_construct.h:140:19 0x612001571c00 is located 64 bytes inside of 312-byte region [0x612001571bc0,0x612001571cf8) freed by thread T0 here: #0 0x4be997 in free (instdir/program/soffice.bin+0x4be997) #1 0x7fc5ea2a5104 in rtl_freeMemory sal/rtl/alloc_global.cxx:51:5 #2 0x7fc5952097f4 in cppu::WeakComponentImplHelperBase::operator delete(void*) include/cppuhelper/compbase_ex.hxx:66:11 #3 0x7fc595211e07 in sdext::presenter::PresenterAccessible::AccessibleObject::~AccessibleObject() sdext/source/presenter/PresenterAccessibility.cxx:67:28 #4 0x7fc5e74a11b4 in cppu::OWeakObject::release() cppuhelper/source/weak.cxx:233:9 #5 0x7fc5e723cb05 in cppu::WeakComponentImplHelperBase::release() cppuhelper/source/implbase.cxx:86:18 #6 0x7fc595211b27 in cppu::PartialWeakComponentImplHelper::release() include/cppuhelper/compbase.hxx:86:36 #7 0x7fc5e7194115 in com::sun::star::uno::Reference::~Reference() include/com/sun/star/uno/Reference.hxx:110:22 #8 0x7fc5e71f3944 in com::sun::star::lang::EventObject::~EventObject() workdir/UnoApiHeadersTarget/udkapi/comprehensive/com/sun/star/lang/EventObject.hdl:18:27 #9 0x7fc5e723d395 in cppu::WeakComponentImplHelperBase::dispose() cppuhelper/source/implbase.cxx:118:5 #10 0x7fc595211e27 in cppu::PartialWeakComponentImplHelper::dispose() include/cppuhelper/compbase.hxx:90:36 #11 0x7fc5e723c6e9 in cppu::WeakComponentImplHelperBase::release() cppuhelper/source/implbase.cxx:79:13 #12 0x7fc595211b27 in cppu::PartialWeakComponentImplHelper::release() include/cppuhelper/compbase.hxx:86:36 #13 0x7fc5952093e4 in rtl::Reference::~Reference() include/rtl/ref.hxx:113:22 #14 0x7fc59522acd4 in void std::_Destroy >(rtl::Reference*) /usr/bin/../lib/gcc/x86_64-redhat-linux/10/../../../../include/c++/10/bits/stl_construct.h:140:19 Change-Id: I95151807e9182ed5f43b63792fba86f83ee0bad8 Reviewed-on: https://gerrit.libreoffice.org/c/core/+/104208 Tested-by: Jenkins Reviewed-by: Michael Stahl --- sdext/source/presenter/PresenterAccessibility.cxx | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) (limited to 'sdext/source') diff --git a/sdext/source/presenter/PresenterAccessibility.cxx b/sdext/source/presenter/PresenterAccessibility.cxx index 951a18c5fca5..936c75df35f7 100644 --- a/sdext/source/presenter/PresenterAccessibility.cxx +++ b/sdext/source/presenter/PresenterAccessibility.cxx @@ -451,9 +451,12 @@ public: void FocusObject (const ::rtl::Reference& rpObject); + ~AccessibleFocusManager(); + private: static std::shared_ptr mpInstance; ::std::vector > maFocusableObjects; + bool m_isInDtor = false; AccessibleFocusManager(); }; @@ -1810,10 +1813,18 @@ std::shared_ptr const & AccessibleFocusManager::Instance } AccessibleFocusManager::AccessibleFocusManager() - : maFocusableObjects() { } +AccessibleFocusManager::~AccessibleFocusManager() +{ + // copy member to stack, then drop it - otherwise will get use-after-free + // from AccessibleObject::disposing(), it will call ~Reference *twice* + auto const temp(std::move(maFocusableObjects)); + (void) temp; + m_isInDtor = true; +} + void AccessibleFocusManager::AddFocusableObject ( const ::rtl::Reference& rpObject) { @@ -1833,7 +1844,7 @@ void AccessibleFocusManager::RemoveFocusableObject ( maFocusableObjects.erase(iObject); else { - OSL_ASSERT(iObject!=maFocusableObjects.end()); + OSL_ASSERT(m_isInDtor); // in dtor, was removed already } } -- cgit v1.2.3