Fix Heap buffer out of bounds write in start_decoder (GHSL-2023-167/CVE-2023-45677)

[JarLob]

A crafted file may trigger out of bounds write in f->vendor[len] = (char)'\0'; [1]. The root cause is that if len read in start_decoder [2] is a negative number and setup_malloc [3] successfully allocates memory in that case [4], but memory write is done with a negative index len [1].

   len = get32_packet(f); // [2]
   f->vendor = (char*)setup_malloc(f, sizeof(char) * (len+1)); // [3]
   if (f->vendor == NULL)                           return error(f, VORBIS_outofmem);
   for(i=0; i < len; ++i) {
      f->vendor[i] = get8_packet(f);
   }
   f->vendor[len] = (char)'\0'; // [1]

...

static void *setup_malloc(vorb *f, int sz)
{
   sz = (sz+7) & ~7; // round up to nearest 8 for alignment of future allocs.
   f->setup_memory_required += sz;
   if (f->alloc.alloc_buffer) {
      void *p = (char *) f->alloc.alloc_buffer + f->setup_offset; // [4]
      if (f->setup_offset + sz > f->temp_offset) return NULL;
      f->setup_offset += sz;
      return p;
   }
   return sz ? malloc(sz) : NULL;
}

Same vulnerability exists in setup_temp_malloc at [5]

static void *setup_temp_malloc(vorb *f, int sz)
{
   sz = (sz+7) & ~7; // round up to nearest 8 for alignment of future allocs.
   if (f->alloc.alloc_buffer) {
      if (f->temp_offset - sz < f->setup_offset) return NULL; // [5]
      f->temp_offset -= sz;
      return (char *) f->alloc.alloc_buffer + f->temp_offset;
   }
   return malloc(sz);
}

Similarly if len is INT_MAX the integer overflow len+1 happens in f->vendor = (char*)setup_malloc(f, sizeof(char) * (len+1)); [1] and f->comment_list[i] = (char*)setup_malloc(f, sizeof(char) * (len+1)); [6]. This case however allows writing multiple times past the end of the internal f->alloc.alloc_buffer buffer.

   for(i=0; i < f->comment_list_length; ++i) {
      len = get32_packet(f);
      f->comment_list[i] = (char*)setup_malloc(f, sizeof(char) * (len+1)); // [6]
      if (f->comment_list[i] == NULL)               return error(f, VORBIS_outofmem);

      for(j=0; j < len; ++j) {
         f->comment_list[i][j] = get8_packet(f);
      }
      f->comment_list[i][len] = (char)'\0';
   }

Impact

This issue may lead to code execution.

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,0xac,0xf4,0x30,
                            0x19,0x50,0x13,0x00,0x68,0x00,0x00,0x00,0x21,
                            0x00,0x40,0x00,0x00,0x00,0x7e,0x84,0x04,0x01,
                            0x1e,0x01,0x76,0x6f,0x72,0x62,0x69,0x73,0x00,
                            0x00,0x00,0x00,0x05,0x00,0x45,0xc5,0x87,0x03,
                            0x00,0x04,0x00,0x02,0x00,0x08,0x00,0x00,0x01,
                            0x00,0x2e,0xa9,0xcb,0x4f,0x67,0x67,0x53,0x00,
                            0x28,0x00,0x00,0xf7,0xff,0xff,0x7f,0x68,0x00,
                            0xff,0xff,0xff,0xff,0xff,0xff,0xff,0x76,0x6f,
                            0x72,0x73,0x00,0x00,0x03,0x76,0x6f,0x72,0x62,
                            0x69,0x73,0xff,0xff,0xff,0xee};
    size_t size = sizeof(data);

    stb_vorbis_alloc alloc;
    int alloc_buffer_length = 600 * 1024;
    alloc.alloc_buffer = (char*)malloc(alloc_buffer_length);
    alloc.alloc_buffer_length_in_bytes = alloc_buffer_length;
    int err;
    stb_vorbis* out = stb_vorbis_open_memory(data, size, &err, &alloc);
    stb_vorbis_close(out);
    free(alloc.alloc_buffer);
    return 0;
}

2. Run the program to hit the error.

AddressSanitizer:DEADLYSIGNAL
=================================================================
==302322==ERROR: AddressSanitizer: SEGV on unknown address 0x7f70bd3697ff (pc 0x0000004e41f4 bp 0x7ffc029b3070 sp 0x7ffc029b0be0 T0)
==302322==The signal is caused by a WRITE memory access.
    #0 0x4e41f4 in start_decoder(stb_vorbis*) tests/../stb_vorbis.c:3658:19
    #1 0x4f9444 in stb_vorbis_open_memory tests/../stb_vorbis.c:5112:8
    #2 0x4fd8e9 in main tests/stb_vorbis_fuzzer.c:24:23

[sezero]

(See comment at #1554 (comment) )