[FFmpeg-devel] Buffer overflow in ALS decoder

[Reimar Döffinger]

On Wed, Feb 17, 2010 at 07:56:55PM -0500, Justin Ruggles wrote:
> Thilo Borgmann wrote:
> 
> > Am 18.02.10 00:07, schrieb Reimar D?ffinger:
> >> On Wed, Feb 17, 2010 at 11:27:43PM +0100, Thilo Borgmann wrote:
> >>> Index: libavcodec/alsdec.c
> >>> ===================================================================
> >>> --- libavcodec/alsdec.c	(Revision 21849)
> >>> +++ libavcodec/alsdec.c	(Arbeitskopie)
> >>> @@ -1563,7 +1563,7 @@
> >>>      // allocate and assign channel data buffer for mcc mode
> >>>      if (sconf->mc_coding) {
> >>>          ctx->chan_data_buffer  = av_malloc(sizeof(*ctx->chan_data_buffer) *
> >>> -                                           num_buffers);
> >>> +                                           num_buffers * num_buffers);
> >>>          ctx->chan_data         = av_malloc(sizeof(ALSChannelData) *
> >>>                                             num_buffers);
> >> Just fix this as well, you are allocating too much, chan_data is ALSChannelData **,
> >> thus it should be sizeof(ALSChannelData *), not sizeof(ALSChannelData) - or
> >> actually better for consistency sizeof(*ctx->chan_data).
> > 
> > Indeed! Tested & applied.
> > 
> > 
> >> Though assuming there is an upper limit on num_buffers and its not above
> >> 40 or so, I'd suggest to have chan_data a fixed-size array in the context
> >> instead of allocating it.
> > 
> > Well there is none and conformance files include one with 512 channels (
> > = num_buffers in that case)...
> 
> I think the upper limit is 65536 channels.

Uh, in that case you will have an integer overflow in the allocation for
chan_data_buffer.
You'd have to add something like
if (num_buffers >= 65535 || num_buffers * num_buffers > INT_MAX / sizeof(*ctx->chan_data_buffer))
    return AVERROR(ENOMEM);