Signed integer overflow in frame_size_select's own validation lets a huge frame_size through to a stack allocation

Inizia da frame_size_select() in src/opus_encoder.c e risali ai suoi chiamanti in opus_encode(), opus_encode24() e opus_multistream_encode_native() di src/opus_multistream_encoder.c. Esegui la riproduzione Meson/ASan fornita per osservare il problema, quindi esamina la validazione esistente e i test correlati. Il lavoro è completo quando i valori di frame_size troppo grandi vengono rifiutati con OPUS_BAD_ARG prima dell’allocazione sullo stack in tutti i punti di ingresso interessati.

Summary

frame_size_select(), the function opus_encode() calls to validate frame_size before sizing a stack allocation, contains a signed int * int multiplication in its own validity check that overflows for sufficiently large inputs. A frame_size of 1073741864 at Fs=8000 makes 200*new_size wrap around to exactly equal Fs, which defeats every branch of the check and lets the value through unmodified. opus_encode() then immediately does ALLOC(in, frame_size*st->channels, opus_res) with that value, allocating a stack buffer sized in the billions of bytes and corrupting the stack. frame_size_select() is shared, unchanged, by three call sites with the identical trust-the-return-value pattern — opus_encode(), opus_encode24(), and opus_multistream_encode_native() (reached from opus_multistream_encode()/opus_multistream_encode24()/opus_multistream_encode_float()) — all three are vulnerable via the same root cause, verified by source inspection below. We're aware opus_encode()'s documentation specifies frame_size must be a legal Opus frame duration for the given sample rate; frame_size_select() is precisely the code the library ships to enforce that and reject violations with OPUS_BAD_ARG, and we think that enforcement failing open on attacker- or bug-controlled input is worth hardening against regardless of the documented contract on the happy path.

Version

opus v1.6.1-50-g3da9f7a6

Description

opus_encode() (src/opus_encoder.c) trusts frame_size_select()'s return value and only rejects it if it's <= 0 — there is no upper-bound check at the call site at all:

// src/opus_encoder.c: opus_encode()

frame_size = frame_size_select(st->application, analysis_frame_size, st->variable_duration, st->Fs);
if (frame_size <= 0)
{
     RESTORE_STACK;
     return OPUS_BAD_ARG;
}
ALLOC(in, frame_size*st->channels, opus_res);   // <-- CRASH HERE: frame_size trusted unconditionally

So the entire burden of rejecting an illegal frame_size (per the documented contract below) falls on frame_size_select() (src/opus_encoder.c:827-852):

// src/opus_encoder.c:827-852
opus_int32 frame_size_select(int application, opus_int32 frame_size, int variable_duration, opus_int32 Fs)
{
     int new_size;
     if (frame_size<Fs/400)
          return -1;
     if (variable_duration == OPUS_FRAMESIZE_ARG)
          new_size = frame_size;
     else if (...) { ... }
     else
          return -1;
     if (new_size>frame_size)
          return -1;
     if (400*new_size!=Fs   && 200*new_size!=Fs   && 100*new_size!=Fs   &&   // <-- CRASH HERE: overflowing multiplication
          50*new_size!=Fs   &&  25*new_size!=Fs   &&  50*new_size!=3*Fs &&
          50*new_size!=4*Fs &&  50*new_size!=5*Fs &&  50*new_size!=6*Fs)
          return -1;
     if (application == OPUS_APPLICATION_RESTRICTED_SILK && new_size < Fs/100)
          return -1;
     return new_size;
}

Call path: opus_encode() → frame_size_select() → back to opus_encode()'s ALLOC(in, frame_size*st->channels, opus_res) (src/opus_encoder.c:2685).

Sibling instances, same frame_size_select() call and same "only checks <= 0, then ALLOCs unconditionally" pattern, confirmed by direct source inspection of the current checkout (not independently rebuilt/run — see caveat in Reproduction Step):

• opus_encode24() (src/opus_encoder.c:2706-2725): byte-for-byte the same shape as opus_encode(), just for 24-bit PCM input.
• opus_multistream_encode_native() (src/opus_multistream_encoder.c:841-905): same missing-upper-bound trust in the return value.

