Skip to content

Commit

Permalink
[PR 24946] [GUIIncludes] Fix heap-use-after-free
Browse files Browse the repository at this point in the history
Only remove the child if the node is actually a child.

==51989==ERROR: AddressSanitizer: heap-use-after-free on address 0x511003b69210 at pc 0x5ce4b249275e bp 0x7fff43e1d430 sp 0x7fff43e1d428
READ of size 8 at 0x511003b69210 thread T0
    #0 0x5ce4b249275d in TiXmlAttributeSet::First() /usr/include/tinyxml.h:915:50
    #1 0x5ce4b2492098 in TiXmlElement::FirstAttribute() /usr/include/tinyxml.h:1087:61
    #2 0x5ce4b2bb091e in CGUIIncludes::ResolveParametersForNode(TiXmlElement*, std::map<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char>>, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char>>, std::less<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char>>>, std::allocator<std::pair<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char>> const, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char>>>>> const&) xbmc/guilib/GUIIncludes.cpp:586:37
    #3 0x5ce4b2bae9bb in CGUIIncludes::ResolveIncludes(TiXmlElement*, std::map<std::shared_ptr<INFO::InfoBool>, bool, std::less<std::shared_ptr<INFO::InfoBool>>, std::allocator<std::pair<std::shared_ptr<INFO::InfoBool> const, bool>>>*) xbmc/guilib/GUIIncludes.cpp:485:9
    #4 0x5ce4b2ba8eaf in CGUIIncludes::Resolve(TiXmlElement*, std::map<std::shared_ptr<INFO::InfoBool>, bool, std::less<std::shared_ptr<INFO::InfoBool>>, std::allocator<std::pair<std::shared_ptr<INFO::InfoBool> const, bool>>>*) xbmc/guilib/GUIIncludes.cpp:312:3
    #5 0x5ce4b2ba8fce in CGUIIncludes::Resolve(TiXmlElement*, std::map<std::shared_ptr<INFO::InfoBool>, bool, std::less<std::shared_ptr<INFO::InfoBool>>, std::allocator<std::pair<std::shared_ptr<INFO::InfoBool> const, bool>>>*) xbmc/guilib/GUIIncludes.cpp:318:5
    #6 0x5ce4b3e808d3 in ADDON::CSkinInfo::ResolveIncludes(TiXmlElement*, std::map<std::shared_ptr<INFO::InfoBool>, bool, std::less<std::shared_ptr<INFO::InfoBool>>, std::allocator<std::pair<std::shared_ptr<INFO::InfoBool> const, bool>>>*) xbmc/addons/Skin.cpp:307:14
    #7 0x5ce4b2e00084 in CGUIWindow::Prepare(std::unique_ptr<TiXmlElement, std::default_delete<TiXmlElement>> const&) xbmc/guilib/GUIWindow.cpp:168:15
    #8 0x5ce4b2dff45e in CGUIWindow::LoadXML(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char>> const&, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char>> const&) xbmc/guilib/GUIWindow.cpp:155:15
    #9 0x5ce4b2dfd540 in CGUIWindow::Load(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char>> const&, bool) xbmc/guilib/GUIWindow.cpp:109:14
    #10 0x5ce4b2e1cac5 in CGUIWindow::AllocResources(bool) xbmc/guilib/GUIWindow.cpp:765:7
    #11 0x5ce4b2e14c77 in CGUIWindow::OnMessage(CGUIMessage&) xbmc/guilib/GUIWindow.cpp:594:52
    #12 0x5ce4b19ce9d2 in CGUIWindowHome::OnMessage(CGUIMessage&) xbmc/windows/GUIWindowHome.cpp:182:22
    #13 0x5ce4b2e613a1 in CGUIWindowManager::ActivateWindow_Internal(int, std::vector<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char>>, std::allocator<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char>>>> const&, bool, bool) xbmc/guilib/GUIWindowManager.cpp:896:15
    #14 0x5ce4b2e5ce3c in CGUIWindowManager::ActivateWindow(int, std::vector<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char>>, std::allocator<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char>>>> const&, bool, bool) xbmc/guilib/GUIWindowManager.cpp:802:5
    #15 0x5ce4b683ad63 in int (anonymous namespace)::ActivateWindow<true>(std::vector<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char>>, std::allocator<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char>>>> const&) xbmc/interfaces/builtins/GUIBuiltins.cpp:109:52
    #16 0x5ce4b6822865 in CBuiltins::Execute(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char>> const&) xbmc/interfaces/builtins/Builtins.cpp:158:14
    #17 0x5ce4b34047ff in CApplication::ExecuteXBMCAction(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char>>, std::shared_ptr<CGUIListItem> const&) xbmc/application/Application.cpp:3037:32
    #18 0x5ce4b3400a16 in CApplication::OnMessage(CGUIMessage&) xbmc/application/Application.cpp:3013:14
    #19 0x5ce4b34058a0 in non-virtual thunk to CApplication::OnMessage(CGUIMessage&) xbmc/application/Application.cpp
    #20 0x5ce4b2e52261 in CGUIWindowManager::SendMessage(CGUIMessage&) xbmc/guilib/GUIWindowManager.cpp:510:23
    #21 0x5ce4b2e7cc7f in CGUIWindowManager::DispatchThreadMessages() xbmc/guilib/GUIWindowManager.cpp:1572:7
    #22 0x5ce4b3405bfa in CApplication::Process() xbmc/application/Application.cpp:3139:48
    #23 0x5ce4b33ddc98 in CApplication::Run() xbmc/application/Application.cpp:1855:5
    #24 0x5ce4b251b323 in XBMC_Run xbmc/platform/xbmc.cpp:61:26
    #25 0x5ce4af14af0f in main xbmc/platform/posix/main.cpp:70:16
    #26 0x76d804243ccf  (/usr/lib/libc.so.6+0x25ccf) (BuildId: c0caa0b7709d3369ee575fcd7d7d0b0fc48733af)
    #27 0x76d804243d89 in __libc_start_main (/usr/lib/libc.so.6+0x25d89) (BuildId: c0caa0b7709d3369ee575fcd7d7d0b0fc48733af)
    #28 0x5ce4af010b94 in _start (/home/mark/Coding/Repos/kodi-git/build_clang_debug_sanitizer/kodi.bin+0xa317b94) (BuildId: 923aa634157be6adc50052366abd3ca0edfeffc0)

