From f160d52babaac78404f7c4cc5d6bba19a835aaab Mon Sep 17 00:00:00 2001 From: Dobroslaw Kijowski Date: Fri, 1 Sep 2023 13:19:51 +0200 Subject: [PATCH] [pipewire] Fix heap-use-after-free in AE::SINK::CAESinkPipewire::EnumerateDevicesEx * Heap-use-after-free [1] happens when EnumerateDevicesEx calls `GetName` on the registry instance. The string view containing `m_name` in CPipewireGlobal has been already freed by the pipewire library in `connection_ensure_size` function [2]. * In order to mitigate the issue copy the strings returned from pipewire. [1]: ================================================================= ==14082==ERROR: AddressSanitizer: heap-use-after-free on address 0x633000010e60 at pc 0x7effc8461003 bp 0x7effa7bb1e50 sp 0x7effa7bb15f8 READ of size 55 at 0x633000010e60 thread T19 #0 0x7effc8461002 in __interceptor_memcpy /usr/src/debug/gcc/gcc/libsanitizer/sanitizer_common/sanitizer_common_interceptors.inc:899 #1 0x7effc6f11222 in std::__cxx11::basic_string, std::allocator >::_M_mutate(unsigned long, unsigned long, char const*, unsigned long) (/usr/lib/libtinyxml.so.0+0xf222) (BuildId: 2f5d236264d4d695dbe432f41e1eb46c7bc2d5d4) #2 0x7effc575a8eb in std::__cxx11::basic_string, std::allocator >::_M_replace(unsigned long, unsigned long, char const*, unsigned long) /usr/src/debug/gcc/gcc-build/x86_64-pc-linux-gnu/libstdc++-v3/include/bits/basic_string.tcc:543 #3 0x55921037c9e7 in std::enable_if > const&, std::basic_string_view > >, std::__not_ > const*, std::__cxx11::basic_string, std::allocator > const*> >, std::__not_ > const&, char const*> > >::value, std::__cxx11::basic_string, std::allocator >&>::type std::__cxx11::basic_string, std::allocator >::assign > >(std::basic_string_view > const&) /usr/include/c++/13.2.1/bits/basic_string.h:1733 #4 0x55921037b622 in std::enable_if > const&, std::basic_string_view > >, std::__not_ > const*, std::__cxx11::basic_string, std::allocator > const*> >, std::__not_ > const&, char const*> > >::value, std::__cxx11::basic_string, std::allocator >&>::type std::__cxx11::basic_string, std::allocator >::operator= > >(std::basic_string_view > const&) /usr/include/c++/13.2.1/bits/basic_string.h:925 #5 0x559213183577 in AE::SINK::CAESinkPipewire::EnumerateDevicesEx(std::vector >&, bool) /home/dobo/kodi/xbmc/xbmc/cores/AudioEngine/Sinks/pipewire/AESinkPipewire.cpp:310 #6 0x55921316198a in void std::__invoke_impl >&, bool), std::vector >&, bool>(std::__invoke_other, void (*&)(std::vector >&, bool), std::vector >&, bool&&) (/usr/lib/kodi/kodi.bin+0x623998a) (BuildId: a994426076ec43899fd3927b99c3ccdf5393f60f) #7 0x55921316015a in std::enable_if >&, bool), std::vector >&, bool>, void>::type std::__invoke_r >&, bool), std::vector >&, bool>(void (*&)(std::vector >&, bool), std::vector >&, bool&&) /usr/include/c++/13.2.1/bits/invoke.h:111 #8 0x55921315befe in std::_Function_handler >&, bool), void (*)(std::vector >&, bool)>::_M_invoke(std::_Any_data const&, std::vector >&, bool&&) /usr/include/c++/13.2.1/bits/std_function.h:290 #9 0x5592130a86bf in std::function >&, bool)>::operator()(std::vector >&, bool) const /usr/include/c++/13.2.1/bits/std_function.h:591 #10 0x5592130a6e5a in AE::CAESinkFactory::EnumerateEx(std::vector >&, bool, std::__cxx11::basic_string, std::allocator > const&) /home/dobo/kodi/xbmc/xbmc/cores/AudioEngine/AESinkFactory.cpp:101 #11 0x559213110f45 in ActiveAE::CActiveAESink::EnumerateSinkList(bool, std::__cxx11::basic_string, std::allocator >) /home/dobo/kodi/xbmc/xbmc/cores/AudioEngine/Engines/ActiveAE/ActiveAESink.cpp:702 #12 0x5592130bdfc2 in ActiveAE::CActiveAE::StateMachine(int, Actor::Protocol*, Actor::Message*) /home/dobo/kodi/xbmc/xbmc/cores/AudioEngine/Engines/ActiveAE/ActiveAE.cpp:517 #13 0x5592130c2baa in ActiveAE::CActiveAE::Process() /home/dobo/kodi/xbmc/xbmc/cores/AudioEngine/Engines/ActiveAE/ActiveAE.cpp:1070 #14 0x55921106f9e2 in CThread::Action() /home/dobo/kodi/xbmc/xbmc/threads/Thread.cpp:283 #15 0x55921106e300 in operator() /home/dobo/kodi/xbmc/xbmc/threads/Thread.cpp:152 #16 0x559211070410 in __invoke_impl)>, CThread*, std::promise > /usr/include/c++/13.2.1/bits/invoke.h:61 #17 0x5592110702c9 in __invoke)>, CThread*, std::promise > /usr/include/c++/13.2.1/bits/invoke.h:96 #18 0x5592110701fc in _M_invoke<0, 1, 2> /usr/include/c++/13.2.1/bits/std_thread.h:292 #19 0x559211070199 in operator() /usr/include/c++/13.2.1/bits/std_thread.h:299 #20 0x55921107017d in _M_run /usr/include/c++/13.2.1/bits/std_thread.h:244 #21 0x7effc56e1942 in execute_native_thread_routine /usr/src/debug/gcc/gcc/libstdc++-v3/src/c++11/thread.cc:104 #22 0x7effc628c9ea (/usr/lib/libc.so.6+0x8c9ea) (BuildId: 316d0d3666387f0e8fb98773f51aa1801027c5ab) #23 0x7effc6310dfb (/usr/lib/libc.so.6+0x110dfb) (BuildId: 316d0d3666387f0e8fb98773f51aa1801027c5ab) 0x633000010e60 is located 67168 bytes inside of 98304-byte region [0x633000000800,0x633000018800) freed by thread T3 here: #0 0x7effc84e007a in __interceptor_realloc /usr/src/debug/gcc/gcc/libsanitizer/asan/asan_malloc_linux.cpp:85 #1 0x7effbee91c2f in connection_ensure_size ../pipewire/src/modules/module-protocol-native/connection.c:143 previously allocated by thread T3 here: #0 0x7effc84e007a in __interceptor_realloc /usr/src/debug/gcc/gcc/libsanitizer/asan/asan_malloc_linux.cpp:85 #1 0x7effbee91c2f in connection_ensure_size ../pipewire/src/modules/module-protocol-native/connection.c:143 Thread T19 created by T0 here: #0 0x7effc844a497 in __interceptor_pthread_create /usr/src/debug/gcc/gcc/libsanitizer/asan/asan_interceptors.cpp:208 #1 0x7effc56e1a29 in __gthread_create /usr/src/debug/gcc/gcc-build/x86_64-pc-linux-gnu/libstdc++-v3/include/x86_64-pc-linux-gnu/bits/gthr-default.h:663 #2 0x7effc56e1a29 in std::thread::_M_start_thread(std::unique_ptr >, void (*)()) /usr/src/debug/gcc/gcc/libstdc++-v3/src/c++11/thread.cc:172 #3 0x55921106ee30 in CThread::Create(bool) /home/dobo/kodi/xbmc/xbmc/threads/Thread.cpp:175 #4 0x5592130d96cd in ActiveAE::CActiveAE::Start() /home/dobo/kodi/xbmc/xbmc/cores/AudioEngine/Engines/ActiveAE/ActiveAE.cpp:2675 #5 0x5592117bc377 in CApplication::Initialize() /home/dobo/kodi/xbmc/xbmc/application/Application.cpp:610 #6 0x559211124646 in XBMC_Run /home/dobo/kodi/xbmc/xbmc/platform/xbmc.cpp:43 #7 0x55920fd30a70 in main /home/dobo/kodi/xbmc/xbmc/platform/posix/main.cpp:77 #8 0x7effc6227ccf (/usr/lib/libc.so.6+0x27ccf) (BuildId: 316d0d3666387f0e8fb98773f51aa1801027c5ab) Thread T3 created by T0 here: #0 0x7effc844a497 in __interceptor_pthread_create /usr/src/debug/gcc/gcc/libsanitizer/asan/asan_interceptors.cpp:208 #1 0x7effc7e73e5f in impl_create ../pipewire/src/pipewire/thread.c:68 SUMMARY: AddressSanitizer: heap-use-after-free /usr/src/debug/gcc/gcc/libsanitizer/sanitizer_common/sanitizer_common_interceptors.inc:899 in __interceptor_memcpy Shadow bytes around the buggy address: 0x633000010b80: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd 0x633000010c00: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd 0x633000010c80: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd 0x633000010d00: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd 0x633000010d80: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd =>0x633000010e00: fd fd fd fd fd fd fd fd fd fd fd fd[fd]fd fd fd 0x633000010e80: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd 0x633000010f00: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd 0x633000010f80: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd 0x633000011000: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd 0x633000011080: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd Shadow byte legend (one shadow byte represents 8 application bytes): Addressable: 00 Partially addressable: 01 02 03 04 05 06 07 Heap left redzone: fa Freed heap region: fd Stack left redzone: f1 Stack mid redzone: f2 Stack right redzone: f3 Stack after return: f5 Stack use after scope: f8 Global redzone: f9 Global init order: f6 Poisoned by user: f7 Container overflow: fc Array cookie: ac Intra object redzone: bb ASan internal: fe Left alloca redzone: ca Right alloca redzone: cb ==14082==ABORTING [2]: https://github.com/PipeWire/pipewire/blob/b5c3f217926f9066a1afbee7eb20967dd6896c56/src/modules/module-protocol-native/connection.c#L143C8-L143C15 --- .../Sinks/pipewire/PipewireGlobal.h | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/xbmc/cores/AudioEngine/Sinks/pipewire/PipewireGlobal.h b/xbmc/cores/AudioEngine/Sinks/pipewire/PipewireGlobal.h index 7b1021ae3853f..67f40c72203fe 100644 --- a/xbmc/cores/AudioEngine/Sinks/pipewire/PipewireGlobal.h +++ b/xbmc/cores/AudioEngine/Sinks/pipewire/PipewireGlobal.h @@ -29,21 +29,21 @@ class CPipewireGlobal CPipewireGlobal() = default; ~CPipewireGlobal(); - CPipewireGlobal& SetName(std::string_view name) + CPipewireGlobal& SetName(const std::string& name) { m_name = name; return *this; } - std::string_view GetName() const { return m_name; } + std::string GetName() const { return m_name; } - CPipewireGlobal& SetDescription(std::string_view description) + CPipewireGlobal& SetDescription(const std::string& description) { m_description = description; return *this; } - std::string_view GetDescription() const { return m_description; } + std::string GetDescription() const { return m_description; } CPipewireGlobal& SetID(uint32_t id) { @@ -61,13 +61,13 @@ class CPipewireGlobal uint32_t GetPermissions() const { return m_permissions; } - CPipewireGlobal& SetType(std::string_view type) + CPipewireGlobal& SetType(const std::string& type) { m_type = type; return *this; } - std::string_view GetType() const { return m_type; } + std::string GetType() const { return m_type; } CPipewireGlobal& SetVersion(uint32_t version) { @@ -95,11 +95,11 @@ class CPipewireGlobal CPipewireNode& GetNode() const { return *m_node; } private: - std::string_view m_name; - std::string_view m_description; + std::string m_name; + std::string m_description; uint32_t m_id; uint32_t m_permissions; - std::string_view m_type; + std::string m_type; uint32_t m_version; std::unique_ptr m_properties; std::unique_ptr m_node;