Fix crash - Avoid nullptr dereference in vorbis_deinit

[DanielJamesCollier]

A corrupt .ogg file can cause a setup_malloc failure inside start_decoder. Specifically in this code:

if (f->comment_list_length > 0)
{
   f->comment_list = (char**) setup_malloc(f, sizeof(char*) * (f->comment_list_length));
   if (f->comment_list == NULL)                  return error(f, VORBIS_outofmem); // <<--------comment_list is NULL if setup_malloc fails.
}

Since setup_malloc can fail, we need to guard against NULL in vorbis_deinit. The subarrays can also be NULL if their allocations fail, so additional checks have been added for those as well.

---

Repro steps if needed/wanted

1. Download end extract the repo.zip somewhere. It contains a corrupted .ogg file and a minimal main.c which hits the crash: repro.zip
2. Grab a copy of stb_vorbis.c from master and put it in the same folder.
3. Compile (obviously need to run vcvarsall.bat or whatever) : cl /Zi /Od main.c
4. Run main.exe and see that it crashes. "After stb_vorbis_open_filename..." does not get printed. The output will look as follows:

C:\dev\stb_vorbis_crash_fix>main.exe
Before stb_vorbis_open_filename...

5. Of course, do the same again but with my changes to stb_vorbis.c and see that there are no crashes. The output will be as follows:

C:\dev\stb_vorbis_crash_fix>main
Before stb_vorbis_open_filename...
After stb_vorbis_open_filename...
Failed to open file (error 3)

Note: the "Failed to open file" is expected because start_decoder() fails which causes vorbis_deinit() to get called. so stb_vorbis_open_filename() returns NULL. 6. Profit!

I tried to make the formatting and style follow the surrounding code, but let me know if it needs changing.

PS: Thanks so much for the stb libraries!

[NBickford-NV]

Thanks Daniel! Adding a quick note here that another way to fix this is to set f->comment_list_length to 0 if the comment_list allocation fails, like in sezero's comment for #1557: https://github.com/nothings/stb/pull/1557#issuecomment-1848997604

[DanielJamesCollier]

@NBickford-NV ah! For what it is worth, I chose this way to fix it because it is more in line with the style of the surrounding code in vorbis_deinit, as most other things in that function have NULL checks.

I didn't come across that PR before. I found this issue independently. Any idea why it was closed?

[NBickford-NV]

@DanielJamesCollier I'm not sure, unfortunately! I'd have to ask @JarLob. (I'm not a maintainer though could step up if stb would like; I just maintain a fork.)

[JarLob]

It was closed automatically because I deleted the fork and forgot that it will close any PRs. The PR was hanging for almost a year. The maintainer expressed an opinion the library should not operate on untrusted data.