Skip to content

Commit

Permalink
RegExp: Fix heap-use-after-free
Browse files Browse the repository at this point in the history 
`m_iOvector` is a pointer array into the match data, so the match data must be
kept as long as `m_iOvector` is used.

Address Sanitizer output:

==28015==ERROR: AddressSanitizer: heap-use-after-free on address 0x5190000406e8 at pc 0x59cf02e5ad9e bp 0x7ffdcd8a1a10 sp 0x7ffdcd8a1a08
READ of size 8 at 0x5190000406e8 thread T0
    #0 0x59cf02e5ad9d in CRegExp::PrivateRegFind(unsigned long, char const*, unsigned int, int) xbmc/utils/RegExp.cpp:425:10
    xbmc#1 0x59cf013cb942 in CRegExp::RegFind(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char>> const&, unsigned int, int) xbmc/utils/RegExp.h:95:12
    xbmc#2 0x59cf0316a9ef in CXBMCTinyXML2::ParseHelper(unsigned long, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char>>&&) xbmc/utils/XBMCTinyXML2.cpp:106:12
    xbmc#3 0x59cf0316965b in CXBMCTinyXML2::Parse(std::basic_string_view<char, std::char_traits<char>>) xbmc/utils/XBMCTinyXML2.cpp:82:10
    xbmc#4 0x59cf03168fac in CXBMCTinyXML2::LoadFile(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char>> const&) xbmc/utils/XBMCTinyXML2.cpp:36:3
    xbmc#5 0x59cf0563fe1b in CMediaSourceSettings::Load(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char>> const&) xbmc/settings/MediaSourceSettings.cpp:85:15
    xbmc#6 0x59cf0563f590 in CMediaSourceSettings::Load() xbmc/settings/MediaSourceSettings.cpp:71:10
    xbmc#7 0x59cf0563f3d0 in CMediaSourceSettings::OnSettingsLoaded() xbmc/settings/MediaSourceSettings.cpp:61:3
    xbmc#8 0x59cf053c86c4 in CSettingsManager::OnSettingsLoaded() xbmc/settings/lib/SettingsManager.cpp:1022:22
    xbmc#9 0x59cf053969dc in CSettingsManager::Load(TiXmlElement const*, bool&, bool, std::map<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char>>, std::shared_ptr<CSetting>, 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::shared_ptr<CSetting>>>>*) xbmc/settings/lib/SettingsManager.cpp:173:5
    xbmc#10 0x59cf056ed775 in CSettingsBase::LoadValuesFromXml(TiXmlElement const*, bool&) xbmc/settings/SettingsBase.cpp:86:29
    xbmc#11 0x59cf056b96b0 in CSettings::Load(TiXmlElement const*, bool&) xbmc/settings/Settings.cpp:217:23
    xbmc#12 0x59cf056b8f00 in CSettings::Load(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char>> const&) xbmc/settings/Settings.cpp:125:8
    xbmc#13 0x59cf056b87e5 in CSettings::Load() xbmc/settings/Settings.cpp:117:10
    xbmc#14 0x59cf05715a60 in CSettingsComponent::Load() xbmc/settings/SettingsComponent.cpp:83:22
    xbmc#15 0x59cf041da912 in CApplication::Create() xbmc/application/Application.cpp:320:27
    xbmc#16 0x59cf033b4eed in XBMC_Run xbmc/platform/xbmc.cpp:26:22
    xbmc#17 0x59cf00752b7f in main xbmc/platform/posix/main.cpp:70:16
    xbmc#18 0x74a07d239c87  (/usr/lib/libc.so.6+0x25c87) (BuildId: 32a656aa5562eece8c59a585f5eacd6cf5e2307b)
    xbmc#19 0x74a07d239d4b in __libc_start_main (/usr/lib/libc.so.6+0x25d4b) (BuildId: 32a656aa5562eece8c59a585f5eacd6cf5e2307b)
    xbmc#20 0x59cf00618804 in _start (/home/mark/Coding/Repos/kodi-git/build_clang_debug_sanitizer/kodi.bin+0x9f91804) (BuildId: fa447ae84e6fbfe91e7ec718a600116496d7607e)

