From 9038eec033843c289b06b83557a381a2648d8fa5 Mon Sep 17 00:00:00 2001 From: Vaxry Date: Tue, 3 Feb 2026 17:30:57 +0000 Subject: [PATCH] memory: fix a few UAF cases, add asan to test --- CMakeLists.txt | 8 +++--- include/hyprutils/memory/SharedPtr.hpp | 37 +++++++++++++------------- src/signal/Signal.cpp | 10 ++++--- tests/memory/Memory.cpp | 23 ++++++++++++++++ 4 files changed, 52 insertions(+), 26 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 750c7a3..a49c3fb 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -62,16 +62,18 @@ if(BUILD_TESTING) file(GLOB_RECURSE TESTFILES CONFIGURE_DEPENDS "tests/*.cpp") add_executable(hyprutils_tests ${TESTFILES}) - target_compile_options(hyprutils_tests PRIVATE --coverage) + target_compile_options(hyprutils_tests PRIVATE --coverage -fsanitize=address) target_link_options(hyprutils_tests PRIVATE --coverage) target_include_directories( hyprutils_tests PUBLIC "./include" PRIVATE "./src" "./src/include" "./protocols" "${CMAKE_BINARY_DIR}") - target_link_libraries(hyprutils_tests PRIVATE hyprutils GTest::gtest_main + target_link_libraries(hyprutils_tests PRIVATE asan hyprutils GTest::gtest_main PkgConfig::deps) - gtest_discover_tests(hyprutils_tests) + gtest_discover_tests(hyprutils_tests + PROPERTIES ENVIRONMENT "ASAN_OPTIONS=detect_leaks=0" + ) # Add coverage to hyprutils for test builds target_compile_options(hyprutils PRIVATE --coverage) diff --git a/include/hyprutils/memory/SharedPtr.hpp b/include/hyprutils/memory/SharedPtr.hpp index ee9172b..ccf1635 100644 --- a/include/hyprutils/memory/SharedPtr.hpp +++ b/include/hyprutils/memory/SharedPtr.hpp @@ -67,7 +67,7 @@ namespace Hyprutils { } ~CSharedPointer() { - decrement(); + decrement(impl_); } template @@ -75,7 +75,7 @@ namespace Hyprutils { if (impl_ == rhs.impl_) return *this; - decrement(); + decrement(impl_); impl_ = rhs.impl_; m_data = rhs.m_data; increment(); @@ -86,7 +86,7 @@ namespace Hyprutils { if (impl_ == rhs.impl_) return *this; - decrement(); + decrement(impl_); impl_ = rhs.impl_; m_data = rhs.m_data; increment(); @@ -133,9 +133,10 @@ namespace Hyprutils { } void reset() { - decrement(); - impl_ = nullptr; - m_data = nullptr; + auto ptr = impl_; + impl_ = nullptr; + m_data = nullptr; + decrement(ptr); } T* get() const { @@ -161,15 +162,15 @@ namespace Hyprutils { may delete the stored object if ref == 0 may delete and reset impl_ if ref == 0 and weak == 0 */ - void decrement() { - if (!impl_) + void decrement(Impl_::impl_base* base) { + if (!base) return; - impl_->dec(); + base->dec(); // if ref == 0, we can destroy impl - if (impl_->ref() == 0) - destroyImpl(); + if (base->ref() == 0) + destroyImpl(base); } /* no-op if there is no impl_ */ void increment() { @@ -181,15 +182,13 @@ namespace Hyprutils { /* destroy the pointed-to object if able, will also destroy impl */ - void destroyImpl() { - // destroy the impl contents - impl_->destroy(); + void destroyImpl(Impl_::impl_base* base) { + // this call can destroy this, so we need to not use thisptr anymore + base->destroy(); - // check for weak refs, if zero, we can also delete impl_ - if (impl_->wref() == 0) { - delete impl_; - impl_ = nullptr; - } + // check for weak refs, if zero, we can also delete base + if (base->wref() == 0) + delete base; } }; diff --git a/src/signal/Signal.cpp b/src/signal/Signal.cpp index 18c8f85..685f819 100644 --- a/src/signal/Signal.cpp +++ b/src/signal/Signal.cpp @@ -10,6 +10,10 @@ using namespace Hyprutils::Memory; #define WP CWeakPointer void Hyprutils::Signal::CSignalBase::emitInternal(void* args) { + + // Save, an event can destroy thisptr + const auto STATICS = m_vStaticListeners; + if (!m_vListeners.empty()) { std::vector> listeners; listeners.reserve(m_vListeners.size()); @@ -34,10 +38,8 @@ void Hyprutils::Signal::CSignalBase::emitInternal(void* args) { listeners.clear(); } - if (!m_vStaticListeners.empty()) { - const auto statics = m_vStaticListeners; - - for (const auto& l : statics) { + if (!STATICS.empty()) { + for (const auto& l : STATICS) { l->emitInternal(args); } } diff --git a/tests/memory/Memory.cpp b/tests/memory/Memory.cpp index ffb5022..99d91e2 100644 --- a/tests/memory/Memory.cpp +++ b/tests/memory/Memory.cpp @@ -236,6 +236,27 @@ static void testHierarchy() { } } +class CSelfDestruct { + public: + SP self; + + // + void youShouldKysNOW() { + self.reset(); + } +}; + +static void testSelfDestruct() { + auto x = makeShared(); + x->self = x; + WP weak = x; + x.reset(); + + // this has no EXPECT, because all we check is if there isn't a UAF here. + // if there is, asan will abort us + weak->youShouldKysNOW(); +} + TEST(Memory, memory) { SP intPtr = makeShared(10); SP intPtr2 = makeShared(-1337); @@ -291,4 +312,6 @@ TEST(Memory, memory) { testAtomicImpl(); testHierarchy(); + + testSelfDestruct(); }