Decoder hardening: overflow, arena, and codebook fixes

src/tremor/CHANGES.md

@@ -159,6 +159,30 @@ and `a629068d`, May 2026):
   rejects referenced codebooks with no value mapping (`dec_type == 0`,
   i.e. maptype-0 books, unusable by `vorbis_book_decodev_set`) or
   `dim < 1`, and rejects `numbooks < 1`.
+- **Tree-walk fall-off forces end-of-packet** (`codebook.c`): when
+  `decode_packed_entry_number` chases a codebook's decode tree through all
+  `dec_maxlength` bits without reaching a leaf, it now sets the overrun
+  sentinel (`oggpack_seteop`) rather than relying on the trailing
+  `oggpack_adv(b, read+1)` to trip it. That advance's overflow check is
+  byte-granular, so slack in the final partial byte could leave end-of-packet
+  unset and let the `0xffffffff` entry reach `decode_map_apply`, which would
+  then dequantize garbage (`dec_type 1`) or read `q_val` out of bounds
+  (`dec_type 2`, when `quantvals` is not a power of two). Only a one-used-entry
+  book reaches this path: fuller trees are complete or rejected by the
+  underpopulated-tree check in `_make_words`, and a valid stream only emits
+  that book's single codeword, so it never falls off. The `dec_type 3` index
+  bound is kept as a secondary guard.
+- **Zero-entry codebooks parse on ESP-IDF** (`codebook.c`): `entries` is a
+  legal 24-bit field, and `_make_decode_table` is deliberately written to
+  accommodate 0- and 1-sized books (the `nodeb==4` special case). But the
+  `lengthlist` allocations (unordered and ordered cases) and the maptype-2
+  `dec_type 3` `q_val` allocation size themselves from `entries`/`used_entries`,
+  so a zero-entry book requests 0 bytes. `heap_caps_malloc(0)` returns `NULL`
+  on ESP-IDF (glibc returns a unique non-`NULL` pointer), which the `if(!p)`
+  checks would misread as OOM and reject the book, causing a stream to decode
+  on-host but not on-target. The three sizes are now clamped to at least one
+  byte; the read/pack loops that follow are empty when the count is zero, so
+  the extra byte is never touched.
 
 ### Performance changes relative to the lowmem branch
 
@@ -252,7 +276,16 @@ and `a629068d`, May 2026):
     recurses into the floor/residue `arena_size` callbacks per submap. The mirror
     is exact: the computed size equals the arena's final used watermark. A
     256-byte `DSP_ARENA_SAFETY` pad is added as insurance against an
-    overlooked site or platform `sizeof` drift.
+    overlooked site or platform `sizeof` drift. `_vorbis_dsp_arena_compute_size`
+    accumulates and returns the total in `ogg_int64_t`: a crafted header can
+    point up to 64 modes * 16 submaps at one residue whose decodemap is hundreds
+    of MB (`res012.c`), and although each per-mode term is a valid `long`, the
+    sum can exceed `2^31` and would wrap a 32-bit `long` on the ESP32 target to a
+    small value. `_ogg_malloc` would then succeed undersized and `res0_look`'s
+    unchecked arena allocations would scribble past the buffer. `_vds_init`
+    rejects any header whose 64-bit total exceeds `DSP_ARENA_MAX_BYTES`
+    (`2^31-1`, the largest arena a `long` offset can address) via the existing
+    `-1` init-failure path before the allocation.
   - **Per-look freeing is gone**: the backend `free_look` hooks are now no-ops
     and `vorbis_dsp_clear` releases the entire DSP state in a single
     `_ogg_free(setup_arena_data)`. `_vds_init` returns `-1` if the one arena
@@ -459,6 +492,17 @@ compiler-defined `#ifdef __XTENSA__`.
   related `q_pack * dim > 32` unpack-time rejection is described in the
   codebook hardening section.
 
+- **Negative shift exponent from a truncated id header** (`info.c`):
+  `_vorbis_unpack_info` set `ci->blocksizes[i] = 1 << oggpack_read(opb,4)`
+  directly. On a truncated identification packet `oggpack_read` returns its
+  `-1` end-of-packet sentinel, making `1 << -1` a shift by a negative count
+  (UB; UBSan `shift-exponent-negative`). The two reads are now taken into
+  locals and rejected if negative before the shift, mirroring the
+  `rangebits` guard in `floor1.c`; the existing EOP check only fires after
+  the shift. Reachable only through the exported `vorbis_synthesis_headerin`
+  API. The C++ wrapper's 30-byte identification-header minimum keeps every
+  in-tree path away from it.
+
 - **Out-of-bounds value-array index from a sparse codebook**
   (`codebook.c`): the lowmem `_make_words` carries only the
   overpopulated-tree check. Master's `sharedbook.c` also rejected an