0x5190000406e8 is located 104 bytes inside of 1112-byte region [0x519000040680,0x519000040ad8)
freed by thread T0 here:
    #0 0x59cf007069b2 in free.part.0 (/home/mark/Coding/Repos/kodi-git/build_clang_debug_sanitizer/kodi.bin+0xa07f9b2) (BuildId: fa447ae84e6fbfe91e7ec718a600116496d7607e)
    xbmc#1 0x59cf02e59ed8 in CRegExp::PrivateRegFind(unsigned long, char const*, unsigned int, int) xbmc/utils/RegExp.cpp:352:3
    xbmc#2 0x59cf013cb942 in CRegExp::RegFind(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char>> const&, unsigned int, int) xbmc/utils/RegExp.h:95:12
    xbmc#3 0x59cf0316a9ef in CXBMCTinyXML2::ParseHelper(unsigned long, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char>>&&) xbmc/utils/XBMCTinyXML2.cpp:106:12
    xbmc#4 0x59cf0316965b in CXBMCTinyXML2::Parse(std::basic_string_view<char, std::char_traits<char>>) xbmc/utils/XBMCTinyXML2.cpp:82:10
    xbmc#5 0x59cf03168fac in CXBMCTinyXML2::LoadFile(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char>> const&) xbmc/utils/XBMCTinyXML2.cpp:36:3
    xbmc#6 0x59cf0563fe1b in CMediaSourceSettings::Load(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char>> const&) xbmc/settings/MediaSourceSettings.cpp:85:15
    xbmc#7 0x59cf0563f590 in CMediaSourceSettings::Load() xbmc/settings/MediaSourceSettings.cpp:71:10
    xbmc#8 0x59cf0563f3d0 in CMediaSourceSettings::OnSettingsLoaded() xbmc/settings/MediaSourceSettings.cpp:61:3
    xbmc#9 0x59cf053c86c4 in CSettingsManager::OnSettingsLoaded() xbmc/settings/lib/SettingsManager.cpp:1022:22
    xbmc#10 0x59cf053969dc in CSettingsManager::Load(TiXmlElement const*, bool&, bool, std::map<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char>>, std::shared_ptr<CSetting>, 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::shared_ptr<CSetting>>>>*) xbmc/settings/lib/SettingsManager.cpp:173:5
    xbmc#11 0x59cf056ed775 in CSettingsBase::LoadValuesFromXml(TiXmlElement const*, bool&) xbmc/settings/SettingsBase.cpp:86:29
    xbmc#12 0x59cf056b96b0 in CSettings::Load(TiXmlElement const*, bool&) xbmc/settings/Settings.cpp:217:23
    xbmc#13 0x59cf056b8f00 in CSettings::Load(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char>> const&) xbmc/settings/Settings.cpp:125:8
    xbmc#14 0x59cf056b87e5 in CSettings::Load() xbmc/settings/Settings.cpp:117:10
    xbmc#15 0x59cf05715a60 in CSettingsComponent::Load() xbmc/settings/SettingsComponent.cpp:83:22
    xbmc#16 0x59cf041da912 in CApplication::Create() xbmc/application/Application.cpp:320:27
    xbmc#17 0x59cf033b4eed in XBMC_Run xbmc/platform/xbmc.cpp:26:22
    xbmc#18 0x59cf00752b7f in main xbmc/platform/posix/main.cpp:70:16
    xbmc#19 0x74a07d239c87  (/usr/lib/libc.so.6+0x25c87) (BuildId: 32a656aa5562eece8c59a585f5eacd6cf5e2307b)
    xbmc#20 0x74a07d239d4b in __libc_start_main (/usr/lib/libc.so.6+0x25d4b) (BuildId: 32a656aa5562eece8c59a585f5eacd6cf5e2307b)
    xbmc#21 0x59cf00618804 in _start (/home/mark/Coding/Repos/kodi-git/build_clang_debug_sanitizer/kodi.bin+0x9f91804) (BuildId: fa447ae84e6fbfe91e7ec718a600116496d7607e)

