floriscreyf / mp3-decoder Goto Github PK

problem - cannot get length of buffer
you are using std::ifstream, why not use std::vector or std::string?

Also, such a mix is a sign of error-prone code style.
you may easily forget delete[] operator.

Do it modern way!
here is replacement version, 4 lines of code, each does bunch of things
Logic is extremely clear:
1. open file
2. create buffer
3. read file to buffer
4 return it

std::string get_file(const std::string& fileName) {
std::ifstream dataFile(dir, std::ios::in | std::ios::binary | std::ios::ate);
std::string dataBuffer(dataFile.tellg(),0);
dataFile.seekg(0, std::ios::beg).read(&dataBuffer[0], dataBuffer.size());
return std::move( dataBuffer );
}

Hi there,

Reporting a bug in parsing ID3 parsing, see debugging info and repro file attached. Did not push further on exploitability analysis, but you can see the basics in the crash log below. Could be straightforward to fix due to suspected crashing via malformed ID3 data with some bounds checks / verification in mp3 files.

crash.zip

cmdline ['mp3-decoder/mp3decoder', 'crash.mp3'] exited with abnormal termination condition (SIGABRT)

$ mp3-decoder/mp3decoder crash.mp3

(or to run with electric fence)

$ LD_PRELOAD=/usr/lib/libefence.so mp3-decoder/mp3decoder crash.mp3

Starting program: mp3-decoder/mp3decoder crash.mp3

*** stack smashing detected ***: terminated

Program received signal SIGABRT, Aborted.
__pthread_kill_implementation (no_tid=0, signo=6, threadid=140737352562496) at ./nptl/pthread_kill.c:44
44 ./nptl/pthread_kill.c: No such file or directory.
#0 __pthread_kill_implementation (no_tid=0, signo=6, threadid=140737352562496)
at ./nptl/pthread_kill.c:44
#1 __pthread_kill_internal (signo=6, threadid=140737352562496)
at ./nptl/pthread_kill.c:78
#2 __GI___pthread_kill (threadid=140737352562496, signo=signo@entry=6)
at ./nptl/pthread_kill.c:89
#3 0x00007ffff7842476 in __GI_raise (sig=sig@entry=6)
at ../sysdeps/posix/raise.c:26
#4 0x00007ffff78287f3 in __GI_abort () at ./stdlib/abort.c:79
#5 0x00007ffff7889676 in __libc_message (action=action@entry=do_abort,
fmt=fmt@entry=0x7ffff79db92e "*** %s **: terminated\n")
at ../sysdeps/posix/libc_fatal.c:155
#6 0x00007ffff793659a in __GI___fortify_fail (
msg=msg@entry=0x7ffff79db916 "stack smashing detected")
at ./debug/fortify_fail.c:26
#7 0x00007ffff7936566 in __stack_chk_fail () at ./debug/stack_chk_fail.c:24
#8 0x000055555555a40f in id3::set_version(unsigned char, unsigned char) ()
#9 0x000055555555a0a2 in id3::id3(unsigned char) ()
#10 0x00005555555892aa in get_id3_tags(std::vector<unsigned char, std::allocator >&, unsigned int&) ()
#11 0x0000555555589470 in main ()

rax 0x0 0
rbx 0x7ffff7e7f740 140737352562496
rcx 0x7ffff78969fc 140737346365948
rdx 0x6 6
rsi 0x49dd1 302545
rdi 0x49dd1 302545
rbp 0x49dd1 0x49dd1
rsp 0x7fffffff5cc0 0x7fffffff5cc0
r8 0x7fffffff5d90 140737488313744
r9 0x0 0
r10 0x8 8
r11 0x246 582
r12 0x6 6
r13 0x16 22
r14 0x2 2
r15 0x1 1
rip 0x7ffff78969fc 0x7ffff78969fc <__GI___pthread_kill+300>
eflags 0x246 [ PF ZF IF ]
cs 0x33 51
ss 0x2b 43
ds 0x0 0
es 0x0 0
fs 0x0 0
gs 0x0 0
=> 0x7ffff78969fc <__GI___pthread_kill+300>: mov %eax,%r13d
0x7ffff78969ff <__GI___pthread_kill+303>: neg %r13d
0x7ffff7896a02 <__GI___pthread_kill+306>: cmp $0xfffff000,%eax
0x7ffff7896a07 <__GI___pthread_kill+311>: mov $0x0,%eax