src/tremor/block.c

@@ -154,11 +154,26 @@ static long _vorbis_arena_compute_size(vorbis_info *vi){
         if(pw>max_partwords_2) max_partwords_2=pw;
       }
     }
-    /* _01inverse: outer array + per-ch inner arrays */
+    /* _01inverse: outer array + per-ch inner arrays. Its `end` cap is pcmend/2
+       (channel-independent) and it allocates `ch` inner arrays per submap, so
+       summed over all submaps the inner arrays total channels*max_partwords_01
+       (the submaps partition the channels) - already covered above. */
     size += channels * (long)sizeof(int **);
     size += channels * max_partwords_01 * (long)sizeof(int *);
-    /* res2: separate partword array */
-    size += max_partwords_2 * (long)sizeof(int *);
+    /* res2: one partword array per submap. mapping0_inverse calls the residue
+       inverse once per submap (mapping0.c) and res2_inverse allocates its
+       partword array unconditionally (res012.c); the block arena is reset only
+       between packets, so every submap's array is live at once. Unlike res0/1,
+       res2's array size is channel-independent when info->end is the binding cap
+       (each submap then allocates the full max_partwords_2, not a ch-scaled
+       share), so a single reservation undercounts by the submap count. A submap
+       must carry >=1 channel to allocate (ch==0 -> n<=0 -> no alloc) and the
+       submaps partition the channels, so at most min(channels,submaps) res2
+       arrays coexist; submaps is a 4-bit field (<=16). Reserve that many. */
+    {
+      long max_res2_submaps = channels < 16 ? channels : 16;
+      size += max_res2_submaps * max_partwords_2 * (long)sizeof(int *);
+    }
   }
 
   /* mapping0 ARENA_STACK allocations (4 arrays of channels pointers/ints) */
