nothings / stb

stb single-file public domain libraries for C/C++
https://twitter.com/nothings
Other
26.31k stars 7.69k forks source link

stb_vorbis: Out-of-memory error when comment list length is zero #1471

Closed seanofw closed 1 year ago

seanofw commented 1 year ago

Describe the bug I have an Ogg Vorbis music file that I converted with Audacity from its original MP3 (the laziest possible way I could convert it). I attempted to load it with stb_vorbis, and it failed inside start_decoder() because comment_list_length is 0 in this file, and a size-zero malloc() fails on some operating systems (like Windows + MSVC).

To Reproduce

  1. Load the attached file into a buffer in memory.
  2. void *handle = stb_vorbis_open_pushdata(buffer, fileLength, &dataConsumed, &error, NULL);
  3. handle is incorrectly NULL, and error is set to "Out of Memory."

Expected behavior The decoder shouldn't crash when trying to open a file without comments in it.

I don't know the Ogg Vorbis format well enough to know if comment_list_length being 0 is legal, but the other software that I have that can play Ogg files can also play this music file, so I presume it's legal. I've attached it for reference (inside a .zip file, because GitHub).

Solution/workaround This is easily fixed by adding a simple, unfancy sz < 1 safety check inside setup_malloc():

static void *setup_malloc(vorb *f, int sz)
{
   sz = (sz+7) & ~7; // round up to nearest 8 for alignment of future allocs.
   if (sz < 1) sz = 1;
   f->setup_memory_required += sz;
   ...

fsm-team-happy-days.zip

N-R-K commented 1 year ago

This is easily fixed by adding a simple, unfancy sz < 1 safety check inside setup_malloc():

Proper fix would be to simply not allocate 0 len buffers to begin with. And just eyeballing the current source, it seems like it's been fixed for a while now https://github.com/nothings/stb/commit/b7b2aaa587977f19bc23c853371c72e4fdd8404c

Do you happen to be using an old version of the library?

seanofw commented 1 year ago

Looks like I'm on 1.20, which I hadn't realized was as old as it apparently is. I'll have to add "upgrade to the latest version" to my list of stuff to do tomorrow :)