'exploitable' version 1.32
Linux dev 6.5.0-1021-aws #21~22.04.1-Ubuntu SMP Fri May 10 20:04:44 UTC 2024 x86_64
Signal si_signo: 6 Signal si_addr: 4294967598545
Nearby code:
Stack trace:
0 __pthread_kill_implementation at 0x7ffff78969fc in r-xp /usr/lib/x86_64-linux-gnu/libc.so.6 (BL)
1 __pthread_kill_internal at 0x7ffff78969fc in r-xp /usr/lib/x86_64-linux-gnu/libc.so.6 (BL)
2 __GI___pthread_kill at 0x7ffff78969fc in r-xp /usr/lib/x86_64-linux-gnu/libc.so.6 (BL)
3 __GI_raise at 0x7ffff7842476 in r-xp /usr/lib/x86_64-linux-gnu/libc.so.6 (BL)
4 __GI_abort at 0x7ffff78287f3 in r-xp /usr/lib/x86_64-linux-gnu/libc.so.6 (BL)
5 __libc_message at 0x7ffff7889676 in r-xp /usr/lib/x86_64-linux-gnu/libc.so.6 (BL)
6 __GI___fortify_fail at 0x7ffff793659a in r-xp /usr/lib/x86_64-linux-gnu/libc.so.6 (BL)
7 __stack_chk_fail at 0x7ffff7936566 in r-xp /usr/lib/x86_64-linux-gnu/libc.so.6 (BL)
8 id3::set_version(unsigned char, unsigned char) at 0x55555555a40f in r-xp mp3-decoder/mp3decoder
9 id3::id3(unsigned char*) at 0x55555555a0a2 in r-xp mp3-decoder/mp3decoder
10 get_id3_tags(std::vector<unsigned char, std::allocator >&, unsigned int&) at 0x5555555892aa in r-xp mp3-decoder/mp3decoder
11 main at 0x555555589470 in r-xp mp3-decoder/mp3decoder
Faulting frame: # 8 id3::set_version(unsigned char, unsigned char) at 0x55555555a40f in r-xp mp3-decoder/mp3decoder

Description: Stack buffer overflow
Short description: StackBufferOverflow (6/22)
Hash: ebbcde9aee16565b0d779ead4f96960d.bcd2531ee1ae7eee747435b9688099b3
Exploitability Classification: EXPLOITABLE
Explanation: The target stopped while handling a signal that was generated by libc due to detection of a stack buffer overflow. Stack buffer overflows are generally considered exploitable.
Other tags: PossibleStackCorruption (7/22), AbortSignal (20/22)

The offending line is

set_side_info(&buffer[4]);

which fails to consider the case in which the CRC bit is 0, which would make the index 6 instead of 4.

Hi there,
dunno if this project is still active but - since I'm trying to help this project to obtain better quality results (at least in the decoding stage) - it would be interesting to understand if lossy decoding to fp64 PCM would help.

I recalled an old - but still valid - comparison about MP3 decoders quality:

...and it claims (google-translated from here):

There is another solution for decoding mp3 files that has been around for less than a year and whose technical specifications are quite impressive, and which should offer a quality that is significantly higher than that of MAD. This software is foobar2000, and uses the mpglib library as a decoder, largely modified by Peter Pawlowski. This decoding solution has the disadvantage of not supporting freeformat encodings, but as advantages, both theoretical and practical, we find the following characteristics:

• decoding performed in 64 floating bits
• integration of a disengageable dithering algorithm, with three noise shaping modes to limit its negative impact
• full consideration of offsets, allowing gapless chaining of lame encodings (automatic) and any other mp3 encoder (manual)
• full support for replaygain
• management of several tag formats (ID3v1, ID3v2.x and APEv2)

All of these qualities are applicable to formats other than mp3, and - this is the most important aspect - are perfectly reproducible in the form of a test PCM file thanks to the complete diskwriter available with the software. Integrating foobar2000 should allow us to measure the contribution of the noise shaping technique, an essential complement to dithering that MAD strangely ignores. This technique should push back the nuisances linked to dithering (background noise) while preserving its contribution (higher resolution). From a theoretical point of view at least, foobar2000 should offer qualitatively superior decoding to that of MAD.

This customized mpglib it's unfortunally no longer developed, but the source code was shared from its author here.

Thanks in advance for any hint you can provide about.

Hi, I commented out stream() definition and use in main.cpp but that was not enough to make it compile.

Severity	Code	Description	Project	File	Line	Suppression State
Error	LNK2005	"struct <unnamed-type-band_index_table> band_index_table" (?band_index_table@@3U<unnamed-type-band_index_table>@@A) already defined in main.obj	mp3-decoder	Z:\mp3-decoder-master\mp3.obj	1	
Error	LNK2005	"struct <unnamed-type-band_width_table> band_width_table" (?band_width_table@@3U<unnamed-type-band_width_table>@@A) already defined in main.obj	mp3-decoder	Z:\mp3-decoder-master\mp3.obj	1	
Error	LNK2005	"struct <unnamed-type-quad_table_1> quad_table_1" (?quad_table_1@@3U<unnamed-type-quad_table_1>@@A) already defined in main.obj	mp3-decoder	Z:\mp3-decoder-master\mp3.obj	1	
Error	LNK1169	one or more multiply defined symbols found	mp3-decoder	Z:\mp3-decoder-master\Debug\mp3-decoder.exe	1	

I'm not experienced enough to be able to solve this. I'm guessing all these are related:

• https://docs.microsoft.com/en-us/cpp/error-messages/tool-errors/linker-tools-error-lnk2005?f1url=https%3A%2F%2Fmsdn.microsoft.com%2Fquery%2Fdev15.query%3FappId%3DDev15IDEF1%26l%3DEN-US%26k%3Dk(LNK2005)%26rd%3Dtrue
• https://stackoverflow.com/questions/1433204/how-do-i-use-extern-to-share-variables-between-source-files
• https://stackoverflow.com/questions/2125209/linker-errors-even-though-i-prevent-them-with-ifndef

PS: also id3.cpp was causing failure in line 111 while (!std::regex_match((string){(char)buffer[i]}, re) && i < size) {, which I turned into

	string istep;
	istep = (char)buffer[0];

	while (!std::regex_match(istep, re) && i < size) {
		istep = (char)buffer[i];