[pipewire] Fix heap-use-after-free in AE::SINK::CAESinkPipewire::EnumerateDevicesEx

dobo90 · dobo90 · commit f160d52babaa · 2023-09-08T13:58:23.000+02:00
* 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<char, std::char_traits<char>, std::allocator<char> >::_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<char, std::char_traits<char>, std::allocator<char> >::_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<std::__and_<std::is_convertible<std::basic_string_view<char, std::char_traits<char> > const&, std::basic_string_view<char, std::char_traits<char> > >, std::__not_<std::is_convertible<std::basic_string_view<char, std::char_traits<char> > const*, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const*> >, std::__not_<std::is_convertible<std::basic_string_view<char, std::char_traits<char> > const&, char const*> > >::value, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >&>::type std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >::assign<std::basic_string_view<char, std::char_traits<char> > >(std::basic_string_view<char, std::char_traits<char> > const&) /usr/include/c++/13.2.1/bits/basic_string.h:1733 #4 0x55921037b622 in std::enable_if<std::__and_<std::is_convertible<std::basic_string_view<char, std::char_traits<char> > const&, std::basic_string_view<char, std::char_traits<char> > >, std::__not_<std::is_convertible<std::basic_string_view<char, std::char_traits<char> > const*, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const*> >, std::__not_<std::is_convertible<std::basic_string_view<char, std::char_traits<char> > const&, char const*> > >::value, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >&>::type std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >::operator=<std::basic_string_view<char, std::char_traits<char> > >(std::basic_string_view<char, std::char_traits<char> > const&) /usr/include/c++/13.2.1/bits/basic_string.h:925 #5 0x559213183577 in AE::SINK::CAESinkPipewire::EnumerateDevicesEx(std::vector<CAEDeviceInfo, std::allocator<CAEDeviceInfo> >&, bool) /home/dobo/kodi/xbmc/xbmc/cores/AudioEngine/Sinks/pipewire/AESinkPipewire.cpp:310 #6 0x55921316198a in void std::__invoke_impl<void, void (*&)(std::vector<CAEDeviceInfo, std::allocator<CAEDeviceInfo> >&, bool), std::vector<CAEDeviceInfo, std::allocator<CAEDeviceInfo> >&, bool>(std::__invoke_other, void (*&)(std::vector<CAEDeviceInfo, std::allocator<CAEDeviceInfo> >&, bool), std::vector<CAEDeviceInfo, std::allocator<CAEDeviceInfo> >&, bool&&) (/usr/lib/kodi/kodi.bin+0x623998a) (BuildId: a994426076ec43899fd3927b99c3ccdf5393f60f) #7 0x55921316015a in std::enable_if<is_invocable_r_v<void, void (*&)(std::vector<CAEDeviceInfo, std::allocator<CAEDeviceInfo> >&, bool), std::vector<CAEDeviceInfo, std::allocator<CAEDeviceInfo> >&, bool>, void>::type std::__invoke_r<void, void (*&)(std::vector<CAEDeviceInfo, std::allocator<CAEDeviceInfo> >&, bool), std::vector<CAEDeviceInfo, std::allocator<CAEDeviceInfo> >&, bool>(void (*&)(std::vector<CAEDeviceInfo, std::allocator<CAEDeviceInfo> >&, bool), std::vector<CAEDeviceInfo, std::allocator<CAEDeviceInfo> >&, bool&&) /usr/include/c++/13.2.1/bits/invoke.h:111 #8 0x55921315befe in std::_Function_handler<void (std::vector<CAEDeviceInfo, std::allocator<CAEDeviceInfo> >&, bool), void (*)(std::vector<CAEDeviceInfo, std::allocator<CAEDeviceInfo> >&, bool)>::_M_invoke(std::_Any_data const&, std::vector<CAEDeviceInfo, std::allocator<CAEDeviceInfo> >&, bool&&) /usr/include/c++/13.2.1/bits/std_function.h:290 #9 0x5592130a86bf in std::function<void (std::vector<CAEDeviceInfo, std::allocator<CAEDeviceInfo> >&, bool)>::operator()(std::vector<CAEDeviceInfo, std::allocator<CAEDeviceInfo> >&, bool) const /usr/include/c++/13.2.1/bits/std_function.h:591 #10 0x5592130a6e5a in AE::CAESinkFactory::EnumerateEx(std::vector<AE::AESinkInfo, std::allocator<AE::AESinkInfo> >&, bool, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&) /home/dobo/kodi/xbmc/xbmc/cores/AudioEngine/AESinkFactory.cpp:101 #11 0x559213110f45 in ActiveAE::CActiveAESink::EnumerateSinkList(bool, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >) /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<void, CThread::Create(bool)::<lambda(CThread*, std::promise<bool>)>, CThread*, std::promise<bool> > /usr/include/c++/13.2.1/bits/invoke.h:61 #17 0x5592110702c9 in __invoke<CThread::Create(bool)::<lambda(CThread*, std::promise<bool>)>, CThread*, std::promise<bool> > /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<std::thread::_State, std::default_delete<std::thread::_State> >, 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
diff --git 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<pw_properties, PipewirePropertiesDeleter> m_properties;
   std::unique_ptr<CPipewireNode> m_node;