0x511003b69210 is located 208 bytes inside of 216-byte region [0x511003b69140,0x511003b69218)
freed by thread T0 here:
    #0 0x5ce4af148d72 in operator delete(void*, unsigned long) (/home/mark/Coding/Repos/kodi-git/build_clang_debug_sanitizer/kodi.bin+0xa44fd72) (BuildId: 923aa634157be6adc50052366abd3ca0edfeffc0)
    #1 0x76d80670ea48 in TiXmlNode::RemoveChild(TiXmlNode*) (/usr/lib/libtinyxml.so.0+0x8a48) (BuildId: 2f5d236264d4d695dbe432f41e1eb46c7bc2d5d4)
    #2 0x5ce4b2bae9a3 in CGUIIncludes::ResolveIncludes(TiXmlElement*, std::map<std::shared_ptr<INFO::InfoBool>, bool, std::less<std::shared_ptr<INFO::InfoBool>>, std::allocator<std::pair<std::shared_ptr<INFO::InfoBool> const, bool>>>*) xbmc/guilib/GUIIncludes.cpp:482:9
    #3 0x5ce4b2ba8eaf in CGUIIncludes::Resolve(TiXmlElement*, std::map<std::shared_ptr<INFO::InfoBool>, bool, std::less<std::shared_ptr<INFO::InfoBool>>, std::allocator<std::pair<std::shared_ptr<INFO::InfoBool> const, bool>>>*) xbmc/guilib/GUIIncludes.cpp:312:3
    #4 0x5ce4b2ba8fce in CGUIIncludes::Resolve(TiXmlElement*, std::map<std::shared_ptr<INFO::InfoBool>, bool, std::less<std::shared_ptr<INFO::InfoBool>>, std::allocator<std::pair<std::shared_ptr<INFO::InfoBool> const, bool>>>*) xbmc/guilib/GUIIncludes.cpp:318:5
    #5 0x5ce4b3e808d3 in ADDON::CSkinInfo::ResolveIncludes(TiXmlElement*, std::map<std::shared_ptr<INFO::InfoBool>, bool, std::less<std::shared_ptr<INFO::InfoBool>>, std::allocator<std::pair<std::shared_ptr<INFO::InfoBool> const, bool>>>*) xbmc/addons/Skin.cpp:307:14
    #6 0x5ce4b2e00084 in CGUIWindow::Prepare(std::unique_ptr<TiXmlElement, std::default_delete<TiXmlElement>> const&) xbmc/guilib/GUIWindow.cpp:168:15
    #7 0x5ce4b2dff45e in CGUIWindow::LoadXML(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char>> const&, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char>> const&) xbmc/guilib/GUIWindow.cpp:155:15
    #8 0x5ce4b2dfd540 in CGUIWindow::Load(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char>> const&, bool) xbmc/guilib/GUIWindow.cpp:109:14
    #9 0x5ce4b2e1cac5 in CGUIWindow::AllocResources(bool) xbmc/guilib/GUIWindow.cpp:765:7
    #10 0x5ce4b2e14c77 in CGUIWindow::OnMessage(CGUIMessage&) xbmc/guilib/GUIWindow.cpp:594:52
    #11 0x5ce4b19ce9d2 in CGUIWindowHome::OnMessage(CGUIMessage&) xbmc/windows/GUIWindowHome.cpp:182:22
    #12 0x5ce4b2e613a1 in CGUIWindowManager::ActivateWindow_Internal(int, std::vector<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char>>, std::allocator<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char>>>> const&, bool, bool) xbmc/guilib/GUIWindowManager.cpp:896:15
    #13 0x5ce4b2e5ce3c in CGUIWindowManager::ActivateWindow(int, std::vector<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char>>, std::allocator<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char>>>> const&, bool, bool) xbmc/guilib/GUIWindowManager.cpp:802:5
    #14 0x5ce4b683ad63 in int (anonymous namespace)::ActivateWindow<true>(std::vector<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char>>, std::allocator<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char>>>> const&) xbmc/interfaces/builtins/GUIBuiltins.cpp:109:52
    #15 0x5ce4b6822865 in CBuiltins::Execute(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char>> const&) xbmc/interfaces/builtins/Builtins.cpp:158:14
    #16 0x5ce4b34047ff in CApplication::ExecuteXBMCAction(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char>>, std::shared_ptr<CGUIListItem> const&) xbmc/application/Application.cpp:3037:32
    #17 0x5ce4b3400a16 in CApplication::OnMessage(CGUIMessage&) xbmc/application/Application.cpp:3013:14
    #18 0x5ce4b34058a0 in non-virtual thunk to CApplication::OnMessage(CGUIMessage&) xbmc/application/Application.cpp
    #19 0x5ce4b2e52261 in CGUIWindowManager::SendMessage(CGUIMessage&) xbmc/guilib/GUIWindowManager.cpp:510:23
    #20 0x5ce4b2e7cc7f in CGUIWindowManager::DispatchThreadMessages() xbmc/guilib/GUIWindowManager.cpp:1572:7
    #21 0x5ce4b3405bfa in CApplication::Process() xbmc/application/Application.cpp:3139:48
    #22 0x5ce4b33ddc98 in CApplication::Run() xbmc/application/Application.cpp:1855:5
    #23 0x5ce4b251b323 in XBMC_Run xbmc/platform/xbmc.cpp:61:26
    #24 0x5ce4af14af0f in main xbmc/platform/posix/main.cpp:70:16
    #25 0x76d804243ccf  (/usr/lib/libc.so.6+0x25ccf) (BuildId: c0caa0b7709d3369ee575fcd7d7d0b0fc48733af)

