search

xiph / opus

Modern audio compression for the internet.
https://opus-codec.org/
Other
2.31k stars 619 forks source link

Limiting the stack allocation request #329

Open gal1ium opened 7 months ago

gal1ium commented 7 months ago

Hi! We're testing with Opus APIs and want to suggest a potential fix in opus_decode for limiting a stack allocation request ALLOC. Its argument frame_size is passed into ALLOC (https://github.com/xiph/opus/blob/v1.5.1/src/opus_decoder.c#L884) and maybe there could be a check before the stack allocation (if it doesn't enter line https://github.com/xiph/opus/blob/v1.5.1/src/opus_decoder.c#L875) so that it wouldn't accept any bad value and having undefined behaviors?

jmvalin commented 7 months ago

Can you be a bit more specific about what you're suggesting?

gal1ium commented 7 months ago

When calling opus_decode with a bad value (too large) argument frame_size, the ALLOC(out, frame_size*st->channels, float); is requesting a large stack allocation (stack overflow), if it doesn't go into the branch frame_size = IMIN(frame_size, nb_samples) before to limit the frame_size in https://github.com/xiph/opus/blob/v1.5.1/src/opus_decoder.c#L875.

I'm wondering if something like if (frame_size*st->channels > a_threshold) { return OPUS_BAD_ARG; } right before the line https://github.com/xiph/opus/blob/v1.5.1/src/opus_decoder.c#L884 is necessary?