vgmrips / vgmplay-legacy

VGM file command-line player and Winamp plugin.
http://vgmrips.net
221 stars 52 forks source link

malloc(0) and divide by zero #50

Closed cuanduo closed 5 years ago

cuanduo commented 5 years ago

if we set SampleRate = 0; in vgmplay.ini will cause divide by zero as follows

Program received signal SIGFPE, Arithmetic exception.
0x0000555555564274 in SamplePbk2VGM_I (SampleVal=0) at VGMPlay.c:3969
3969        return (INT32)((INT64)SampleVal * VGMSmplRateDiv / VGMSmplRateMul);
(gdb) bt
#0  0x0000555555564274 in SamplePbk2VGM_I (SampleVal=0) at VGMPlay.c:3969
#1  0x00005555555650a5 in InterpretVGM (SampleCount=0) at VGMPlay.c:4741
#2  0x0000555555564487 in InterpretFile (SampleCount=0) at VGMPlay.c:4178
#3  0x000055555555c427 in PlayVGM () at VGMPlay.c:1058
#4  0x000055555555a6d7 in PlayVGM_UI () at VGMPlayUI.c:2132
#5  0x0000555555556d25 in main (argc=3, argv=0x7fffffffe048) at VGMPlayUI.c:530
(gdb) p VGMSmplRateMul 
$20 = 0

if set SampleRate = 1; and commandline with -LogSound:1 will cause malloc(0) and may cause heap overflow

Breakpoint 6, __GI___libc_malloc (bytes=0) at malloc.c:3028
3028    malloc.c: No such file or directory.
(gdb) bt
#0  __GI___libc_malloc (bytes=0) at malloc.c:3028
#1  0x000055555555a96e in PlayVGM_UI () at VGMPlayUI.c:2222
#2  0x0000555555556d25 in main (argc=3, argv=0x7fffffffe048) at VGMPlayUI.c:530
ValleyBell commented 5 years ago

Well, if you want to break VGMPlay, there are more creative ways to do it. Like partially broken VGMs, for example.
Setting the sample rate to anything smaller than 1000 won't result in any useful sound anyway.

So I consider this a "not worth fixing", as nobody sane would use those configurations. (And I verified that with SampleRate = 1, the buffer is never used.)

I also edited your post with code blocks, so that it doesn't do unwanted references to other issues.