Possible buffer underflow caused by integer overflow in the image conversion routines

[emk]

This is a source code review based observation of the image conversion routines

latest snapshot libavcodec/imgconvert.c

possibly Vulnerable Function avpicture_layout line 618

While reviewing I noticed that there are checks to protect against 'size'
integer overflows :
    if (size > dest_size || size < 0)
        return -1;

doesnt seem though to be a check to protect against integer overflows on 'w'
below, seems that an integer overflow on lines 638 or 636 may lead to a
buffer underflow (due to a signedness error) in memcpy on line 659

--- snippet ---

 if (pf->pixel_type == FF_PIXEL_PACKED || pf->pixel_type == FF_PIXEL_PALETTE) {
        if (pix_fmt == PIX_FMT_YUV422 ||
            pix_fmt == PIX_FMT_UYVY422 ||
            pix_fmt == PIX_FMT_BGR565 ||
            pix_fmt == PIX_FMT_BGR555 ||
            pix_fmt == PIX_FMT_RGB565 ||
            pix_fmt == PIX_FMT_RGB555)
638: w = width * 2;
        else if (pix_fmt == PIX_FMT_UYVY411)
636: w = width + width/2;
.......

     s = src->data[i];
         for(j=0; j<h; j++) {
659: memcpy(dest, s, w);

[Kees Cook (kees)]

Thanks for the bug report! Do you have a public reproducer to help test for this issue?

[Paul Gevers (paul-climbing)]

Kees Cook wrote:
> Thanks for the bug report! Do you have a public reproducer to help test
> for this issue?

The bug report at upstream [1] states that this bug was closed/invalid
because:

[quote]
I dont see how this can overflow, if you disgaree please provide a full
example with values.
RESPONSE:
nope!! you are right , I didn't notice the uint64_t casting the size
check which limits the width to smaller values !!
[/quote]

[1] http://roundup.mplayerhq.hu/roundup/ffmpeg/issue701

[Jamie Strandboge (jdstrand)]

Thanks Paul. Marking Invalid per upstream bug report.