A global-buffer-overflow bug was found in MP3_Handler.cpp
0x00 Introduction
In exempi 2.5.0, I found a global-buffer-overflow bug in MP3_Handler.cpp, the ASAN report is as below.
liwc@ubuntu:~/exempi-master_asan/exempi$ ./exempi -x poc2
processing file poc2
dump_xmp for file poc2
=================================================================
==80231==ERROR: AddressSanitizer: global-buffer-overflow on address 0x0000006cdce0 at pc 0x0000004dbb24 bp 0x7ffc0ad43a50 sp 0x7ffc0ad43a40
READ of size 4 at 0x0000006cdce0 thread T0
#0 0x4dbb23 in GetUns32BE ../../../source/EndianUtils.hpp:152
#1 0x4de11f in MP3_MetaHandler::ProcessXMP() /home/liwc/exempi-master/XMPFiles/source/FileHandlers/MP3_Handler.cpp:320
#2 0x48aafd in XMPFiles::GetXMP(TXMPMeta<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > >*, char const**, unsigned int*, XMP_PacketInfo*) /home/liwc/exempi-master/XMPFiles/source/XMPFiles.cpp:1471
#3 0x47fac8 in WXMPFiles_GetXMP_1 /home/liwc/exempi-master/XMPFiles/source/WXMPFiles.cpp:331
#4 0x41e353 in TXMPFiles<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > >::GetXMP(TXMPMeta<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > >*, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >*, XMP_PacketInfo*) (/home/liwc/exempi-master_asan/exempi/exempi+0x41e353)
#5 0x40c0f1 in xmp_files_get_new_xmp /home/liwc/exempi-master/exempi/exempi.cpp:346
#6 0x408d07 in get_xmp_from_file /home/liwc/exempi-master/exempi/main.cpp:244
#7 0x408ece in dump_xmp /home/liwc/exempi-master/exempi/main.cpp:257
#8 0x409b54 in process_file /home/liwc/exempi-master/exempi/main.cpp:348
#9 0x408728 in main /home/liwc/exempi-master/exempi/main.cpp:194
#10 0x7f2eeb8bc82f in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x2082f)
#11 0x407a58 in _start (/home/liwc/exempi-master_asan/exempi/exempi+0x407a58)
0x0000006cdce1 is located 0 bytes to the right of global variable '*.LC33' defined in 'MP3_Handler.cpp' (0x6cdce0) of size 1
'*.LC33' is ascii string ''
SUMMARY: AddressSanitizer: global-buffer-overflow ../../../source/EndianUtils.hpp:152 GetUns32BE
Shadow bytes around the buggy address:
0x0000800d1b40: f9 f9 f9 f9 04 f9 f9 f9 f9 f9 f9 f9 00 03 f9 f9
0x0000800d1b50: f9 f9 f9 f9 05 f9 f9 f9 f9 f9 f9 f9 04 f9 f9 f9
0x0000800d1b60: f9 f9 f9 f9 00 00 00 05 f9 f9 f9 f9 00 03 f9 f9
0x0000800d1b70: f9 f9 f9 f9 05 f9 f9 f9 f9 f9 f9 f9 04 f9 f9 f9
0x0000800d1b80: f9 f9 f9 f9 05 f9 f9 f9 f9 f9 f9 f9 04 f9 f9 f9
=>0x0000800d1b90: f9 f9 f9 f9 05 f9 f9 f9 f9 f9 f9 f9[01]f9 f9 f9
0x0000800d1ba0: f9 f9 f9 f9 05 f9 f9 f9 f9 f9 f9 f9 04 f9 f9 f9
0x0000800d1bb0: f9 f9 f9 f9 00 00 02 f9 f9 f9 f9 f9 05 f9 f9 f9
0x0000800d1bc0: f9 f9 f9 f9 04 f9 f9 f9 f9 f9 f9 f9 07 f9 f9 f9
0x0000800d1bd0: f9 f9 f9 f9 05 f9 f9 f9 f9 f9 f9 f9 04 f9 f9 f9
0x0000800d1be0: f9 f9 f9 f9 00 01 f9 f9 f9 f9 f9 f9 05 f9 f9 f9
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
Heap right redzone: fb
Freed heap region: fd
Stack left redzone: f1
Stack mid redzone: f2
Stack right redzone: f3
Stack partial redzone: f4
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
==80231==ABORTING
0x01 Analysis
I think this issue was a fault caused by careless. From line 43 to line 67, a const static array was declared.
struct ReconProps {
const char* mainID; // The stored v2.3 and v2.4 ID, also used as the main logical ID.
const char* v22ID; // The stored v2.2 ID.
const char* ns;
const char* prop;
};The element of reconProps whose mainID was "TDRC" had an incomplete v22ID.
const static ReconProps reconProps[] = {
{ "TPE1", "TP1", kXMP_NS_DM, "artist" },
{ "TALB", "TAL", kXMP_NS_DM, "album" },
{ "TRCK", "TRK", kXMP_NS_DM, "trackNumber" },
// exceptions that need attention:
{ "TCON", "TCO", kXMP_NS_DM, "genre" }, // genres might be numeric
{ "TIT2", "TT2", kXMP_NS_DC, "title" }, // ["x-default"] language alternatives
{ "COMM", "COM", kXMP_NS_DM, "logComment" }, // two distinct strings, language alternative
{ "TYER", "TYE", kXMP_NS_XMP, "CreateDate" }, // Year (YYYY) Deprecated in 2.4
{ "TDAT", "TDA", kXMP_NS_XMP, "CreateDate" }, // Date (DDMM) Deprecated in 2.4
{ "TIME", "TIM", kXMP_NS_XMP, "CreateDate" }, // Time (HHMM) Deprecated in 2.4
{ "TDRC", "", kXMP_NS_XMP, "CreateDate" }, // assembled date/time v2.4 <= miss v22id
// new reconciliations introduced in Version 5
{ "TCMP", "TCP", kXMP_NS_DM, "partOfCompilation" }, // presence/absence of TCMP frame dedides
{ "USLT", "ULT", kXMP_NS_DM, "lyrics" },
{ "TCOM", "TCM", kXMP_NS_DM, "composer" },
{ "TPOS", "TPA", kXMP_NS_DM, "discNumber" }, // * a text field! might contain "/<total>"
{ "TCOP", "TCR", kXMP_NS_DC, "rights" }, // ["x-default"] language alternatives
{ "TPE4", "TP4", kXMP_NS_DM, "engineer" },
{ "WCOP", "WCP", kXMP_NS_XMP_Rights, "WebStatement" },
{ 0, 0, 0, 0 } // must be last as a sentinel
};The call of GetUns32BE at MP3_Handler.cpp:320 passes a "" when r is 9, and the storedID will be a unexpected value.
315 for ( int r = 0; reconProps[r].mainID != 0; ++r ) {
316
317 //get the frame ID to look for
318 XMP_Uns32 logicalID = GetUns32BE ( reconProps[r].mainID );
319 XMP_Uns32 storedID = logicalID;
320 if ( this->majorVersion == 2 ) storedID = GetUns32BE ( reconProps[r].v22ID );0x02 Reproduce
OS:Ubuntu 16.04 x86_64
export CFLAGS="-fsanitize=address -ggdb"
export CXXFLAGS="-fsanitize=address -ggdb"
export LDFLAGS="-fsanitize=address -ggdb"
sudo apt install libboost-dev
sudo apt install libboost-test-dev
./autogen.sh
./configure --disable-shared
make
./exampi/exempi -x POC -o out0x03 Discoverer
WenChao Li of VARAS@IIE