@@ -177,8 +192,11 @@ static long _vorbis_arena_compute_size(vorbis_info *vi){
        channels   pcm data buffers         (synthesis.c)
        4          mapping bundles          (mapping0.c: pcm/zero/nonzero/floormemo)
        channels   floor memos              (floor0/1 inverse1)
-       channels   residue inner partwords  (res012.c, summed over submaps)
-       channels   residue outer partwords  (res012.c, per non-empty submap)
+       channels   residue inner arrays     (res012.c: res0/1 per-channel inner
+                                             arrays, summed over submaps)
+       channels   residue outer arrays     (res012.c: res0/1 outer array or res2
+                                             partword array - one per submap, and
+                                             the submaps partition the channels)
      The slack has to scale with channels (Vorbis allows up to 255): the
      decode path dereferences _vorbis_block_alloc's NULL return unchecked,
      so an undersized arena is a crash, not a clean failure. */
@@ -212,6 +230,17 @@ int vorbis_block_clear(vorbis_block *vb){
    next to the arena itself (which is dominated by residue decodemaps). */
 #define DSP_ARENA_SAFETY 256
 
+/* Ceiling on the DSP setup arena. The arena is addressed with `long` offsets
+   (setup_arena_used/_capacity) and handed to a single _ogg_malloc, so on the
+   ILP32 target (ESP32, 32-bit long) it can never exceed 2^31-1 bytes. A crafted
+   header - many modes/submaps all pointing at one residue whose decodemap is
+   hundreds of MB (res012.c) - can drive _vorbis_dsp_arena_compute_size's mirror
+   total past that. Summed into a 32-bit long it would wrap to a small value,
+   _ogg_malloc would then succeed undersized, and res0_look's unchecked arena
+   allocations would scribble past the buffer. The total is computed in 64 bits
+   and any stream over this cap is rejected before the malloc. */
+#define DSP_ARENA_MAX_BYTES 0x7fffffffLL
+
 /* Compute the size of the DSP setup arena from codec_setup_info. Mirrors the
    top-level allocations in _vds_init and, through the per-backend arena_size
    vtable entries, every mode/floor/residue lookup that mapping0_look builds.
@@ -224,11 +253,14 @@ int vorbis_block_clear(vorbis_block *vb){
    mask is fixed before the arena is sized, dropped channels are never allocated
    at all, so there is nothing to free later and the whole DSP state collapses to
    this single allocation. */
-static long _vorbis_dsp_arena_compute_size(vorbis_dsp_state *v,const unsigned char *keep){
+/* Returns the mirrored arena size in 64-bit so a maliciously large mode/submap
+   fan-out (each per-mode term is a valid long, but up to 64 modes * 16 submaps
+   can sum past 2^31) cannot wrap; the caller enforces DSP_ARENA_MAX_BYTES. */
+static ogg_int64_t _vorbis_dsp_arena_compute_size(vorbis_dsp_state *v,const unsigned char *keep){
   vorbis_info *vi=v->vi;
   codec_setup_info *ci=(codec_setup_info *)vi->codec_setup;
   int channels=vi->channels;
-  long size=0;
+  ogg_int64_t size=0;
   int i;
 
   size+=_vorbis_arena_round(sizeof(private_state));                 /* backend_state */
@@ -257,7 +289,7 @@ static long _vorbis_dsp_arena_compute_size(vorbis_dsp_state *v,const unsigned ch
 
 static int _vds_init(vorbis_dsp_state *v,vorbis_info *vi,const unsigned char *keep){
   int i;
-  long arena_size;
+  ogg_int64_t arena_size;
   codec_setup_info *ci=(codec_setup_info *)vi->codec_setup;
   private_state *b=NULL;
 
@@ -276,10 +308,17 @@ static int _vds_init(vorbis_dsp_state *v,vorbis_info *vi,const unsigned char *ke
      is fixed here, before sizing, so dropped channels' history buffers are never
      allocated rather than allocated-then-freed. */
   arena_size=_vorbis_dsp_arena_compute_size(v,keep);
-  v->setup_arena_data=_ogg_malloc(arena_size+DSP_ARENA_SAFETY);
+  /* Reject a header whose mirrored arena can't fit a `long` (see
+     DSP_ARENA_MAX_BYTES). Leaving room for DSP_ARENA_SAFETY keeps the malloc
+     argument and setup_arena_capacity within a positive long on the 32-bit
+     target. A genuinely large but representable arena still fails cleanly at the
+     _ogg_malloc NULL check below. */
+  if(arena_size<0 || arena_size>DSP_ARENA_MAX_BYTES-DSP_ARENA_SAFETY)
+    return -1;
+  v->setup_arena_data=_ogg_malloc((long)arena_size+DSP_ARENA_SAFETY);
   if(!v->setup_arena_data)
     return -1;
-  v->setup_arena_capacity=arena_size+DSP_ARENA_SAFETY;
+  v->setup_arena_capacity=(long)arena_size+DSP_ARENA_SAFETY;
   v->setup_arena_used=0;
 
   b=(private_state *)(v->backend_state=_vorbis_setup_calloc(v,1,sizeof(*b)));

src/tremor/codebook.c

@@ -49,6 +49,16 @@ static inline int oggpack_eop(oggpack_buffer *b){
   return (b->ptr == NULL) ? -1 : 0;
 }
 
+/* Force the end-of-packet (overrun) sentinel, mirroring the state that
+   bitwise.c's oggpack_read/oggpack_adv set on overflow. Used where the
+   decoder must signal a corrupt packet but no read actually overran the
+   buffer, so oggpack_eop would otherwise report "no overrun". */
+static inline void oggpack_seteop(oggpack_buffer *b){
+  b->ptr=NULL;
+  b->endbyte=b->storage;
+  b->endbit=1;
+}
+
 /**** pack/unpack helpers ******************************************/
 int _ilog(unsigned int v){
   int ret=0;
@@ -436,8 +446,12 @@ int vorbis_book_unpack(oggpack_buffer *opb,codebook *s){
     if((s->entries*(unused?1:5)+7)>>3>opb->storage-oggpack_bytes(opb))
       goto _eofout;
     /* unordered */
-    /* ESP32 stack safety: use heap for lengthlist (up to 8192 entries) */
-    lengthlist=(char *)_ogg_malloc(sizeof(*lengthlist)*s->entries);
+    /* ESP32 stack safety: use heap for lengthlist (entries is a 24-bit field).
+       Request >=1 byte: entries==0 is a legal 24-bit book that the decode
+       table builder accommodates (nodeb==4 special case), but heap_caps_malloc(0)
+       returns NULL on ESP-IDF (glibc returns non-NULL), which would otherwise
+       reject the book. The read loop below is empty when entries==0. */
+    lengthlist=(char *)_ogg_malloc(s->entries?sizeof(*lengthlist)*s->entries:1);
     if(!lengthlist)goto _eofout;
 
     /* allocated but unused entries? */
@@ -474,8 +488,10 @@ int vorbis_book_unpack(oggpack_buffer *opb,codebook *s){
       if(length==0)goto _eofout;
 
       s->used_entries=s->entries;
-      /* ESP32 stack safety: use heap for lengthlist */
-      lengthlist=(char *)_ogg_malloc(sizeof(*lengthlist)*s->entries);
+      /* ESP32 stack safety: use heap for lengthlist. >=1 byte so a legal
+         entries==0 book is not rejected by heap_caps_malloc(0)==NULL on
+         ESP-IDF (see the unordered case above). */
+      lengthlist=(char *)_ogg_malloc(s->entries?sizeof(*lengthlist)*s->entries:1);
       if(!lengthlist)goto _eofout;
 
       for(i=0;i<s->entries;){
@@ -625,7 +641,9 @@ int vorbis_book_unpack(oggpack_buffer *opb,codebook *s){
 
       /* get the vals & pack them */
       s->q_pack=(s->q_bits+7)/8*s->dim;
-      s->q_val=_ogg_codebook_malloc(s->q_pack*s->used_entries);
+      /* >=1 byte: used_entries==0 (a legal zero-entry book) makes this a
+         size-0 request, NULL on ESP-IDF. The pack loop below is then empty. */
+      s->q_val=_ogg_codebook_malloc(s->used_entries?s->q_pack*s->used_entries:1);
       if(!s->q_val)goto _eofout;
 
       if(s->q_bits<=8){
@@ -736,7 +754,17 @@ static inline ogg_uint32_t decode_packed_entry_number(codebook *book,
     oggpack_adv(b,i+1);
     return chase;
   }
-  oggpack_adv(b,read+1);
+  /* The walk consumed all dec_maxlength bits without reaching a leaf. A
+     valid stream never gets here: a book's tree is fully populated, and the
+     lone exception (a one-used-entry book, exempted from the underpopulated
+     reject in _make_words) only ever sees its single codeword. So this is
+     corrupt input. The original oggpack_adv(b,read+1) was meant to overrun
+     and set EOP, but the overflow check is byte-granular -- slack in the
+     final partial byte leaves EOP unset, letting the 0xffffffff sentinel
+     reach decode_map_apply (bounded garbage for dec_type 1, an out-of-range
+     q_val index for dec_type 2). Force EOP so every caller's eop check
+     rejects the vector. */
+  oggpack_seteop(b);
   return(-1);
 }
 
@@ -823,10 +851,11 @@ static inline int decode_map_apply(const decode_map_ctx *ctx,
   }
   case 3:{
     /* entry is a scalar index into the packed value array (q_pack bytes per
-       used entry), not packed bits as in types 1/2.  A sparse or single-entry
-       book can let decode_packed_entry_number fall through with
-       entry==0xffffffff and EOP unset; reject the out-of-range index before
-       it reads q_val past the end. */
+       used entry), not packed bits as in types 1/2. decode_packed_entry_number
+       now forces EOP on a tree-walk fall-through, so the 0xffffffff sentinel
+       is already caught by the oggpack_eop check above; this bound is kept as
+       a defensive net that stops any out-of-range index from reading q_val
+       past its end. */
     if(entry>=(ogg_uint32_t)s->used_entries)return(-1);
     if(ctx->q_bits_le8){
       const unsigned char *ptr =

src/tremor/info.c

@@ -111,9 +111,17 @@ static int _vorbis_unpack_info(vorbis_info *vi,oggpack_buffer *opb){
   vi->bitrate_nominal=(ogg_int32_t)oggpack_read(opb,32);
   vi->bitrate_lower=(ogg_int32_t)oggpack_read(opb,32);
 
-  ci->blocksizes[0]=1<<oggpack_read(opb,4);
-  ci->blocksizes[1]=1<<oggpack_read(opb,4);
-
+  {
+    /* Reject a truncated header before shifting: oggpack_read returns the -1
+       EOP sentinel on overrun, and 1<<-1 is undefined. Mirrors floor1.c's
+       rangebits guard; the EOP check below only fires after the shift. */
+    int bs0=oggpack_read(opb,4);
+    int bs1=oggpack_read(opb,4);
+    if(bs0<0||bs1<0)goto err_out;
+    ci->blocksizes[0]=1<<bs0;
+    ci->blocksizes[1]=1<<bs1;
+  }
+
   if(vi->rate<1)goto err_out;
   if(vi->channels<1)goto err_out;
   if(ci->blocksizes[0]<64)goto err_out;