[FFmpeg-devel] [PATCH] mpegpicture: use coded_width/coded_height to allocate frame

[Andreas Cadhalpun]

On 23.11.2016 15:01, Michael Niedermayer wrote:
> On Mon, Nov 07, 2016 at 10:32:29PM +0100, Andreas Cadhalpun wrote:
>> This fixes a heap-buffer-overflow in ff_er_frame_end when decoding mss2 with
>> coded_width/coded_height larger than width/height.
>>
>> Signed-off-by: Andreas Cadhalpun <Andreas.Cadhalpun at googlemail.com>
>> ---
>>  libavcodec/mpegpicture.c | 12 ++++++------
>>  1 file changed, 6 insertions(+), 6 deletions(-)
>>
>> diff --git a/libavcodec/mpegpicture.c b/libavcodec/mpegpicture.c
>> index 6748fc2..70b4d3c 100644
>> --- a/libavcodec/mpegpicture.c
>> +++ b/libavcodec/mpegpicture.c
>> @@ -108,15 +108,15 @@ static int alloc_frame_buffer(AVCodecContext *avctx,  Picture *pic,
>>          avctx->codec_id != AV_CODEC_ID_VC1IMAGE  &&
>>          avctx->codec_id != AV_CODEC_ID_MSS2) {
>>          if (edges_needed) {
>> -            pic->f->width  = avctx->width  + 2 * EDGE_WIDTH;
>> -            pic->f->height = avctx->height + 2 * EDGE_WIDTH;
>> +            pic->f->width  = avctx->coded_width  + 2 * EDGE_WIDTH;
>> +            pic->f->height = avctx->coded_height + 2 * EDGE_WIDTH;
>>          }
>>  
>>          r = ff_thread_get_buffer(avctx, &pic->tf,
>>                                   pic->reference ? AV_GET_BUFFER_FLAG_REF : 0);
>>      } else {
>> -        pic->f->width  = avctx->width;
>> -        pic->f->height = avctx->height;
>> +        pic->f->width  = avctx->coded_width;
>> +        pic->f->height = avctx->coded_height;
>>          pic->f->format = avctx->pix_fmt;
>>          r = avcodec_default_get_buffer2(avctx, pic->f, 0);
>>      }
>> @@ -135,8 +135,8 @@ static int alloc_frame_buffer(AVCodecContext *avctx,  Picture *pic,
>>                           (EDGE_WIDTH >> (i ? chroma_x_shift : 0));
>>              pic->f->data[i] += offset;
>>          }
>> -        pic->f->width  = avctx->width;
>> -        pic->f->height = avctx->height;
>> +        pic->f->width  = avctx->coded_width;
>> +        pic->f->height = avctx->coded_height;
>>      }
> 
> why would the error concealment code need a differently sized output
> than the decoder itself ?

This code is rather convoluted, so it's not quite obvious, however the
needed buffer size for the error concealment is determined by the
MpegEncContext.width/height, which is set via:
mss2_decode_init
|-> wmv9_init
    |-> ff_msmpeg4_decode_init
        |-> ff_h263_decode_init
            |-> ff_mpv_decode_init:
    s->width           = avctx->coded_width;
    s->height          = avctx->coded_height;

Thus the buffer needs the same size.

Best regards,
Andreas