previously allocated by thread T0 here:
    #0 0x59cf007079e9 in malloc (/home/mark/Coding/Repos/kodi-git/build_clang_debug_sanitizer/kodi.bin+0xa0809e9) (BuildId: fa447ae84e6fbfe91e7ec718a600116496d7607e)
    xbmc#1 0x74a07fb7faed  (/usr/lib/libpcre2-8.so.0+0x12aed) (BuildId: d6a22ace8f92ae592b620499fc467ef7899f99a0)
    xbmc#2 0x74a07fbbf29f in pcre2_match_data_create_8 (/usr/lib/libpcre2-8.so.0+0x5229f) (BuildId: d6a22ace8f92ae592b620499fc467ef7899f99a0)
    xbmc#3 0x59cf02e59c65 in CRegExp::PrivateRegFind(unsigned long, char const*, unsigned int, int) xbmc/utils/RegExp.cpp:347:8
    xbmc#4 0x59cf013cb942 in CRegExp::RegFind(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char>> const&, unsigned int, int) xbmc/utils/RegExp.h:95:12
    xbmc#5 0x59cf0316a9ef in CXBMCTinyXML2::ParseHelper(unsigned long, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char>>&&) xbmc/utils/XBMCTinyXML2.cpp:106:12
    xbmc#6 0x59cf0316965b in CXBMCTinyXML2::Parse(std::basic_string_view<char, std::char_traits<char>>) xbmc/utils/XBMCTinyXML2.cpp:82:10
    xbmc#7 0x59cf03168fac in CXBMCTinyXML2::LoadFile(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char>> const&) xbmc/utils/XBMCTinyXML2.cpp:36:3
    xbmc#8 0x59cf0563fe1b in CMediaSourceSettings::Load(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char>> const&) xbmc/settings/MediaSourceSettings.cpp:85:15
    xbmc#9 0x59cf0563f590 in CMediaSourceSettings::Load() xbmc/settings/MediaSourceSettings.cpp:71:10
    xbmc#10 0x59cf0563f3d0 in CMediaSourceSettings::OnSettingsLoaded() xbmc/settings/MediaSourceSettings.cpp:61:3
    xbmc#11 0x59cf053c86c4 in CSettingsManager::OnSettingsLoaded() xbmc/settings/lib/SettingsManager.cpp:1022:22
    xbmc#12 0x59cf053969dc in CSettingsManager::Load(TiXmlElement const*, bool&, bool, std::map<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char>>, std::shared_ptr<CSetting>, 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::shared_ptr<CSetting>>>>*) xbmc/settings/lib/SettingsManager.cpp:173:5
    xbmc#13 0x59cf056ed775 in CSettingsBase::LoadValuesFromXml(TiXmlElement const*, bool&) xbmc/settings/SettingsBase.cpp:86:29
    xbmc#14 0x59cf056b96b0 in CSettings::Load(TiXmlElement const*, bool&) xbmc/settings/Settings.cpp:217:23
    xbmc#15 0x59cf056b8f00 in CSettings::Load(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char>> const&) xbmc/settings/Settings.cpp:125:8
    xbmc#16 0x59cf056b87e5 in CSettings::Load() xbmc/settings/Settings.cpp:117:10
    xbmc#17 0x59cf05715a60 in CSettingsComponent::Load() xbmc/settings/SettingsComponent.cpp:83:22
    xbmc#18 0x59cf041da912 in CApplication::Create() xbmc/application/Application.cpp:320:27
    xbmc#19 0x59cf033b4eed in XBMC_Run xbmc/platform/xbmc.cpp:26:22
    xbmc#20 0x59cf00752b7f in main xbmc/platform/posix/main.cpp:70:16
    xbmc#21 0x74a07d239c87  (/usr/lib/libc.so.6+0x25c87) (BuildId: 32a656aa5562eece8c59a585f5eacd6cf5e2307b)
    xbmc#22 0x74a07d239d4b in __libc_start_main (/usr/lib/libc.so.6+0x25d4b) (BuildId: 32a656aa5562eece8c59a585f5eacd6cf5e2307b)
    xbmc#23 0x59cf00618804 in _start (/home/mark/Coding/Repos/kodi-git/build_clang_debug_sanitizer/kodi.bin+0x9f91804) (BuildId: fa447ae84e6fbfe91e7ec718a600116496d7607e)