previously allocated by thread T0 here:
    #0 0x5ce4af147e12 in operator new(unsigned long) (/home/mark/Coding/Repos/kodi-git/build_clang_debug_sanitizer/kodi.bin+0xa44ee12) (BuildId: 923aa634157be6adc50052366abd3ca0edfeffc0)
    #1 0x76d806711497 in TiXmlElement::Clone() const (/usr/lib/libtinyxml.so.0+0xb497) (BuildId: 2f5d236264d4d695dbe432f41e1eb46c7bc2d5d4)

SUMMARY: AddressSanitizer: heap-use-after-free /usr/include/tinyxml.h:915:50 in TiXmlAttributeSet::First()
Shadow bytes around the buggy address:
  0x511003b68f80: 00 00 00 fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x511003b69000: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x511003b69080: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x511003b69100: fa fa fa fa fa fa fa fa fd fd fd fd fd fd fd fd
  0x511003b69180: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
=>0x511003b69200: fd fd[fd]fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x511003b69280: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x511003b69300: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x511003b69380: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x511003b69400: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x511003b69480: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
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
==51989==ABORTING

(cherry picked from commit 52988c9)
  • Loading branch information
neo1973 authored and garbear committed May 12, 2024
1 parent c38c1fb commit f75e298
Showing 1 changed file with 2 additions and 1 deletion.
3 changes: 2 additions & 1 deletion xbmc/guilib/GUIIncludes.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -529,7 +529,8 @@ void CGUIIncludes::InsertNested(TiXmlElement *controls, TiXmlElement *include, T
}
child = child->NextSiblingElement();
}
target->RemoveChild(nested);
if (nested != node)
target->RemoveChild(nested);
}

}
Expand Down

0 comments on commit f75e298

Please sign in to comment.