What the library does wrong, worked out precisely: st->variable_duration defaults to OPUS_FRAMESIZE_ARG (set at opus_encoder_init(), src/opus_encoder.c:309, and never changed unless the caller explicitly issues an OPUS_SET_EXPERT_FRAME_DURATION CTL — the harness doesn't), so new_size = frame_size unmodified, and if (new_size>frame_size) return -1; is trivially false (they're equal). Only the multiplication-based check stands between an arbitrary frame_size and return new_size;. new_size is a plain int; with new_size = 1073741864 and Fs = 8000: 200 * 1073741864 = 214748372800, and 214748372800 mod 2^32 = 8000 exactly (50 * 4294967296 = 214748364800, remainder 8000) — so the wrapped 32-bit result of 200*new_size is exactly 8000, making 200*new_size!=Fs evaluate false and short-circuiting the entire &&-chain to false, skipping return -1. application = OPUS_APPLICATION_VOIP (2048, a normal documented value, not OPUS_APPLICATION_RESTRICTED_SILK = 2052), so the final guard doesn't apply either. frame_size_select() returns 1073741864 unchanged, and opus_encode() immediately does ALLOC(in, frame_size*st->channels, opus_res) — a stack-allocated array sized in the billions of bytes, corrupting the stack before any of it is even used.

Why the PoC's API usage is legitimate: opus_encode()'s own doc (include/opus.h) states the function's contract for bad input is "a negative error code (see @ref opus_errorcodes) on failure" — not a crash — and frame_size_select() exists in the library specifically to enforce that contract by rejecting exactly this class of out-of-range input and mapping it to OPUS_BAD_ARG. The bug is that this specific validation, written to catch exactly this input, is defeated by an integer-overflow bug in its own implementation.

PoC Code

// poc.c
// frame_size=1073741864 is deliberately not a documented-legal Opus
// frame size at Fs=8000 -- that's the point: it's the exact input
// frame_size_select() exists to reject with OPUS_BAD_ARG, and this PoC
// demonstrates that rejection logic itself failing via integer
// overflow. pcm is heap-allocated at the full documented required
// length (frame_size*channels*sizeof(opus_int16)) so the PoC also
// satisfies that precondition precisely, not just frame_size.
#include <opus.h>
#include <stdint.h>
#include <stdlib.h>

int main(void) {
    int error;
    int channels = 1;
    opus_int32 frame_size = 1073741864;

    OpusEncoder *st = opus_encoder_create(8000, channels, OPUS_APPLICATION_VOIP, &error);
    if (st == NULL) return 1;

    opus_int16 *pcm = calloc((size_t)frame_size * channels, sizeof(opus_int16));
    if (pcm == NULL) return 1;

    unsigned char data[16];
    opus_int32 ret = opus_encode(st, pcm, frame_size, data, sizeof(data));

    return 0;
}

Stack Trace

AddressSanitizer:DEADLYSIGNAL
=================================================================
==414==ERROR: AddressSanitizer: stack-overflow on address 0x7ffe7d393120 (pc 0x740639dde017 bp 0x7fff7d393230 sp 0x7ffe7d393120 T0)
    #0 0x740639dde017 in opus_encode /root/src/libopus/src/opus_encoder.c:2688:13
    #1 0x590d06dcb923 in main /root/src/libopus/crashes/poc.c:13:24
    #2 0x740639ab91c9  (/lib/x86_64-linux-gnu/libc.so.6+0x2a1c9) (BuildId: 8e9fd827446c24067541ac5390e6f527fb5947bb)
    #3 0x740639ab928a in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x2a28a) (BuildId: 8e9fd827446c24067541ac5390e6f527fb5947bb)
    #4 0x590d06cf2364 in _start (/root/src/libopus/crashes/poc.asan+0x2b364) (BuildId: 353540275f7cef1f58106f7be95b020b7b4d9b79)

SUMMARY: AddressSanitizer: stack-overflow /root/src/libopus/src/opus_encoder.c:2688:13 in opus_encode
==414==ABORTING

Reproduction Step

git clone https://github.com/xiph/opus.git
cd opus
meson setup _build -Db_sanitize=address,undefined -Ddefault_library=shared \
  -Dtests=disabled -Ddocs=disabled
meson compile -C _build

clang poc.c -g -O1 -fsanitize=address,undefined -o poc \
    -I <path-to-opus>/include \
    -L <path-to-opus>/_build/src -lopus \
    -Wl,-rpath,<path-to-opus>/_build/src

./poc

Submission Statement

This report was produced by FuzzAnything's AI-assisted library fuzzer and manually verified by a team member. We reviewed the PoC against the upstream API documentation — call order, parameters, and memory ownership — and found no API misuse.

Signed-off-by: FuzzAnything [email protected]