SUMMARY: AddressSanitizer: heap-use-after-free xbmc/utils/RegExp.cpp:425:10 in CRegExp::PrivateRegFind(unsigned long, char const*, unsigned int, int)
Shadow bytes around the buggy address:
  0x519000040400: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
  0x519000040480: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
  0x519000040500: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
  0x519000040580: fd fd fd fd fd fd fa fa fa fa fa fa fa fa fa fa
  0x519000040600: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
=>0x519000040680: fd fd fd fd fd fd fd fd fd fd fd fd fd[fd]fd fd
  0x519000040700: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
  0x519000040780: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
  0x519000040800: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
  0x519000040880: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
  0x519000040900: 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
==28015==ABORTING
  • Loading branch information
@neo1973
neo1973 committed Jun 28, 2024
1 parent 5810c32 commit 6efd635
Show file tree
Hide file tree
Showing 2 changed files with 14 additions and 6 deletions.
19 changes: 13 additions & 6 deletions xbmc/utils/RegExp.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ void CRegExp::InitValues(bool caseless /*= false*/, CRegExp::utf8Mode utf8 /*= a
m_jitCompiled = false;
m_bMatched = false;
m_iMatchCount = 0;
m_matchData = nullptr;
m_iOvector = nullptr;
m_jitStack = NULL;
}
Expand Down Expand Up @@ -204,6 +205,7 @@ CRegExp::CRegExp(const CRegExp& re)
{
m_re = NULL;
m_ctxt = nullptr;
m_matchData = nullptr;
m_iOvector = nullptr;
m_jitStack = NULL;
m_utf8Mode = re.m_utf8Mode;
Expand Down Expand Up @@ -303,7 +305,6 @@ int CRegExp::RegFind(const char *str, unsigned int startoffset /*= 0*/, int maxN

int CRegExp::PrivateRegFind(size_t bufferLen, const char *str, unsigned int startoffset /* = 0*/, int maxNumberOfCharsToTest /*= -1*/)
{
pcre2_match_data* md;
PCRE2_SIZE offset;

m_offset = 0;
Expand Down Expand Up @@ -344,12 +345,12 @@ int CRegExp::PrivateRegFind(size_t bufferLen, const char *str, unsigned int star
bufferLen = std::min<size_t>(bufferLen, startoffset + maxNumberOfCharsToTest);

m_subject.assign(str + startoffset, bufferLen - startoffset);
md = pcre2_match_data_create(OVECCOUNT, nullptr);
if (m_matchData == nullptr)
m_matchData = pcre2_match_data_create(OVECCOUNT, nullptr);
int rc = pcre2_match(m_re, reinterpret_cast<PCRE2_SPTR>(m_subject.c_str()), m_subject.length(), 0,
0, md, m_ctxt);
m_iOvector = pcre2_get_ovector_pointer(md);
offset = pcre2_get_startchar(md);
pcre2_match_data_free(md);
0, m_matchData, m_ctxt);
m_iOvector = pcre2_get_ovector_pointer(m_matchData);
offset = pcre2_get_startchar(m_matchData);

if (rc<1)
{
Expand Down Expand Up @@ -549,6 +550,12 @@ void CRegExp::Cleanup()
pcre2_jit_stack_free(m_jitStack);
m_jitStack = NULL;
}

if (m_matchData)
{
pcre2_match_data_free(m_matchData);
m_matchData = nullptr;
}
}

inline bool CRegExp::IsValidSubNumber(int iSub) const
Expand Down
1 change: 1 addition & 0 deletions xbmc/utils/RegExp.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -134,6 +134,7 @@ class CRegExp
pcre2_match_context* m_ctxt;
static const int OVECCOUNT=(m_MaxNumOfBackrefrences + 1) * 3;
unsigned int m_offset;
pcre2_match_data* m_matchData;
PCRE2_SIZE* m_iOvector;
utf8Mode m_utf8Mode;
int m_iMatchCount;
Expand Down

0 comments on commit 6efd635

Please sign in to comment.