Skip to content

stb_vorbis: fix codebook integer overflows in lookup_type 2 and pre-expansion - #2028

Closed
TristanInSec wants to merge 1 commit into
nothings:masterfrom
TristanInSec:fix/vorbis-codebook-overflow
Closed

TristanInSec wants to merge 1 commit into
nothings:masterfrom
TristanInSec:fix/vorbis-codebook-overflow

Conversation

@TristanInSec

Copy link
Copy Markdown

Summary

Two independent integer overflow bugs in the Vorbis codebook setup path:

1. lookup_type 2: entries * dimensions overflow (line 3860)

c->lookup_values = c->entries * c->dimensions multiplies two int values (24-bit entries from bitstream, 16-bit dimensions) that can exceed INT_MAX, causing signed integer overflow (undefined behavior). The wrapped result leads to an undersized multiplicands allocation, and OOB heap reads at decode time when z *= c->dimensions indexes past the array.

2. lookup_type 1 pre-expansion: index overflow (line 3889)

The index expression j * c->dimensions + k is computed as int * int + int. For large codebook parameters (e.g. j >= 32768 with dimensions = 65535), the product exceeds INT_MAX, causing signed overflow. The wrapped index causes OOB writes into the multiplicands heap allocation during codebook setup.

Fix

  1. Add overflow check for entries * dimensions before storing to lookup_values, and compute in uint32 arithmetic.
  2. Add entries * dimensions bounds checks before the pre-expansion allocation (both sparse and non-sparse paths).
  3. Cast index arithmetic in the pre-expansion loop to unsigned int to avoid signed overflow UB.

…xpansion

Two independent integer overflow bugs in the Vorbis codebook setup:

1. lookup_type 2: `c->lookup_values = c->entries * c->dimensions`
   multiplies two int values (24-bit entries, 16-bit dimensions) that
   can exceed INT_MAX, causing signed overflow (UB). The wrapped value
   leads to undersized multiplicands allocation and OOB read at decode
   time. Add overflow check and compute in uint32.

2. lookup_type 1 pre-expansion: the index `j*c->dimensions + k`
   overflows as int*int for large codebook parameters, causing OOB
   write into the multiplicands array during setup. Add overflow
   bounds check on entries*dimensions before allocation, and cast
   the index arithmetic to unsigned int to avoid signed overflow UB.
@rygorous

rygorous commented Nov 1, 2026

Copy link
Copy Markdown
Collaborator

See comments for previous PR. We now enforce a much tighter limit for dimension*entries (at most 1 << 18) since all codebooks used in practice are in fact well below 65536 entries, and allowing huge codebooks is still a potential DoS vector. Since as far as we can tell these huge codebooks never actually occur in "real" Vorbis files, there is no reason to support them in the first place.

As for 2), in that loop, j is bounded above by c->entries and k by c->dimensions; since the version in the current dev branch (as of a5ccde4) guarantees that c->entries * c->dimensions <= CODEBOOK_LIMIT (which is the aforementioned 1 << 18), that means we're not at risk of 32-bit integer overflow anymore.

All these fixes are already in the dev branch and will be in the next release.

@rygorous rygorous closed this Nov 1, 2026
@TristanInSec

Copy link
Copy Markdown
Author

Agreed, the codebook entry limit is the right approach -- eliminates the overflow class entirely. Thanks for the explanation on the dev branch changes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants